From 65624021ecd77dcb44804eeb6bc317e74ddd705e Mon Sep 17 00:00:00 2001 From: Daniel Franz Date: Mon, 23 Mar 2026 23:46:54 +0900 Subject: [PATCH] Default Probes Refactor Migrates the default progression probes out of the CER controller into the boxcutter applier in order to use the ProgressionProbes API to transparently stamp out the checks. All probes will now check that status.observedGeneration==metadata.generation before executing probes, or execute them normally if objects do not contain those fields. Signed-off-by: Daniel Franz --- api/v1/clusterextensionrevision_types.go | 4 +- .../operator-controller/applier/boxcutter.go | 110 +++++++++++++++++- .../applier/boxcutter_test.go | 11 +- .../clusterextensionrevision_controller.go | 94 +-------------- test/e2e/features/revision.feature | 22 ++++ 5 files changed, 147 insertions(+), 94 deletions(-) diff --git a/api/v1/clusterextensionrevision_types.go b/api/v1/clusterextensionrevision_types.go index 208d96d9ff..a4d389867a 100644 --- a/api/v1/clusterextensionrevision_types.go +++ b/api/v1/clusterextensionrevision_types.go @@ -227,8 +227,8 @@ const ( type ProbeType string const ( - ProbeTypeFieldCondition ProbeType = "ConditionEqual" - ProbeTypeFieldEqual ProbeType = "FieldsEqual" + ProbeTypeConditionEqual ProbeType = "ConditionEqual" + ProbeTypeFieldsEqual ProbeType = "FieldsEqual" ProbeTypeFieldValue ProbeType = "FieldValue" ) diff --git a/internal/operator-controller/applier/boxcutter.go b/internal/operator-controller/applier/boxcutter.go index cb4da7e534..d74c0e1f77 100644 --- a/internal/operator-controller/applier/boxcutter.go +++ b/internal/operator-controller/applier/boxcutter.go @@ -12,8 +12,12 @@ import ( "slices" "strings" + "github.com/cert-manager/cert-manager/pkg/apis/certmanager" "helm.sh/helm/v3/pkg/release" "helm.sh/helm/v3/pkg/storage/driver" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -211,7 +215,8 @@ func (r *SimpleRevisionGenerator) buildClusterExtensionRevision( spec := ocv1ac.ClusterExtensionRevisionSpec(). WithLifecycleState(ocv1.ClusterExtensionRevisionLifecycleStateActive). - WithPhases(phases...) + WithPhases(phases...). + WithProgressionProbes(defaultProgressionProbes...) if p := ext.Spec.ProgressDeadlineMinutes; p > 0 { spec.WithProgressDeadlineMinutes(p) } @@ -602,6 +607,109 @@ func latestRevisionNumber(prevRevisions []ocv1.ClusterExtensionRevision) int64 { return prevRevisions[len(prevRevisions)-1].Spec.Revision } +var ( + // defaultProgressionProbes is the default set of progression probes used to check for phase readiness + defaultProgressionProbes = []*ocv1ac.ProgressionProbeApplyConfiguration{ + // CRD probe + ocv1ac.ProgressionProbe(). + WithSelector(ocv1ac.ObjectSelector(). + WithType(ocv1.SelectorTypeGroupKind). + WithGroupKind(metav1.GroupKind{ + Group: "apiextensions.k8s.io", + Kind: "CustomResourceDefinition", + })). + WithAssertions(ocv1ac.Assertion(). + WithType(ocv1.ProbeTypeConditionEqual). + WithConditionEqual( + ocv1ac.ConditionEqualProbe(). + WithType(string(apiextensions.Established)). + WithStatus(string(corev1.ConditionTrue)))), + // certmanager Certificate probe + ocv1ac.ProgressionProbe(). + WithSelector(ocv1ac.ObjectSelector(). + WithType(ocv1.SelectorTypeGroupKind). + WithGroupKind(metav1.GroupKind{ + Group: certmanager.GroupName, + Kind: "Certificate", + })). + WithAssertions(readyConditionAssertion), + // certmanager Issuer probe + ocv1ac.ProgressionProbe(). + WithSelector(ocv1ac.ObjectSelector(). + WithType(ocv1.SelectorTypeGroupKind). + WithGroupKind(metav1.GroupKind{ + Group: certmanager.GroupName, + Kind: "Issuer", + })). + WithAssertions(readyConditionAssertion), + // namespace probe; asserts that the namespace is in "Active" phase + ocv1ac.ProgressionProbe(). + WithSelector(ocv1ac.ObjectSelector(). + WithType(ocv1.SelectorTypeGroupKind). + WithGroupKind(metav1.GroupKind{ + Group: corev1.GroupName, + Kind: "Namespace", + })). + WithAssertions(ocv1ac.Assertion(). + WithType(ocv1.ProbeTypeFieldValue). + WithFieldValue(ocv1ac.FieldValueProbe(). + WithFieldPath("status.phase"). + WithValue(string(corev1.NamespaceActive)))), + // PVC probe; asserts that the PVC is in "Bound" phase + ocv1ac.ProgressionProbe(). + WithSelector(ocv1ac.ObjectSelector(). + WithType(ocv1.SelectorTypeGroupKind). + WithGroupKind(metav1.GroupKind{ + Group: corev1.GroupName, + Kind: "PersistentVolumeClaim", + })). + WithAssertions(ocv1ac.Assertion(). + WithType(ocv1.ProbeTypeFieldValue). + WithFieldValue(ocv1ac.FieldValueProbe(). + WithFieldPath("status.phase"). + WithValue(string(corev1.ClaimBound)))), + // StatefulSet probe + ocv1ac.ProgressionProbe().WithSelector( + ocv1ac.ObjectSelector().WithType(ocv1.SelectorTypeGroupKind). + WithGroupKind(metav1.GroupKind{ + Group: appsv1.GroupName, + Kind: "StatefulSet", + }), + ).WithAssertions(replicasUpdatedAssertion, availableConditionAssertion), + // Deployment probe + ocv1ac.ProgressionProbe().WithSelector( + ocv1ac.ObjectSelector().WithType(ocv1.SelectorTypeGroupKind). + WithGroupKind(metav1.GroupKind{ + Group: appsv1.GroupName, + Kind: "Deployment", + }), + ).WithAssertions(replicasUpdatedAssertion, availableConditionAssertion), + } + + // readyConditionAssertion checks that the Type: "Ready" Condition is "True" + readyConditionAssertion = ocv1ac.Assertion(). + WithType(ocv1.ProbeTypeConditionEqual). + WithConditionEqual( + ocv1ac.ConditionEqualProbe(). + WithType("Ready"). + WithStatus("True")) + + // availableConditionAssertion checks if the Type: "Available" Condition is "True". + availableConditionAssertion = ocv1ac.Assertion(). + WithType(ocv1.ProbeTypeConditionEqual). + WithConditionEqual(ocv1ac.ConditionEqualProbe(). + WithType(string(appsv1.DeploymentAvailable)). + WithStatus(string(corev1.ConditionTrue))) + + // replicasUpdatedAssertion checks if status.updatedReplicas == status.replicas. + // Works for StatefulSets, Deployments and ReplicaSets. + replicasUpdatedAssertion = ocv1ac.Assertion(). + WithType(ocv1.ProbeTypeFieldsEqual). + WithFieldsEqual(ocv1ac.FieldsEqualProbe(). + WithFieldA("status.updatedReplicas"). + WithFieldB("status.replicas")) +) + func splitManifestDocuments(file string) []string { // Estimate: typical manifests have ~50-100 lines per document // Pre-allocate for reasonable bundle size to reduce allocations diff --git a/internal/operator-controller/applier/boxcutter_test.go b/internal/operator-controller/applier/boxcutter_test.go index 4f84612509..5d7fcf79bf 100644 --- a/internal/operator-controller/applier/boxcutter_test.go +++ b/internal/operator-controller/applier/boxcutter_test.go @@ -143,10 +143,15 @@ func Test_SimpleRevisionGenerator_GenerateRevisionFromHelmRelease(t *testing.T) }, }, }), - ), - ), + )), ) - assert.Equal(t, expected, rev) + assert.Equal(t, expected.Name, rev.Name) + assert.Equal(t, expected.Labels, rev.Labels) + assert.Equal(t, expected.Annotations, rev.Annotations) + assert.Equal(t, expected.Spec.LifecycleState, rev.Spec.LifecycleState) + assert.Equal(t, expected.Spec.CollisionProtection, rev.Spec.CollisionProtection) + assert.Equal(t, expected.Spec.Revision, rev.Spec.Revision) + assert.Equal(t, expected.Spec.Phases, rev.Spec.Phases) } func Test_SimpleRevisionGenerator_GenerateRevision(t *testing.T) { diff --git a/internal/operator-controller/controllers/clusterextensionrevision_controller.go b/internal/operator-controller/controllers/clusterextensionrevision_controller.go index 2e9d2fce65..e3646bb63e 100644 --- a/internal/operator-controller/controllers/clusterextensionrevision_controller.go +++ b/internal/operator-controller/controllers/clusterextensionrevision_controller.go @@ -10,9 +10,6 @@ import ( "strings" "time" - appsv1 "k8s.io/api/apps/v1" - corev1 "k8s.io/api/core/v1" - "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -479,9 +476,7 @@ func (c *ClusterExtensionRevisionReconciler) buildBoxcutterPhases(ctx context.Co opts := []boxcutter.RevisionReconcileOption{ boxcutter.WithPreviousOwners(previousObjs), - boxcutter.WithProbe(boxcutter.ProgressProbeType, probing.And{ - &namespaceActiveProbe, deploymentProbe, statefulSetProbe, crdProbe, issuerProbe, certProbe, &pvcBoundProbe, progressionProbes, - }), + boxcutter.WithProbe(boxcutter.ProgressProbeType, progressionProbes), } phases := make([]boxcutter.Phase, 0) @@ -535,10 +530,10 @@ func buildProgressionProbes(progressionProbes []ocv1.ProgressionProbe) (probing. for _, probe := range progressionProbe.Assertions { switch probe.Type { // Switch based on the union discriminator - case ocv1.ProbeTypeFieldCondition: + case ocv1.ProbeTypeConditionEqual: conditionProbe := probing.ConditionProbe(probe.ConditionEqual) assertions = append(assertions, &conditionProbe) - case ocv1.ProbeTypeFieldEqual: + case ocv1.ProbeTypeFieldsEqual: fieldsEqualProbe := probing.FieldsEqualProbe(probe.FieldsEqual) assertions = append(assertions, &fieldsEqualProbe) case ocv1.ProbeTypeFieldValue: @@ -570,90 +565,13 @@ func buildProgressionProbes(progressionProbes []ocv1.ProgressionProbe) (probing. default: return nil, fmt.Errorf("unknown progressionProbe selector type: %s", progressionProbe.Selector.Type) } - userProbes = append(userProbes, selectorProbe) + userProbes = append(userProbes, &probing.ObservedGenerationProbe{ + Prober: selectorProbe, + }) } return userProbes, nil } -var ( - deploymentProbe = &probing.GroupKindSelector{ - GroupKind: schema.GroupKind{Group: appsv1.GroupName, Kind: "Deployment"}, - Prober: deplStatefulSetProbe, - } - statefulSetProbe = &probing.GroupKindSelector{ - GroupKind: schema.GroupKind{Group: appsv1.GroupName, Kind: "StatefulSet"}, - Prober: deplStatefulSetProbe, - } - crdProbe = &probing.GroupKindSelector{ - GroupKind: schema.GroupKind{Group: "apiextensions.k8s.io", Kind: "CustomResourceDefinition"}, - Prober: &probing.ObservedGenerationProbe{ - Prober: &probing.ConditionProbe{ // "Available" == "True" - Type: string(apiextensions.Established), - Status: string(corev1.ConditionTrue), - }, - }, - } - certProbe = &probing.GroupKindSelector{ - GroupKind: schema.GroupKind{Group: "acme.cert-manager.io", Kind: "Certificate"}, - Prober: &probing.ObservedGenerationProbe{ - Prober: readyConditionProbe, - }, - } - issuerProbe = &probing.GroupKindSelector{ - GroupKind: schema.GroupKind{Group: "acme.cert-manager.io", Kind: "Issuer"}, - Prober: &probing.ObservedGenerationProbe{ - Prober: readyConditionProbe, - }, - } - - // namespaceActiveProbe is a probe which asserts that the namespace is in "Active" phase - namespaceActiveProbe = probing.GroupKindSelector{ - GroupKind: schema.GroupKind{Group: corev1.GroupName, Kind: "Namespace"}, - Prober: &applier.FieldValueProbe{ - FieldPath: "status.phase", - Value: "Active", - }, - } - - // pvcBoundProbe is a probe which asserts that the PVC is in "Bound" phase - pvcBoundProbe = probing.GroupKindSelector{ - GroupKind: schema.GroupKind{Group: corev1.GroupName, Kind: "PersistentVolumeClaim"}, - Prober: &applier.FieldValueProbe{ - FieldPath: "status.phase", - Value: "Bound", - }, - } - - // deplStaefulSetProbe probes Deployment, StatefulSet objects. - deplStatefulSetProbe = &probing.ObservedGenerationProbe{ - Prober: probing.And{ - availableConditionProbe, - replicasUpdatedProbe, - }, - } - - // Checks if the Type: "Available" Condition is "True". - availableConditionProbe = &probing.ConditionProbe{ // "Available" == "True" - Type: string(appsv1.DeploymentAvailable), - Status: string(corev1.ConditionTrue), - } - - // Checks if the Type: "Ready" Condition is "True" - readyConditionProbe = &probing.ObservedGenerationProbe{ - Prober: &probing.ConditionProbe{ - Type: "Ready", - Status: "True", - }, - } - - // Checks if .status.updatedReplicas == .status.replicas. - // Works for StatefulSts, Deployments and ReplicaSets. - replicasUpdatedProbe = &probing.FieldsEqualProbe{ - FieldA: ".status.updatedReplicas", - FieldB: ".status.replicas", - } -) - func setRetryingConditions(cer *ocv1.ClusterExtensionRevision, message string) { markAsProgressing(cer, ocv1.ClusterExtensionRevisionReasonRetrying, message) if meta.FindStatusCondition(cer.Status.Conditions, ocv1.ClusterExtensionRevisionTypeAvailable) != nil { diff --git a/test/e2e/features/revision.feature b/test/e2e/features/revision.feature index 755762a8a9..f2c2c7fa29 100644 --- a/test/e2e/features/revision.feature +++ b/test/e2e/features/revision.feature @@ -20,6 +20,17 @@ Feature: Install ClusterExtensionRevision spec: lifecycleState: Active collisionProtection: Prevent + progressionProbes: + - selector: + type: GroupKind + groupKind: + group: "" + kind: PersistentVolumeClaim + assertions: + - type: FieldValue + fieldValue: + fieldPath: "status.phase" + value: "Bound" phases: - name: pvc objects: @@ -72,6 +83,17 @@ Feature: Install ClusterExtensionRevision spec: lifecycleState: Active collisionProtection: Prevent + progressionProbes: + - selector: + type: GroupKind + groupKind: + group: "" + kind: PersistentVolumeClaim + assertions: + - type: FieldValue + fieldValue: + fieldPath: "status.phase" + value: "Bound" phases: - name: pvc objects: