K8SPXC-1835 | status condition improvements#2480
Draft
mayankshah1607 wants to merge 6 commits into
Draft
Conversation
Signed-off-by: Mayank Shah <mayank.shah@percona.com>
Signed-off-by: Mayank Shah <mayank.shah@percona.com>
Signed-off-by: Mayank Shah <mayank.shah@percona.com>
Signed-off-by: Mayank Shah <mayank.shah@percona.com>
Signed-off-by: Mayank Shah <mayank.shah@percona.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR revamps PerconaXtraDBCluster status conditions by migrating from the operator’s legacy ClusterCondition/annotation-based tracking to Kubernetes-native metav1.Condition, and introduces new condition types (TLS enabled, reconcile error, component readiness, paused, volume resizing) to improve status signaling.
Changes:
- Migrate
.status.conditionsto[]metav1.Conditionand update status-writing logic to set/remove the new condition types. - Replace PVC resize progress tracking from CR annotations to a
VolumeResizingstatus condition (and adjust related logic/tests). - Add legacy-condition cleanup/normalization paths (notably for the old
"tls"condition) and update CRD schemas accordingly.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/naming/condition.go | Updates TLS condition state helper to return metav1.ConditionStatus. |
| pkg/controller/pxc/volumes.go | Switches PVC resize in-progress tracking from annotations to a status condition. |
| pkg/controller/pxc/volumes_test.go | Updates tests to use status conditions for resize in-progress setup/verification. |
| pkg/controller/pxc/tls.go | Adds legacy TLS condition normalization and migrates TLS condition handling to metav1.Condition. |
| pkg/controller/pxc/status.go | Reworks status condition updates to use new condition types and meta.Set/RemoveStatusCondition. |
| pkg/apis/pxc/v1/zz_generated.deepcopy.go | Regenerates deepcopy for []metav1.Condition status field. |
| pkg/apis/pxc/v1/pxc_types.go | Adds new condition type constants; changes status conditions field type; removes legacy condition helpers; updates PVC resize detection. |
| deploy/cw-bundle.yaml | Updates CRD schema for metav1.Condition fields/validation. |
| deploy/crd.yaml | Updates CRD schema for metav1.Condition fields/validation. |
| deploy/bundle.yaml | Updates CRD schema for metav1.Condition fields/validation. |
| config/crd/bases/pxc.percona.com_perconaxtradbclusters.yaml | Updates CRD base schema for metav1.Condition fields/validation. |
Files not reviewed (1)
- pkg/apis/pxc/v1/zz_generated.deepcopy.go: Language not supported
Comments suppressed due to low confidence (1)
pkg/controller/pxc/status.go:72
updateStatusalways removesConditionErrorReconcile, but only re-adds it whencr.Status.Status != AppStateError. If reconciles keep failing after the first error, the condition gets cleared even thoughreconcileErr != nil, losing the error condition and making status inconsistent. Consider settingConditionErrorReconcile=Trueunconditionally wheneverreconcileErr != nil(updating message/reason as needed), or at least avoid removing it in the error path.
func (r *ReconcilePerconaXtraDBCluster) updateStatus(ctx context.Context, cr *api.PerconaXtraDBCluster, inProgress bool, reconcileErr error) (err error) {
cleanupLegacyConditions(cr)
meta.RemoveStatusCondition(&cr.Status.Conditions, api.ConditionErrorReconcile)
if reconcileErr != nil {
if cr.Status.Status != api.AppStateError {
meta.SetStatusCondition(&cr.Status.Conditions, metav1.Condition{
Type: api.ConditionErrorReconcile,
Status: metav1.ConditionTrue,
Reason: "Error",
Message: reconcileErr.Error(),
})
cr.Status.Messages = append(cr.Status.Messages, "Error: "+reconcileErr.Error())
cr.Status.Status = api.AppStateError
}
return r.writeStatus(ctx, cr)
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+489
to
+506
| func normalizeLegacyTLSCondition(cr *api.PerconaXtraDBCluster) { | ||
| for i, cond := range cr.Status.Conditions { | ||
| if cond.Type == "tls" { | ||
| updated := metav1.Condition{ | ||
| Type: api.ConditionTLSEnabled, | ||
| LastTransitionTime: cond.LastTransitionTime, | ||
| Status: metav1.ConditionFalse, | ||
| Message: "ReconcileTLSToggle", | ||
| Reason: "ReconcileTLSToggle", | ||
| } | ||
| if cond.Status == "enabled" { | ||
| updated.Status = metav1.ConditionTrue | ||
| } | ||
| cr.Status.Conditions[i] = updated | ||
| return | ||
| } | ||
| } | ||
| } |
| Status: metav1.ConditionTrue, | ||
| Reason: "VolumeResizing", | ||
| Message: "Volume resizing in progress", | ||
| }) |
Collaborator
commit: 98f4e09 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CHANGE DESCRIPTION
Revamps status conditions and adds some new condition types:
CHECKLIST
Jira
Needs Doc) and QA (Needs QA)?Tests
compare/*-oc.yml)?Config/Logging/Testability