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

Handle conditions without a type of "Ready" #2837

Merged
merged 1 commit into from
Oct 6, 2022
Merged

Conversation

joshri
Copy link
Contributor

@joshri joshri commented Oct 5, 2022

Closes #2796

In adding display of a lot more objects in #2765, we noticed that computeReady and computeMessage weren't working as expected in some of the new object kinds (the most visible example was in CRDs in the ReconciledObjectsTable).

What was happening here is that we were specifically looking for conditions with a type of Ready or Available, and assigning NotReady to the status when that type didn't exist. It turns out that CRDs don't have a conditions object with a type of Ready, but that doesn't mean that they are failing.

What made the most sense to me for now was to add a little extra handling for when Ready isn't there - if ANY conditions object has a status of "False", we assign NotReady and display that conditions message, otherwise, we assign "Ready" and display the message of the first conditions object in the list.

The problems with this are:

  • Some conditions use negative polarity, where the normal state is actually False. There's no standard for indicating this.
  • When all conditions are True, the message of the first condition in the list might not be the most relevant. There is no standard on how to indicate which conditions are more important than others.

I found this which is a.....semi-recent discussion on setting some new standards for conditions
kubernetes/community#4521

Here's what the guidelines say right now:
https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#spec-and-status

Here's a screenshot of the ReconciledObjectsTable after my changes:
image

@joshri joshri added the area/ui Issues that require front-end work label Oct 5, 2022
@opudrovs
Copy link
Contributor

opudrovs commented Oct 5, 2022

LGTM! 🎉 A very interesting and tricky issue.

Copy link
Contributor

@ozamosi ozamosi left a comment

Choose a reason for hiding this comment

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

✨ That's a great investigation, and a brilliant writeup in the PR of the outcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ui Issues that require front-end work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect message from computeMessage function
3 participants