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

Consul with CNI and host_network addresses #9095

Merged
merged 6 commits into from
Oct 15, 2020

Conversation

nickethier
Copy link
Member

@nickethier nickethier commented Oct 15, 2020

This PR implements a new service and check address_mode named alloc. The alloc service address mode signals that the address used when making Consul service and check registrations should be derived from the allocation's network namespace. This enables addresses which are created via CNI to be registered directly with Consul rather than having to expose them through Nomad's port mapping.

I added some validation to error when this mode is uses with service blocks under a task.

This PR also uses the correct host_network address when registering in host/default mode.

fixes #8801
fixes #8698

@shoenig
Copy link
Member

shoenig commented Oct 15, 2020

Looks like a number of tests are gonna need newGroupServiceHook.networkStatusGetter set

Copy link
Member

@shoenig shoenig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@nickethier nickethier merged commit 7b50685 into master Oct 15, 2020
@nickethier nickethier deleted the f-consul-cni-registration branch October 15, 2020 19:32
@nickethier nickethier added this to the 1.0 milestone Oct 15, 2020
fredrikhgrelland pushed a commit to fredrikhgrelland/nomad that referenced this pull request Oct 22, 2020
* consul: advertise cni and multi host interface addresses

* structs: add service/check address_mode validation

* ar/groupservices: fetch networkstatus at hook runtime

* ar/groupservice: nil check network status getter before calling

* consul: comment network status can be nil
@Legogris
Copy link

Legogris commented Oct 28, 2020

This looks really promising :)

@tgross @schmichael I see this is slated for 1.0 - what are your thoughts on releasing a 0.12.8 with this included? Could make the upgrade path to 1.0 a lot smoother I think.

@tgross
Copy link
Member

tgross commented Oct 28, 2020

Hi @Legogris generally speaking we don't backport features to previous versions of Nomad, only critical bugs and security patches.

@Legogris
Copy link

Legogris commented Oct 28, 2020

@tgross This is a bugfix, no? Given that only with this change is it possible (hopefully!) to get the mapping between services and IPs working as documented when running with Consul (this has been broken since 0.12 as brought up in several issues). I don't know how representative we are but this is a significant one here.

@Legogris
Copy link

Legogris commented Nov 8, 2020

@nickethier Since you've been the one mostly working on this, what's your stance on the above?

Just so there's no confusion, using host_network configuration with 0.12.8 is currently registering corresponding Consul services under arbitrary IP addresses and the feature (which was introduced in 0.12) is therefore unusable together with Consul.

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to announce CNI provided IP address in consul Use IP returned from CNI to register service in Consul
4 participants