Skip to content
This repository has been archived by the owner on May 21, 2024. It is now read-only.

health_check: Allow "unhealthy" #77

Merged
merged 2 commits into from
Dec 8, 2023
Merged

health_check: Allow "unhealthy" #77

merged 2 commits into from
Dec 8, 2023

Conversation

utopiabound
Copy link
Contributor

Fix regression to allow bare "unhealthy" to be returned for health_check.

Also update version to 0.7.7

@utopiabound utopiabound added the bug Something isn't working label Dec 7, 2023
@utopiabound utopiabound requested review from jgrund and RDruon December 7, 2023 15:29
@utopiabound utopiabound self-assigned this Dec 7, 2023
Copy link
Member

@jgrund jgrund left a comment

Choose a reason for hiding this comment

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

Thanks, needs fixture and test too.

@RDruon
Copy link
Contributor

RDruon commented Dec 7, 2023

Can such string occurs? From the code it seems to be either healthy or NOT HEALTHY https://github.com/whamcloud/lustre/blob/a336d7c7c1cd62a5a5213835aa85b8eaa87b076a/lustre/obdclass/obd_sysfs.c#L269-L274

src/top_level_parser.rs Outdated Show resolved Hide resolved
Copy link
Member

@jgrund jgrund left a comment

Choose a reason for hiding this comment

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

Please add fixture of params including older lustre version in this state.

Fix regression to allow bare "NOT HEALTH" and "LBUG" to be returned for health_check.

Signed-off-by: Nathaniel Clark <nclark@whamcloud.com>
RDruon
RDruon previously approved these changes Dec 7, 2023
Use 6.2.0 fixture to test NOT HEALTHY

Signed-off-by: Nathaniel Clark <nclark@whamcloud.com>
healthy: false,
targets: vec![],
})),
(string("NOT HEALTHY").map(|_| HealthCheckStat {
Copy link
Member

Choose a reason for hiding this comment

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

Nits:

  • This should be a const since we repeat it below
  • We should condense these two blocks with an or

@jgrund jgrund merged commit fa2225f into master Dec 8, 2023
5 checks passed
@jgrund jgrund deleted the nclark/unhealthy branch December 8, 2023 19:37
@jgrund
Copy link
Member

jgrund commented Dec 8, 2023

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants