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

fix: use interpolated address when performing health checks #18584

Merged
merged 3 commits into from
Oct 4, 2023

Conversation

MattJustMatt
Copy link
Contributor

Fixes #18574

Copy link
Member

@jrasell jrasell left a comment

Choose a reason for hiding this comment

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

Hi @MattJustMatt and thanks for the PR. The approach seems right to me, but. the change in function signature has broken some tests, which will need to be fixed up. It would also be nice to add some additional tests to cover the new behaviour and ensure interpolated service checks work as we now expected. Finally, a changelog entry for the addition can be included by running the make cl command.

Thanks again!

@jrasell jrasell moved this from Needs Triage to In Progress in Nomad - Community Issues Triage Sep 27, 2023
@MattJustMatt
Copy link
Contributor Author

@jrasell Thanks for the review!

I've fixed the broken tests and added the changelog entry. I don't have a good enough understanding of the test infra to easily add a regression test for this, but if it's needed for this PR please let me know and I can dive in deeper.

.changelog/18584.txt Outdated Show resolved Hide resolved
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! If you don't mind just tweaking the CL comment so it clarifies this is about nomad service checks @MattJustMatt

@MattJustMatt
Copy link
Contributor Author

100% cool with me!

Co-authored-by: Seth Hoenig <shoenig@duck.com>
@vercel
Copy link

vercel bot commented Oct 3, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
nomad-storybook-and-ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 3, 2023 5:34pm

@vercel
Copy link

vercel bot commented Oct 3, 2023

@MattJustMatt is attempting to deploy a commit to the HashiCorp Team on Vercel.

A member of the Team first needs to authorize it.

@shoenig shoenig added backport/1.4.x backport to 1.4.x release line backport/1.5.x backport to 1.5.x release line backport/1.6.x backport to 1.6.x release line labels Oct 4, 2023
@shoenig shoenig requested a review from jrasell October 4, 2023 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.4.x backport to 1.4.x release line backport/1.5.x backport to 1.5.x release line backport/1.6.x backport to 1.6.x release line
Projects
Development

Successfully merging this pull request may close these issues.

Nomad native service health checks fail when using interpolated service address in job
3 participants