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

healthv2: Various fixes #32549

Merged
merged 5 commits into from
May 17, 2024
Merged

Conversation

joamaki
Copy link
Contributor

@joamaki joamaki commented May 15, 2024

This fixes a set of small issues with healthv2:

  • Degraded message was not passed onto the status
  • Error not shown in table dump or in debug logs
  • deletePrefix deleted beyond just the prefix (everything that had id equal to or larger got deleted)
  • Empty status was inserted on stop() if a health provider was unused

Marking this as release-note/misc as healthv2 is not yet part of any released version.

- Set the message that was passed in Degraded()
- Show the degraded error in table dump and in debug logs

Signed-off-by: Jussi Maki <jussi@isovalent.com>
deletePrefix used LowerBound without checking the prefix,
and thus would've iterated over and deleted statuses that did not
match prefix. Use Prefix() search instead.

Signed-off-by: Jussi Maki <jussi@isovalent.com>
The message & level might stay the same but error may change,
so check for that as well before skipping the update.

Signed-off-by: Jussi Maki <jussi@isovalent.com>
QueryFromObject is pretty indirect. Better to do the queries
directly with the search term.

Signed-off-by: Jussi Maki <jussi@isovalent.com>
If a health reporter has not been used we should not
insert an (empty) status when it stops.

Signed-off-by: Jussi Maki <jussi@isovalent.com>
@joamaki joamaki requested a review from a team as a code owner May 15, 2024 12:35
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label May 15, 2024
@joamaki joamaki added the release-note/misc This PR makes changes that have no direct user impact. label May 15, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label May 15, 2024
@joamaki
Copy link
Contributor Author

joamaki commented May 15, 2024

/test

1 similar comment
@joamaki
Copy link
Contributor Author

joamaki commented May 16, 2024

/test

@joamaki joamaki enabled auto-merge May 17, 2024 09:55
@joamaki joamaki added this pull request to the merge queue May 17, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label May 17, 2024
Merged via the queue into cilium:main with commit ac0a45e May 17, 2024
66 checks passed
@joamaki joamaki deleted the pr/joamaki/health-show-error branch May 17, 2024 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants