Skip to content

Commit 6562402

Browse files
dtfranzdtfranz
committed
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 <dfranz@redhat.com>
1 parent a307a6d commit 6562402

File tree

5 files changed

+147
-94
lines changed

5 files changed

+147
-94
lines changed

‎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"
@@ -211,7 +215,8 @@ func (r *SimpleRevisionGenerator) buildClusterExtensionRevision(
211215

212216
spec := ocv1ac.ClusterExtensionRevisionSpec().
213217
WithLifecycleState(ocv1.ClusterExtensionRevisionLifecycleStateActive).
214-
WithPhases(phases...)
218+
WithPhases(phases...).
219+
WithProgressionProbes(defaultProgressionProbes...)
215220
if p := ext.Spec.ProgressDeadlineMinutes; p > 0 {
216221
spec.WithProgressDeadlineMinutes(p)
217222
}
@@ -602,6 +607,109 @@ func latestRevisionNumber(prevRevisions []ocv1.ClusterExtensionRevision) int64 {
602607
return prevRevisions[len(prevRevisions)-1].Spec.Revision
603608
}
604609

610+
var (
611+
// defaultProgressionProbes is the default set of progression probes used to check for phase readiness
612+
defaultProgressionProbes = []*ocv1ac.ProgressionProbeApplyConfiguration{
613+
// CRD probe
614+
ocv1ac.ProgressionProbe().
615+
WithSelector(ocv1ac.ObjectSelector().
616+
WithType(ocv1.SelectorTypeGroupKind).
617+
WithGroupKind(metav1.GroupKind{
618+
Group: "apiextensions.k8s.io",
619+
Kind: "CustomResourceDefinition",
620+
})).
621+
WithAssertions(ocv1ac.Assertion().
622+
WithType(ocv1.ProbeTypeConditionEqual).
623+
WithConditionEqual(
624+
ocv1ac.ConditionEqualProbe().
625+
WithType(string(apiextensions.Established)).
626+
WithStatus(string(corev1.ConditionTrue)))),
627+
// certmanager Certificate probe
628+
ocv1ac.ProgressionProbe().
629+
WithSelector(ocv1ac.ObjectSelector().
630+
WithType(ocv1.SelectorTypeGroupKind).
631+
WithGroupKind(metav1.GroupKind{
632+
Group: certmanager.GroupName,
633+
Kind: "Certificate",
634+
})).
635+
WithAssertions(readyConditionAssertion),
636+
// certmanager Issuer probe
637+
ocv1ac.ProgressionProbe().
638+
WithSelector(ocv1ac.ObjectSelector().
639+
WithType(ocv1.SelectorTypeGroupKind).
640+
WithGroupKind(metav1.GroupKind{
641+
Group: certmanager.GroupName,
642+
Kind: "Issuer",
643+
})).
644+
WithAssertions(readyConditionAssertion),
645+
// namespace probe; asserts that the namespace is in "Active" phase
646+
ocv1ac.ProgressionProbe().
647+
WithSelector(ocv1ac.ObjectSelector().
648+
WithType(ocv1.SelectorTypeGroupKind).
649+
WithGroupKind(metav1.GroupKind{
650+
Group: corev1.GroupName,
651+
Kind: "Namespace",
652+
})).
653+
WithAssertions(ocv1ac.Assertion().
654+
WithType(ocv1.ProbeTypeFieldValue).
655+
WithFieldValue(ocv1ac.FieldValueProbe().
656+
WithFieldPath("status.phase").
657+
WithValue(string(corev1.NamespaceActive)))),
658+
// PVC probe; asserts that the PVC is in "Bound" phase
659+
ocv1ac.ProgressionProbe().
660+
WithSelector(ocv1ac.ObjectSelector().
661+
WithType(ocv1.SelectorTypeGroupKind).
662+
WithGroupKind(metav1.GroupKind{
663+
Group: corev1.GroupName,
664+
Kind: "PersistentVolumeClaim",
665+
})).
666+
WithAssertions(ocv1ac.Assertion().
667+
WithType(ocv1.ProbeTypeFieldValue).
668+
WithFieldValue(ocv1ac.FieldValueProbe().
669+
WithFieldPath("status.phase").
670+
WithValue(string(corev1.ClaimBound)))),
671+
// StatefulSet probe
672+
ocv1ac.ProgressionProbe().WithSelector(
673+
ocv1ac.ObjectSelector().WithType(ocv1.SelectorTypeGroupKind).
674+
WithGroupKind(metav1.GroupKind{
675+
Group: appsv1.GroupName,
676+
Kind: "StatefulSet",
677+
}),
678+
).WithAssertions(replicasUpdatedAssertion, availableConditionAssertion),
679+
// Deployment probe
680+
ocv1ac.ProgressionProbe().WithSelector(
681+
ocv1ac.ObjectSelector().WithType(ocv1.SelectorTypeGroupKind).
682+
WithGroupKind(metav1.GroupKind{
683+
Group: appsv1.GroupName,
684+
Kind: "Deployment",
685+
}),
686+
).WithAssertions(replicasUpdatedAssertion, availableConditionAssertion),
687+
}
688+
689+
// readyConditionAssertion checks that the Type: "Ready" Condition is "True"
690+
readyConditionAssertion = ocv1ac.Assertion().
691+
WithType(ocv1.ProbeTypeConditionEqual).
692+
WithConditionEqual(
693+
ocv1ac.ConditionEqualProbe().
694+
WithType("Ready").
695+
WithStatus("True"))
696+
697+
// availableConditionAssertion checks if the Type: "Available" Condition is "True".
698+
availableConditionAssertion = ocv1ac.Assertion().
699+
WithType(ocv1.ProbeTypeConditionEqual).
700+
WithConditionEqual(ocv1ac.ConditionEqualProbe().
701+
WithType(string(appsv1.DeploymentAvailable)).
702+
WithStatus(string(corev1.ConditionTrue)))
703+
704+
// replicasUpdatedAssertion checks if status.updatedReplicas == status.replicas.
705+
// Works for StatefulSets, Deployments and ReplicaSets.
706+
replicasUpdatedAssertion = ocv1ac.Assertion().
707+
WithType(ocv1.ProbeTypeFieldsEqual).
708+
WithFieldsEqual(ocv1ac.FieldsEqualProbe().
709+
WithFieldA("status.updatedReplicas").
710+
WithFieldB("status.replicas"))
711+
)
712+
605713
func splitManifestDocuments(file string) []string {
606714
// Estimate: typical manifests have ~50-100 lines per document
607715
// 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
@@ -143,10 +143,15 @@ func Test_SimpleRevisionGenerator_GenerateRevisionFromHelmRelease(t *testing.T)
143143
},
144144
},
145145
}),
146-
),
147-
),
146+
)),
148147
)
149-
assert.Equal(t, expected, rev)
148+
assert.Equal(t, expected.Name, rev.Name)
149+
assert.Equal(t, expected.Labels, rev.Labels)
150+
assert.Equal(t, expected.Annotations, rev.Annotations)
151+
assert.Equal(t, expected.Spec.LifecycleState, rev.Spec.LifecycleState)
152+
assert.Equal(t, expected.Spec.CollisionProtection, rev.Spec.CollisionProtection)
153+
assert.Equal(t, expected.Spec.Revision, rev.Spec.Revision)
154+
assert.Equal(t, expected.Spec.Phases, rev.Spec.Phases)
150155
}
151156

152157
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)