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.

Sign up for GitHub

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

Jump to bottom

added multiple host network aliases for the same interface #10104

Merged
merged 5 commits into from
Apr 13, 2021
Merged

added multiple host network aliases for the same interface #10104

merged 5 commits into from
Apr 13, 2021

Conversation

AndrewChubatiuk
Copy link
Contributor

@AndrewChubatiuk AndrewChubatiuk commented Mar 1, 2021 •
edited
Loading

FIxes #10106

@vercel vercel bot temporarily deployed to Preview – nomad March 1, 2021 10:38 Inactive
Base automatically changed from master to main March 8, 2021 19:25
tgross
tgross requested changes Mar 24, 2021
View reviewed changes
nomad/structs/structs.go Outdated
@@ -2439,7 +2441,7 @@ const (

type NodeNetworkAddress struct {
Family NodeNetworkAF
Alias string
Aliases []string
Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately this approach breaks backwards compatibility for nodes that are already saved in raft. Given that we can already store multiple NodeNetworkAddress for a node, we can solve this problem by writing one for each alias from the node fingerprint.

@vercel vercel bot temporarily deployed to Preview – nomad March 24, 2021 21:25 Inactive
@vercel vercel bot temporarily deployed to Preview – nomad March 24, 2021 21:31 Inactive
@AndrewChubatiuk
Copy link
Contributor Author

@tgross I've updated it

@vercel vercel bot temporarily deployed to Preview – nomad March 24, 2021 22:42 Inactive
@tgross
Copy link
Member

tgross commented Apr 13, 2021

LGTM. I've pushed a small style change to the test; you copied what was nearby which was totally the right thing for you to do but we've been using require in new tests over the last year or two and have been happy with it.

@AndrewChubatiuk
Copy link
Contributor Author

@tgross
okay, thank you

@vercel vercel bot temporarily deployed to Preview – nomad April 13, 2021 13:14 Inactive
@tgross tgross merged commit bab0d39 into hashicorp:main Apr 13, 2021
This was referenced Apr 13, 2021
Closed
Merged
@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 Nov 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@tgross tgross tgross approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

support multiple host_network names for the same interface
2 participants
@AndrewChubatiuk @tgross