Skip to content
New issue

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

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

Already on GitHub? # to your account

Fix multi-interface networking in the system scheduler #8822

Merged
merged 1 commit into from
Sep 22, 2020
Merged

Fix multi-interface networking in the system scheduler #8822

merged 1 commit into from
Sep 22, 2020

Conversation

neilmock
Copy link
Contributor

@neilmock neilmock commented Sep 3, 2020

This was originally mentioned in #8208 (comment), and purportedly fixed in #8230, but we noticed our system job appeared to bind on all interfaces instead of the specified host_network.

We noticed we were always evaluating to the first half of this conditional which didn't seem right:

if len(alloc.AllocatedResources.Shared.Ports) == 0 && len(alloc.AllocatedResources.Shared.Networks) > 0 {

This is a proposed fix, it seems correct given previous discussion but we are new to Nomad internals.

@hashicorp-cla
Copy link

hashicorp-cla commented Sep 3, 2020

CLA assistant check
All committers have signed the CLA.

@notnoop
Copy link
Contributor

notnoop commented Sep 18, 2020

Thank you very much for this contribution. Any chance you can add a test as well?

@neilmock
Copy link
Contributor Author

Thank you very much for this contribution. Any chance you can add a test as well?

I may not have the bandwidth to test this fully, as I don't think much of the multi-interface networking code is under coverage at the moment; e.g. we are also seeing issues with multi-interface IP assignment in Consul service registrations (which we have patched in our fork). But I'm very much willing to collaborate on the effort to wrangle these bugs.

@nickethier
Copy link
Member

Thanks @neilmock I'll take on trying to get this tested a bit better. I'd love to hear more about any other patches you've made wrt multi-interface IP networking.

@nickethier nickethier merged commit ed385bd into hashicorp:master Sep 22, 2020
@neilmock
Copy link
Contributor Author

Thanks @neilmock I'll take on trying to get this tested a bit better. I'd love to hear more about any other patches you've made wrt multi-interface IP networking.

We haven't done much, but we did put in a hack to make Consul register the correct host IP/interface for services. You can see a diff here.

master...getenv:getenv

tgross added a commit that referenced this pull request Dec 7, 2020
tgross added a commit that referenced this pull request Dec 7, 2020
@neilmock neilmock deleted the getenv-fix-system-multi-interface-networking branch December 29, 2020 22:57
@github-actions
Copy link

github-actions bot commented Dec 5, 2022

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 5, 2022
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants