Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Partition on ConsulUpstream, ConsulTerminatingConfigEntry, ConsulIngressConfigEntry and ConsulIngressService #16768

Conversation

Sol-Stiep
Copy link
Contributor

@Sol-Stiep Sol-Stiep commented Apr 3, 2023

Description

Changes

  • Updated ConsulUpstream, ConsulTerminatingConfigEntry, ConsulIngressConfigEntry and ConsulIngressService Structs adding the Partition field.
  • Update related tests
  • Update Nomad Documentation for ConsulUpstream, ConsulTerminatingConfigEntry, ConsulIngressConfigEntry and ConsulIngressService Structs.

Progress

Work completed:

  • Added partition fields at api/consul.go and nomad/structs/services.go, on the following structs:
    • ConsulUpstream
    • ConsulIngressService
    • ConsulTerminatingConfigEntry
    • ConsulIngressConfigEntry
  • Update receiver functions at api/consul.go and nomad/structs/services.go:
    • Copy
    • Equal
  • Update parse and apiConnect on jobspec/parse_service.go and command/agent/job_endpoint.go
  • Update connect and conver functions on command/agent/consul/connect.go and nomad/consul.go
  • Update mdx docs.
  • Apply inherit behavior on partition field for ConsulIngressService:
    • When ConsulIngressService partition is empty, set value from ConsulIngressConfigEntry partition field.

Pending work:

  • We assumed that partition should only be inherited on ConsulIngressService, since is the only struct that have a partition defined in upper levels(ConsulIngressConfigEntry). Is pending if this assumption is correct, or if field value should be inherited in other cases too.
  • Running end-to-end tests. Since partitions are Consul Enterprise fields, we couldn't tested end to end, as we test other values.

@tgross
Copy link
Member

tgross commented Mar 21, 2024

@mmcquillan I've been looking at this PR as part of the rest of the Consul API update work, and it's not clear to me this makes sense to for anything other than upstream blocks (which is done in #20167). The way we implemented admin partition support fingerprinting implies that everything originating from a specific Nomad node is associated with a specific Consul agent, which is in exactly one admin partition.

I'll probably circle back to this one after we've completed the remainder of the Consul work for 1.8.0.

@tgross tgross modified the milestones: 1.7.x, 1.8.0 Apr 15, 2024
@tgross tgross added the stage/needs-rebase This PR needs to be rebased on main before it can be backported to pick up new BPA workflows label May 17, 2024
@tgross tgross modified the milestones: 1.8.0, 1.8.x Jun 4, 2024
@tgross
Copy link
Member

tgross commented Jun 4, 2024

Implemented in #19665 #20167, and #19665

@tgross tgross closed this Jun 4, 2024
@tgross tgross modified the milestones: 1.8.x, 1.8.0 Jun 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stage/needs-rebase This PR needs to be rebased on main before it can be backported to pick up new BPA workflows theme/consul
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants