Skip to content

Fix ready and reconcile status during upgrade#439

Merged
leochr merged 2 commits intoapplication-stacks:mainfrom
smcclem:issue326
Nov 29, 2022
Merged

Fix ready and reconcile status during upgrade#439
leochr merged 2 commits intoapplication-stacks:mainfrom
smcclem:issue326

Conversation

@smcclem
Copy link
Copy Markdown
Collaborator

@smcclem smcclem commented Nov 29, 2022

What this PR does / why we need it?:

When upgrading, and the Reconciled and Ready field are already set (possibly to true), and the ResourcesReady
condition becomes false again, set the Ready and Reconciled fields to reflect the overall new state.

Does this PR introduce a user-facing change?

  • User guide
  • CHANGELOG.md

Which issue(s) this PR fixes:

Fixes #

Copy link
Copy Markdown
Member

@leochr leochr left a comment

Choose a reason for hiding this comment

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

@smcclem Looks good. Added minor comments about the message. Thanks.

Comment thread utils/reconciler.go Outdated
// conditions for Ready and Reconciled are set to false
readyNewCondition := s.NewCondition(common.StatusConditionTypeReady)
readyNewCondition.SetStatus(corev1.ConditionFalse)
readyNewCondition.SetMessage("Resources not ready.")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggestion: "Resources are not ready."

Comment thread utils/reconciler.go Outdated
s.SetCondition(readyNewCondition)
reconciledNewCondition := s.NewCondition(common.StatusConditionTypeReconciled)
reconciledNewCondition.SetStatus(corev1.ConditionFalse)
reconciledNewCondition.SetMessage("Resources not ready.")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggestion: "Resources are not ready."

@leochr leochr merged commit 5566d80 into application-stacks:main Nov 29, 2022
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