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: missing UDP field in checkType #14885
Conversation
FYI @dhiaayachi - this is a small follow-up fix on top of #12722 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@huikang : I love seeing 1-line PRs that fix an issue with a nice test :) thank you for this.
I'll defer to the engineering team on formally approving.
I feel like this bugfix deserves a changelog entry. Can you add one like this?
```release-note:bug
checks: Fixed a bug that prevented registration of UDP health checks from agent configuration files, such as service definition files with embedded health check definitions.
```
@@ -1562,6 +1562,7 @@ func (b *builder) checkVal(v *CheckDefinition) *structs.CheckDefinition { | |||
Body: stringVal(v.Body), | |||
DisableRedirects: boolVal(v.DisableRedirects), | |||
TCP: stringVal(v.TCP), | |||
UDP: stringVal(v.UDP), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see agent/config/builder.go
mentioned in the developer docs that type to capture all the places that need to be checked for health check changes. Can you mention the config builder file? Maybe it can just be included in lines 4 and 5 (which both reference agent/config/config.go
, which I think rely on this builder.go file).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realize I didn't link to the dev docs I was referencing 🤦
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jkirschner-hashicorp , thanks. The doc is updated; please take a look.
Removed backports to 1.11 and 1.12. It seems the UDP health check functionality was first added in 1.13.0, per the changelog: https://github.com/hashicorp/consul/blob/main/CHANGELOG.md#1130-august-9-2022 |
Description
Fix missing UDP field in builder, when the build parses the service definition file.
Testing & Reproduction steps
Refer to #14864
Links
Fix #14864
PR Checklist