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 cke_node_reboot_status metrics #660

Merged
merged 3 commits into from
Sep 12, 2023
Merged

Fix cke_node_reboot_status metrics #660

merged 3 commits into from
Sep 12, 2023

Conversation

zoetrope
Copy link
Member

@zoetrope zoetrope commented Sep 7, 2023

Currently, there is an issue with cke_node_reboot_status metrics not being reflected correctly in the following cases:

  • When drain/uncordon is repeated
  • When node additions and deletions occur with sabakan integration

This PR calculates the value of metrics each time they are collected.
As a result we will be able to get correct metrics.

@zoetrope zoetrope self-assigned this Sep 7, 2023
@zoetrope zoetrope changed the title WIP Fix node metrics Sep 7, 2023
@zoetrope zoetrope force-pushed the fix-node-metrics branch 4 times, most recently from fb07a25 to 8b23c08 Compare September 7, 2023 07:00
@zoetrope zoetrope changed the title Fix node metrics Fix cke_node_reboot_status metrics Sep 7, 2023
@zoetrope zoetrope marked this pull request as ready for review September 7, 2023 07:50
Copy link
Contributor

@morimoto-cybozu morimoto-cybozu left a comment

Choose a reason for hiding this comment

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

The new logic seems good!
I can only leave trivial comments.

metrics/collector.go Show resolved Hide resolved
metrics/collector.go Show resolved Hide resolved
metrics/collector.go Show resolved Hide resolved
metrics/metrics.go Outdated Show resolved Hide resolved
metrics/metrics.go Outdated Show resolved Hide resolved
metrics/metrics.go Outdated Show resolved Hide resolved
metrics/updater_test.go Outdated Show resolved Hide resolved
sabakan/generator.go Outdated Show resolved Hide resolved
metrics/collector.go Outdated Show resolved Hide resolved
Co-authored-by: morimoto-cybozu <[email protected]>
Copy link
Contributor

@morimoto-cybozu morimoto-cybozu left a comment

Choose a reason for hiding this comment

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

LGTM

@zoetrope zoetrope merged commit e5a1269 into main Sep 12, 2023
6 checks passed
@zoetrope zoetrope deleted the fix-node-metrics branch September 12, 2023 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants