Skip to content

Commit 789df82

Browse files
dtfranzdtfranz
authored
Default Probes Refactor (#2586)
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 <dfranz@redhat.com>
1 parent 1dae77d commit 789df82

5 files changed

Lines changed: 147 additions & 94 deletions

File tree

‎api/v1/clusterextensionrevision_types.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -227,8 +227,8 @@ const (
227227
type ProbeType string
228228

229229
const (
230-
ProbeTypeFieldCondition ProbeType = "ConditionEqual"
231-
ProbeTypeFieldEqual ProbeType = "FieldsEqual"
230+
ProbeTypeConditionEqual ProbeType = "ConditionEqual"
231+
ProbeTypeFieldsEqual ProbeType = "FieldsEqual"
232232
ProbeTypeFieldValue ProbeType = "FieldValue"
233233
)
234234

‎internal/operator-controller/applier/boxcutter.go‎

Lines changed: 109 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,12 @@ import (
1212
"slices"
1313
"strings"
1414

15+
"github.com/cert-manager/cert-manager/pkg/apis/certmanager"
1516
"helm.sh/helm/v3/pkg/release"
1617
"helm.sh/helm/v3/pkg/storage/driver"
18+
appsv1 "k8s.io/api/apps/v1"
19+
corev1 "k8s.io/api/core/v1"
20+
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
1721
apierrors "k8s.io/apimachinery/pkg/api/errors"
1822
"k8s.io/apimachinery/pkg/api/meta"
1923
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -233,7 +237,8 @@ func (r *SimpleRevisionGenerator) buildClusterExtensionRevision(
233237

234238
spec := ocv1ac.ClusterExtensionRevisionSpec().
235239
WithLifecycleState(ocv1.ClusterExtensionRevisionLifecycleStateActive).
236-
WithPhases(phases...)
240+
WithPhases(phases...).
241+
WithProgressionProbes(defaultProgressionProbes...)
237242
if p := ext.Spec.ProgressDeadlineMinutes; p > 0 {
238243
spec.WithProgressDeadlineMinutes(p)
239244
}
@@ -624,6 +629,109 @@ func latestRevisionNumber(prevRevisions []ocv1.ClusterExtensionRevision) int64 {
624629
return prevRevisions[len(prevRevisions)-1].Spec.Revision
625630
}
626631

632+
var (
633+
// defaultProgressionProbes is the default set of progression probes used to check for phase readiness
634+
defaultProgressionProbes = []*ocv1ac.ProgressionProbeApplyConfiguration{
635+
// CRD probe
636+
ocv1ac.ProgressionProbe().
637+
WithSelector(ocv1ac.ObjectSelector().
638+
WithType(ocv1.SelectorTypeGroupKind).
639+
WithGroupKind(metav1.GroupKind{
640+
Group: "apiextensions.k8s.io",
641+
Kind: "CustomResourceDefinition",
642+
})).
643+
WithAssertions(ocv1ac.Assertion().
644+
WithType(ocv1.ProbeTypeConditionEqual).
645+
WithConditionEqual(
646+
ocv1ac.ConditionEqualProbe().
647+
WithType(string(apiextensions.Established)).
648+
WithStatus(string(corev1.ConditionTrue)))),
649+
// certmanager Certificate probe
650+
ocv1ac.ProgressionProbe().
651+
WithSelector(ocv1ac.ObjectSelector().
652+
WithType(ocv1.SelectorTypeGroupKind).
653+
WithGroupKind(metav1.GroupKind{
654+
Group: certmanager.GroupName,
655+
Kind: "Certificate",
656+
})).
657+
WithAssertions(readyConditionAssertion),
658+
// certmanager Issuer probe
659+
ocv1ac.ProgressionProbe().
660+
WithSelector(ocv1ac.ObjectSelector().
661+
WithType(ocv1.SelectorTypeGroupKind).
662+
WithGroupKind(metav1.GroupKind{
663+
Group: certmanager.GroupName,
664+
Kind: "Issuer",
665+
})).
666+
WithAssertions(readyConditionAssertion),
667+
// namespace probe; asserts that the namespace is in "Active" phase
668+
ocv1ac.ProgressionProbe().
669+
WithSelector(ocv1ac.ObjectSelector().
670+
WithType(ocv1.SelectorTypeGroupKind).
671+
WithGroupKind(metav1.GroupKind{
672+
Group: corev1.GroupName,
673+
Kind: "Namespace",
674+
})).
675+
WithAssertions(ocv1ac.Assertion().
676+
WithType(ocv1.ProbeTypeFieldValue).
677+
WithFieldValue(ocv1ac.FieldValueProbe().
678+
WithFieldPath("status.phase").
679+
WithValue(string(corev1.NamespaceActive)))),
680+
// PVC probe; asserts that the PVC is in "Bound" phase
681+
ocv1ac.ProgressionProbe().
682+
WithSelector(ocv1ac.ObjectSelector().
683+
WithType(ocv1.SelectorTypeGroupKind).
684+
WithGroupKind(metav1.GroupKind{
685+
Group: corev1.GroupName,
686+
Kind: "PersistentVolumeClaim",
687+
})).
688+
WithAssertions(ocv1ac.Assertion().
689+
WithType(ocv1.ProbeTypeFieldValue).
690+
WithFieldValue(ocv1ac.FieldValueProbe().
691+
WithFieldPath("status.phase").
692+
WithValue(string(corev1.ClaimBound)))),
693+
// StatefulSet probe
694+
ocv1ac.ProgressionProbe().WithSelector(
695+
ocv1ac.ObjectSelector().WithType(ocv1.SelectorTypeGroupKind).
696+
WithGroupKind(metav1.GroupKind{
697+
Group: appsv1.GroupName,
698+
Kind: "StatefulSet",
699+
}),
700+
).WithAssertions(replicasUpdatedAssertion, availableConditionAssertion),
701+
// Deployment probe
702+
ocv1ac.ProgressionProbe().WithSelector(
703+
ocv1ac.ObjectSelector().WithType(ocv1.SelectorTypeGroupKind).
704+
WithGroupKind(metav1.GroupKind{
705+
Group: appsv1.GroupName,
706+
Kind: "Deployment",
707+
}),
708+
).WithAssertions(replicasUpdatedAssertion, availableConditionAssertion),
709+
}
710+
711+
// readyConditionAssertion checks that the Type: "Ready" Condition is "True"
712+
readyConditionAssertion = ocv1ac.Assertion().
713+
WithType(ocv1.ProbeTypeConditionEqual).
714+
WithConditionEqual(
715+
ocv1ac.ConditionEqualProbe().
716+
WithType("Ready").
717+
WithStatus("True"))
718+
719+
// availableConditionAssertion checks if the Type: "Available" Condition is "True".
720+
availableConditionAssertion = ocv1ac.Assertion().
721+
WithType(ocv1.ProbeTypeConditionEqual).
722+
WithConditionEqual(ocv1ac.ConditionEqualProbe().
723+
WithType(string(appsv1.DeploymentAvailable)).
724+
WithStatus(string(corev1.ConditionTrue)))
725+
726+
// replicasUpdatedAssertion checks if status.updatedReplicas == status.replicas.
727+
// Works for StatefulSets, Deployments and ReplicaSets.
728+
replicasUpdatedAssertion = ocv1ac.Assertion().
729+
WithType(ocv1.ProbeTypeFieldsEqual).
730+
WithFieldsEqual(ocv1ac.FieldsEqualProbe().
731+
WithFieldA("status.updatedReplicas").
732+
WithFieldB("status.replicas"))
733+
)
734+
627735
func splitManifestDocuments(file string) []string {
628736
// Estimate: typical manifests have ~50-100 lines per document
629737
// Pre-allocate for reasonable bundle size to reduce allocations

‎internal/operator-controller/applier/boxcutter_test.go‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -151,10 +151,15 @@ func Test_SimpleRevisionGenerator_GenerateRevisionFromHelmRelease(t *testing.T)
151151
},
152152
},
153153
}),
154-
),
155-
),
154+
)),
156155
)
157-
assert.Equal(t, expected, rev)
156+
assert.Equal(t, expected.Name, rev.Name)
157+
assert.Equal(t, expected.Labels, rev.Labels)
158+
assert.Equal(t, expected.Annotations, rev.Annotations)
159+
assert.Equal(t, expected.Spec.LifecycleState, rev.Spec.LifecycleState)
160+
assert.Equal(t, expected.Spec.CollisionProtection, rev.Spec.CollisionProtection)
161+
assert.Equal(t, expected.Spec.Revision, rev.Spec.Revision)
162+
assert.Equal(t, expected.Spec.Phases, rev.Spec.Phases)
158163
}
159164

