From 1756aafe44e94c67fadda8cc42f12d2e182c268e 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 | 9 +- .../operator-controller/applier/boxcutter.go | 112 +++++++++++++++++- .../applier/boxcutter_test.go | 102 +++++++++++++++- .../clusterextensionrevision_controller.go | 94 +-------------- test/e2e/features/revision.feature | 22 ++++ 5 files changed, 247 insertions(+), 92 deletions(-) diff --git a/api/v1/clusterextensionrevision_types.go b/api/v1/clusterextensionrevision_types.go index 208d96d9f..872389422 100644 --- a/api/v1/clusterextensionrevision_types.go +++ b/api/v1/clusterextensionrevision_types.go @@ -227,9 +227,14 @@ const ( type ProbeType string const ( - ProbeTypeFieldCondition ProbeType = "ConditionEqual" - ProbeTypeFieldEqual ProbeType = "FieldsEqual" + ProbeTypeConditionEqual ProbeType = "ConditionEqual" + ProbeTypeFieldsEqual ProbeType = "FieldsEqual" ProbeTypeFieldValue ProbeType = "FieldValue" + + // Deprecated: use ProbeTypeConditionEqual instead. + ProbeTypeFieldCondition = ProbeTypeConditionEqual + // Deprecated: use ProbeTypeFieldsEqual instead. + ProbeTypeFieldEqual = ProbeTypeFieldsEqual ) // Assertion is a discriminated union which defines the probe type and definition used as an assertion. diff --git a/internal/operator-controller/applier/boxcutter.go b/internal/operator-controller/applier/boxcutter.go index cb4da7e53..b46c3f330 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" @@ -208,10 +212,12 @@ func (r *SimpleRevisionGenerator) buildClusterExtensionRevision( annotations[labels.ServiceAccountNamespaceKey] = ext.Spec.Namespace phases := PhaseSort(objects) + progressionProbes := defaultProgressionProbes() spec := ocv1ac.ClusterExtensionRevisionSpec(). WithLifecycleState(ocv1.ClusterExtensionRevisionLifecycleStateActive). - WithPhases(phases...) + WithPhases(phases...). + WithProgressionProbes(progressionProbes...) if p := ext.Spec.ProgressDeadlineMinutes; p > 0 { spec.WithProgressDeadlineMinutes(p) } @@ -602,6 +608,110 @@ func latestRevisionNumber(prevRevisions []ocv1.ClusterExtensionRevision) int64 { return prevRevisions[len(prevRevisions)-1].Spec.Revision } +func defaultProgressionProbes() []*ocv1ac.ProgressionProbeApplyConfiguration { + crdProbe := 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)))) + + // Checks if the Type: "Ready" Condition is "True" + readyConditionAssertion := ocv1ac.Assertion(). + WithType(ocv1.ProbeTypeConditionEqual). + WithConditionEqual( + ocv1ac.ConditionEqualProbe(). + WithType("Ready"). + WithStatus("True")) + + certProbe := ocv1ac.ProgressionProbe(). + WithSelector(ocv1ac.ObjectSelector(). + WithType(ocv1.SelectorTypeGroupKind). + WithGroupKind(metav1.GroupKind{ + Group: certmanager.GroupName, + Kind: "Certificate", + })). + WithAssertions(readyConditionAssertion) + issuerProbe := ocv1ac.ProgressionProbe(). + WithSelector(ocv1ac.ObjectSelector(). + WithType(ocv1.SelectorTypeGroupKind). + WithGroupKind(metav1.GroupKind{ + Group: certmanager.GroupName, + Kind: "Issuer", + })). + WithAssertions(readyConditionAssertion) + + // namespaceActiveProbe is a probe which asserts that the namespace is in "Active" phase + namespaceActiveProbe := 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)))) + + // pvcBoundProbe is a probe which asserts that the PVC is in "Bound" phase + pvcBoundProbe := 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)))) + + // 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))) + + // 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")) + + statefulSetProbe := ocv1ac.ProgressionProbe().WithSelector( + ocv1ac.ObjectSelector().WithType(ocv1.SelectorTypeGroupKind). + WithGroupKind(metav1.GroupKind{ + Group: appsv1.GroupName, + Kind: "StatefulSet", + }), + ).WithAssertions(replicasUpdatedAssertion, availableConditionAssertion) + + deploymentProbe := ocv1ac.ProgressionProbe().WithSelector( + ocv1ac.ObjectSelector().WithType(ocv1.SelectorTypeGroupKind). + WithGroupKind(metav1.GroupKind{ + Group: appsv1.GroupName, + Kind: "Deployment", + }), + ).WithAssertions(replicasUpdatedAssertion, availableConditionAssertion) + + return []*ocv1ac.ProgressionProbeApplyConfiguration{ + deploymentProbe, statefulSetProbe, pvcBoundProbe, namespaceActiveProbe, issuerProbe, certProbe, crdProbe, + } +} + 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 4f8461250..30ef3354f 100644 --- a/internal/operator-controller/applier/boxcutter_test.go +++ b/internal/operator-controller/applier/boxcutter_test.go @@ -10,6 +10,7 @@ import ( "testing" "testing/fstest" + "github.com/cert-manager/cert-manager/pkg/apis/certmanager" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -18,6 +19,7 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" + "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" apierrors "k8s.io/apimachinery/pkg/api/errors" apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -143,7 +145,105 @@ func Test_SimpleRevisionGenerator_GenerateRevisionFromHelmRelease(t *testing.T) }, }, }), - ), + )). + WithProgressionProbes( + ocv1ac.ProgressionProbe().WithSelector( + ocv1ac.ObjectSelector().WithType(ocv1.SelectorTypeGroupKind). + WithGroupKind(metav1.GroupKind{ + Group: appsv1.GroupName, + Kind: "Deployment", + })). + WithAssertions( + ocv1ac.Assertion(). + WithType(ocv1.ProbeTypeFieldsEqual). + WithFieldsEqual(ocv1ac.FieldsEqualProbe(). + WithFieldA("status.updatedReplicas"). + WithFieldB("status.replicas")), + ocv1ac.Assertion(). + WithType(ocv1.ProbeTypeConditionEqual). + WithConditionEqual(ocv1ac.ConditionEqualProbe(). + WithType(string(appsv1.DeploymentAvailable)). + WithStatus(string(corev1.ConditionTrue)))), + ocv1ac.ProgressionProbe().WithSelector( + ocv1ac.ObjectSelector().WithType(ocv1.SelectorTypeGroupKind). + WithGroupKind(metav1.GroupKind{ + Group: appsv1.GroupName, + Kind: "StatefulSet", + })). + WithAssertions( + ocv1ac.Assertion(). + WithType(ocv1.ProbeTypeFieldsEqual). + WithFieldsEqual(ocv1ac.FieldsEqualProbe(). + WithFieldA("status.updatedReplicas"). + WithFieldB("status.replicas")), + ocv1ac.Assertion(). + WithType(ocv1.ProbeTypeConditionEqual). + WithConditionEqual(ocv1ac.ConditionEqualProbe(). + WithType(string(appsv1.DeploymentAvailable)). + WithStatus(string(corev1.ConditionTrue)))), + 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)))), + 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)))), + ocv1ac.ProgressionProbe(). + WithSelector(ocv1ac.ObjectSelector(). + WithType(ocv1.SelectorTypeGroupKind). + WithGroupKind(metav1.GroupKind{ + Group: certmanager.GroupName, + Kind: "Issuer", + })). + WithAssertions(ocv1ac.Assertion(). + WithType(ocv1.ProbeTypeConditionEqual). + WithConditionEqual( + ocv1ac.ConditionEqualProbe(). + WithType("Ready"). + WithStatus("True"))), + ocv1ac.ProgressionProbe(). + WithSelector(ocv1ac.ObjectSelector(). + WithType(ocv1.SelectorTypeGroupKind). + WithGroupKind(metav1.GroupKind{ + Group: certmanager.GroupName, + Kind: "Certificate", + })). + WithAssertions(ocv1ac.Assertion(). + WithType(ocv1.ProbeTypeConditionEqual). + WithConditionEqual( + ocv1ac.ConditionEqualProbe(). + WithType("Ready"). + WithStatus("True"))), + 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)))), ), ) assert.Equal(t, expected, rev) diff --git a/internal/operator-controller/controllers/clusterextensionrevision_controller.go b/internal/operator-controller/controllers/clusterextensionrevision_controller.go index 2e9d2fce6..e3646bb63 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 755762a8a..f2c2c7fa2 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: