ui: [BUGFIX] Fix missing or duplicate service instance health checks #9660
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
johncowen
added
type/bug
|
🍒 If backport labels were added before merging, cherry-picking will start automatically. To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/319581. |
|
🍒✅ Cherry pick of commit 7c277aa onto |
hashicorp-ci
pushed a commit
that referenced
this pull request
Jan 29, 2021
…9660) * Use NodeName not Node for cross checking proxies/instances * Also copy over the meta data to keep the correct cursor/index * When we sync checks to the ProxyInstance replace rather than accumulate
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #9648
This PR fixes essentially two issue related to the listing of healthchecks correctly.
In #9314 we changed our separate Proxy/ServiceInstance models to use a single ServiceInstance model extending from the Proxy model. This meant we had a single model to represent a Service Instance that happened to be a connect sidecar proxy. Unfortunately we didn't realize a clash of property names when we did this, both the Proxy endpoint and the ServiceInstance endpoint contain a
Nodeproperty that are two different shapes. One is a plain string that is the name of the node and one is a map/object containing all the properties of a node (including its name),"name"vs{Name: "name"}.In #9524 we added a fix that added a
NodeNameproperty and mapped the string basedNodeproperty to that instead, meaning these special ProxyServiceInstance models have bothNodeName= "name"andNode = {Name: "name"...}.Unfortunately we missed that we were using the
Nodeproperty to link our instances and proxy instances together. This updates this code to use theNodeNameproperty.Additionally we noticed that proxy instance healthchecks could potentially accumulate instead of being replaced when blocking queries would respond, eternally adding more and more duplicate healthchecks to a proxy service instance.
The second fix here solves this by resetting the checks array before copying over the healthchecks to the proxy service instance.