160165
func Test_SimpleRevisionGenerator_GenerateRevision(t *testing.T) {

‎internal/operator-controller/controllers/clusterextensionrevision_controller.go‎

Lines changed: 6 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,6 @@ import (
1010
"strings"
1111
"time"
1212

13-
appsv1 "k8s.io/api/apps/v1"
14-
corev1 "k8s.io/api/core/v1"
15-
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
1613
"k8s.io/apimachinery/pkg/api/equality"
1714
"k8s.io/apimachinery/pkg/api/meta"
1815
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -479,9 +476,7 @@ func (c *ClusterExtensionRevisionReconciler) buildBoxcutterPhases(ctx context.Co
479476

480477
opts := []boxcutter.RevisionReconcileOption{
481478
boxcutter.WithPreviousOwners(previousObjs),
482-
boxcutter.WithProbe(boxcutter.ProgressProbeType, probing.And{
483-
&namespaceActiveProbe, deploymentProbe, statefulSetProbe, crdProbe, issuerProbe, certProbe, &pvcBoundProbe, progressionProbes,
484-
}),
479+
boxcutter.WithProbe(boxcutter.ProgressProbeType, progressionProbes),
485480
}
486481

487482
phases := make([]boxcutter.Phase, 0)
@@ -535,10 +530,10 @@ func buildProgressionProbes(progressionProbes []ocv1.ProgressionProbe) (probing.
535530
for _, probe := range progressionProbe.Assertions {
536531
switch probe.Type {
537532
// Switch based on the union discriminator
538-
case ocv1.ProbeTypeFieldCondition:
533+
case ocv1.ProbeTypeConditionEqual:
539534
conditionProbe := probing.ConditionProbe(probe.ConditionEqual)
540535
assertions = append(assertions, &conditionProbe)
541-
case ocv1.ProbeTypeFieldEqual:
536+
case ocv1.ProbeTypeFieldsEqual:
542537
fieldsEqualProbe := probing.FieldsEqualProbe(probe.FieldsEqual)
543538
assertions = append(assertions, &fieldsEqualProbe)
544539
case ocv1.ProbeTypeFieldValue:
@@ -570,90 +565,13 @@ func buildProgressionProbes(progressionProbes []ocv1.ProgressionProbe) (probing.
570565
default:
571566
return nil, fmt.Errorf("unknown progressionProbe selector type: %s", progressionProbe.Selector.Type)
572567
}
573-
userProbes = append(userProbes, selectorProbe)
568+
userProbes = append(userProbes, &probing.ObservedGenerationProbe{
569+
Prober: selectorProbe,
570+
})
574571
}
575572
return userProbes, nil
576573
}
577574

578-
var (
579-
deploymentProbe = &probing.GroupKindSelector{
580-
GroupKind: schema.GroupKind{Group: appsv1.GroupName, Kind: "Deployment"},
581-
Prober: deplStatefulSetProbe,
582-
}
583-
statefulSetProbe = &probing.GroupKindSelector{
584-
GroupKind: schema.GroupKind{Group: appsv1.GroupName, Kind: "StatefulSet"},
585-
Prober: deplStatefulSetProbe,
586-
}
587-
crdProbe = &probing.GroupKindSelector{
588-
GroupKind: schema.GroupKind{Group: "apiextensions.k8s.io", Kind: "CustomResourceDefinition"},
589-
Prober: &probing.ObservedGenerationProbe{
590-
Prober: &probing.ConditionProbe{ // "Available" == "True"
591-
Type: string(apiextensions.Established),
592-
Status: string(corev1.ConditionTrue),
593-
},
594-
},
595-
}
596-
certProbe = &probing.GroupKindSelector{
597-
GroupKind: schema.GroupKind{Group: "acme.cert-manager.io", Kind: "Certificate"},
598-
Prober: &probing.ObservedGenerationProbe{
599-
Prober: readyConditionProbe,
600-
},
601-
}
602-
issuerProbe = &probing.GroupKindSelector{
603-
GroupKind: schema.GroupKind{Group: "acme.cert-manager.io", Kind: "Issuer"},
604-
Prober: &probing.ObservedGenerationProbe{
605-
Prober: readyConditionProbe,
606-
},
607-
}
608-
609-
// namespaceActiveProbe is a probe which asserts that the namespace is in "Active" phase
610-
namespaceActiveProbe = probing.GroupKindSelector{
611-
GroupKind: schema.GroupKind{Group: corev1.GroupName, Kind: "Namespace"},
612-
Prober: &applier.FieldValueProbe{
613-
FieldPath: "status.phase",
614-
Value: "Active",
615-
},
616-
}
617-
618-
// pvcBoundProbe is a probe which asserts that the PVC is in "Bound" phase
619-
pvcBoundProbe = probing.GroupKindSelector{
620-
GroupKind: schema.GroupKind{Group: corev1.GroupName, Kind: "PersistentVolumeClaim"},
621-
Prober: &applier.FieldValueProbe{
622-
FieldPath: "status.phase",
623-
Value: "Bound",
624-
},
625-
}
626-
627-
// deplStaefulSetProbe probes Deployment, StatefulSet objects.
628-
deplStatefulSetProbe = &probing.ObservedGenerationProbe{
629-
Prober: probing.And{
630-
availableConditionProbe,
631-
replicasUpdatedProbe,
632-
},
633-
}
634-
635-
// Checks if the Type: "Available" Condition is "True".
636-
availableConditionProbe = &probing.ConditionProbe{ // "Available" == "True"
637-
Type: string(appsv1.DeploymentAvailable),
638-
Status: string(corev1.ConditionTrue),
639-
}
640-
641-
// Checks if the Type: "Ready" Condition is "True"
642-
readyConditionProbe = &probing.ObservedGenerationProbe{
643-
Prober: &probing.ConditionProbe{
644-
Type: "Ready",
645-
Status: "True",
646-
},
647-
}
648-
649-
// Checks if .status.updatedReplicas == .status.replicas.
650-
// Works for StatefulSts, Deployments and ReplicaSets.
651-
replicasUpdatedProbe = &probing.FieldsEqualProbe{
652-
FieldA: ".status.updatedReplicas",
653-
FieldB: ".status.replicas",
654-
}
655-
)
656-
657575
func setRetryingConditions(cer *ocv1.ClusterExtensionRevision, message string) {
658576
markAsProgressing(cer, ocv1.ClusterExtensionRevisionReasonRetrying, message)
659577
if meta.FindStatusCondition(cer.Status.Conditions, ocv1.ClusterExtensionRevisionTypeAvailable) != nil {

‎test/e2e/features/revision.feature‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,17 @@ Feature: Install ClusterExtensionRevision
2020
spec:
2121
lifecycleState: Active
2222
collisionProtection: Prevent
23+
progressionProbes:
24+
- selector:
25+
type: GroupKind
26+
groupKind:
27+
group: ""
28+
kind: PersistentVolumeClaim
29+
assertions:
30+
- type: FieldValue
31+
fieldValue:
32+
fieldPath: "status.phase"
33+
value: "Bound"
2334
phases:
2435
- name: pvc
2536
objects:
@@ -72,6 +83,17 @@ Feature: Install ClusterExtensionRevision
7283
spec:
7384
lifecycleState: Active
7485
collisionProtection: Prevent
86+
progressionProbes:
87+
- selector:
88+
type: GroupKind
89+
groupKind:
90+
group: ""
91+
kind: PersistentVolumeClaim
92+
assertions:
93+
- type: FieldValue
94+
fieldValue:
95+
fieldPath: "status.phase"
96+
value: "Bound"
7597
phases:
7698
- name: pvc
7799
objects:

0 commit comments

Comments
 (0)