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

added host network interpolation #10196

Merged
merged 3 commits into from
Apr 13, 2021
Merged

added host network interpolation #10196

merged 3 commits into from
Apr 13, 2021

Conversation

AndrewChubatiuk
Copy link
Contributor

Fixes #10195

@hashicorp-cla
Copy link

hashicorp-cla commented Mar 18, 2021

CLA assistant check
All committers have signed the CLA.

@vercel vercel bot temporarily deployed to Preview – nomad March 18, 2021 10:05 Inactive
@vercel vercel bot temporarily deployed to Preview – nomad March 18, 2021 10:11 Inactive
@tgross tgross added this to Needs Triage in Nomad - Community Issues Triage via automation Mar 22, 2021
@tgross
Copy link
Member

tgross commented Mar 23, 2021

Hi @AndrewChubatiuk sorry about the delay here... I want to make sure that the root problem of #10106 is better understood before we review this.

@tgross tgross self-requested a review March 23, 2021 14:10
@tgross tgross moved this from Needs Triage to In Progress in Nomad - Community Issues Triage Mar 23, 2021
Copy link
Member

@tgross tgross left a comment

Choose a reason for hiding this comment

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

Hi @AndrewChubatiuk! I'm not sure I understand what this approach to interpolation is doing. But generally speaking we do any interpolation that's going to happen at either job submission time or on the client (sometimes both). It might help if there were tests?

@AndrewChubatiuk
Copy link
Contributor Author

AndrewChubatiuk commented Mar 24, 2021

@tgross
Without these changes workload will not even schedule on any agent
Could you please advise where can I move these changes?

@tgross
Copy link
Member

tgross commented Mar 25, 2021

@AndrewChubatiuk maybe the code is fine where it is, but there's no tests and no "interpolation" going on here, so I can't even use the commit message to figure out what you're doing. If you can provide more context, I can review it.

@AndrewChubatiuk
Copy link
Contributor Author

AndrewChubatiuk commented Mar 25, 2021

@tgross
using resolveTarget function which makes this "interpolation". I'll add tests anyway

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

@tgross
Added some tests

@vercel vercel bot temporarily deployed to Preview – nomad April 13, 2021 13:32 Inactive
@tgross
Copy link
Member

tgross commented Apr 13, 2021

LGTM. I've rebased this on main to pick up recent fixes and added two more smallish tests. Once this is ✅ on CI I'll merge it.

@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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

networking: add host network interpolation
3 participants