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. Also adds flag to the API to allow checking status.observedGeneration==metadata.generation before executing probes.

Signed-off-by: Daniel Franz <dfranz@redhat.com>

Author: dtfranz

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.

internal/operator-controller/applier/boxcutter.go

@@ -14,6 +14,9 @@ import (
 
 	"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 +211,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 +607,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: "acme.cert-manager.io",
+				Kind:  "Certificate",
+			})).
+		WithAssertions(readyConditionAssertion)
+	issuerProbe := ocv1ac.ProgressionProbe().
+		WithSelector(ocv1ac.ObjectSelector().
+			WithType(ocv1.SelectorTypeGroupKind).
+			WithGroupKind(metav1.GroupKind{
+				Group: "acme.cert-manager.io",
+				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

internal/operator-controller/applier/boxcutter_test.go

@@ -18,6 +18,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 +144,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: "acme.cert-manager.io",
+							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: "acme.cert-manager.io",
+							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)

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 {