Skip to content

Conversation

makkes
Copy link
Member

@makkes makkes commented Jul 4, 2022

This commit fixes that field never being set to "true" when the
respective condition is "true", eventually leading to the UI always
marking it with a red indicator.

This commit fixes that field never being set to "true" when the
respective condition is "true", eventually leading to the UI always
marking it with a red indicator.
@makkes makkes requested a review from bigkevmcd July 4, 2022 15:55
@makkes makkes self-assigned this Jul 4, 2022
@makkes makkes added the bug Something isn't working label Jul 4, 2022
Copy link
Collaborator

@yiannistri yiannistri left a comment

Choose a reason for hiding this comment

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

🥇

@foot
Copy link
Collaborator

foot commented Jul 5, 2022

I wonder if the controlPlaneInitialized field should even be part of that list to begin with because the other two are part of Cluster.Status field while controlPlaneInitialized is a condition.

This is a great point. Though its a minimal diff in this case, I think it's a nice guideline to expose the properties of cluster objects as they are in the UI as much as possible. E.g. there might be some confusion comparing the UI output to kubectl get cluster -oyaml and realising the Statuses don't align.

Copy link
Collaborator

@foot foot left a comment

Choose a reason for hiding this comment

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

Nice one! Very happy to merge as is for now, its strange that its isn't in the status anyway 🤷 , maybe it will appear there one day.

image

@makkes makkes merged commit aae28c5 into main Jul 5, 2022
@makkes makkes deleted the set-cp-initialized branch July 5, 2022 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants