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

Condition transitions False->Progressing on reason/message change #3013

Merged
merged 1 commit into from
Oct 23, 2020

Conversation

rfranzke
Copy link
Member

@rfranzke rfranzke commented Oct 21, 2020

How to categorize this PR?

/area ops-productivity
/kind enhancement
/priority normal

What this PR does / why we need it:
Failed conditions on Shoots do now transition from False status to Progressing in case the reason or message changes (if thresholds are defined in the gardenlet component config only, otherwise Progressing is not used anyways).
This will improve ops productivity as a condition will no longer stay False for a long time although different errors are happening subsequently.

Special notes for your reviewer:
/cc @timuthy

Release note:

Failed conditions on `Shoot`s do now transition from `False` status to `Progressing` in case the reason or message changes (if thresholds are defined in the gardenlet component config only, otherwise `Progressing` is not used anyways).

@rfranzke rfranzke requested a review from a team as a code owner October 21, 2020 12:10
@gardener-robot gardener-robot added area/ops-productivity Operator productivity related (how to improve operations) kind/enhancement Enhancement, improvement, extension priority/normal size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 21, 2020
timuthy
timuthy previously approved these changes Oct 22, 2020
Copy link
Member

@timuthy timuthy left a comment

Choose a reason for hiding this comment

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

/lgtm

@rfranzke
Copy link
Member Author

Any further opinions?

Copy link
Member

@timebertt timebertt left a comment

Choose a reason for hiding this comment

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

Hmm, TBH I'm not completely sure about the implications of this change.
I think, we need a bit more sophisticated logic here, unfortunately.
WDYT?

pkg/operation/botanist/health_check.go Outdated Show resolved Hide resolved
@timebertt
Copy link
Member

/status author-action

@gardener-robot
Copy link

@rfranzke The pull request was assigned to you under author-action. Please unassign yourself when you are done. Thank you.

@rfranzke
Copy link
Member Author

/unassign
/needs review

Incorporated feedback from @timebertt to only react on changes of the reason field (not message).

Copy link
Member

@timebertt timebertt left a comment

Choose a reason for hiding this comment

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

Thanks
/lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ops-productivity Operator productivity related (how to improve operations) kind/enhancement Enhancement, improvement, extension size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants