diff --git a/pkg/controller/constrainttemplate/constrainttemplate_controller.go b/pkg/controller/constrainttemplate/constrainttemplate_controller.go index cb99b7864f4..e4731d46a8b 100644 --- a/pkg/controller/constrainttemplate/constrainttemplate_controller.go +++ b/pkg/controller/constrainttemplate/constrainttemplate_controller.go @@ -324,7 +324,7 @@ func (r *ReconcileConstraintTemplate) Reconcile(ctx context.Context, request rec } unversionedProposedCRD, err := r.opa.CreateCRD(context.Background(), unversionedCT) if err != nil { - r.tracker.CancelTemplate(unversionedCT) // Don't track templates that failed compilation + r.tracker.TryCancelTemplate(unversionedCT) // Don't track templates that failed compilation r.metrics.registry.add(request.NamespacedName, metrics.ErrorStatus) var createErr *v1beta1.CreateCRDError if parseErrs, ok := err.(ast.Errors); ok { @@ -347,7 +347,7 @@ func (r *ReconcileConstraintTemplate) Reconcile(ctx context.Context, request rec proposedCRD := &apiextensionsv1beta1.CustomResourceDefinition{} if err := r.scheme.Convert(unversionedProposedCRD, proposedCRD, nil); err != nil { - r.tracker.CancelTemplate(unversionedCT) // Don't track templates that failed compilation + r.tracker.TryCancelTemplate(unversionedCT) // Don't track templates that failed compilation r.metrics.registry.add(request.NamespacedName, metrics.ErrorStatus) log.Error(err, "conversion error") logError(request.NamespacedName.Name) @@ -419,7 +419,7 @@ func (r *ReconcileConstraintTemplate) handleUpdate( log.Error(err, "failed to report constraint template ingestion duration") } err := r.reportErrorOnCTStatus("ingest_error", "Could not ingest Rego", status, err) - r.tracker.CancelTemplate(unversionedCT) // Don't track templates that failed compilation + r.tracker.TryCancelTemplate(unversionedCT) // Don't track templates that failed compilation return reconcile.Result{}, err } diff --git a/pkg/readiness/noop_expectations.go b/pkg/readiness/noop_expectations.go index 6b842bc1864..4285914c14a 100644 --- a/pkg/readiness/noop_expectations.go +++ b/pkg/readiness/noop_expectations.go @@ -24,7 +24,9 @@ func (n noopExpectations) Expect(o runtime.Object) {} func (n noopExpectations) CancelExpect(o runtime.Object) {} -func (n noopExpectations) TryCancelExpect(o runtime.Object) {} +func (n noopExpectations) TryCancelExpect(o runtime.Object) bool { + return false +} func (n noopExpectations) ExpectationsDone() {} diff --git a/pkg/readiness/object_tracker.go b/pkg/readiness/object_tracker.go index 2646ebab054..49dd5f55d3a 100644 --- a/pkg/readiness/object_tracker.go +++ b/pkg/readiness/object_tracker.go @@ -39,7 +39,7 @@ var readinessRetries = flag.Int("readiness-retries", 0, "The number of resource type Expectations interface { Expect(o runtime.Object) CancelExpect(o runtime.Object) - TryCancelExpect(o runtime.Object) + TryCancelExpect(o runtime.Object) bool ExpectationsDone() Observe(o runtime.Object) Satisfied() bool @@ -146,19 +146,22 @@ func (t *objectTracker) CancelExpect(o runtime.Object) { t.cancelExpectNoLock(k) } -func (t *objectTracker) TryCancelExpect(o runtime.Object) { +// TryCancelExpect will check the readinessRetries left on an Object, and cancel +// the expectation for that object if no retries remain. Returns True if the +// expecatation was cancelled. +func (t *objectTracker) TryCancelExpect(o runtime.Object) bool { t.mu.Lock() defer t.mu.Unlock() // Respect circuit-breaker. if t.allSatisfied { - return + return false } k, err := objKeyFromObject(o) if err != nil { log.Error(err, "skipping") - return + return false } // Check if it's time to delete an expectation or just decrement its allotted retries @@ -173,6 +176,8 @@ func (t *objectTracker) TryCancelExpect(o runtime.Object) { if shouldDel { t.cancelExpectNoLock(k) } + + return shouldDel } // ExpectationsDone tells the tracker to stop accepting new expectations. diff --git a/pkg/readiness/ready_tracker.go b/pkg/readiness/ready_tracker.go index 62831f6096f..ddbb602deef 100644 --- a/pkg/readiness/ready_tracker.go +++ b/pkg/readiness/ready_tracker.go @@ -131,16 +131,30 @@ func (t *Tracker) ForData(gvk schema.GroupVersionKind) Expectations { return t.data.Get(gvk) } -// CancelTemplate stops expecting the provided ConstraintTemplate and associated Constraints. -func (t *Tracker) CancelTemplate(ct *templates.ConstraintTemplate) { - log.V(1).Info("cancel tracking for template", "namespace", ct.GetNamespace(), "name", ct.GetName()) - t.templates.CancelExpect(ct) +func (t *Tracker) templateCleanup(ct *templates.ConstraintTemplate) { gvk := constraintGVK(ct) t.constraints.Remove(gvk) <-t.ready // constraintTrackers are setup in Run() t.constraintTrackers.Cancel(gvk.String()) } +// CancelTemplate stops expecting the provided ConstraintTemplate and associated Constraints. +func (t *Tracker) CancelTemplate(ct *templates.ConstraintTemplate) { + log.V(1).Info("cancel tracking for template", "namespace", ct.GetNamespace(), "name", ct.GetName()) + t.templates.CancelExpect(ct) + t.templateCleanup(ct) +} + +// TryCancelTemplate will check the readiness retries left on a CT and +// cancel the expectation for that CT and its associated Constraints if +// no retries remain. +func (t *Tracker) TryCancelTemplate(ct *templates.ConstraintTemplate) { + log.V(1).Info("try to cancel tracking for template", "namespace", ct.GetNamespace(), "name", ct.GetName()) + if t.templates.TryCancelExpect(ct) { + t.templateCleanup(ct) + } +} + // CancelData stops expecting data for the specified resource kind. func (t *Tracker) CancelData(gvk schema.GroupVersionKind) { log.V(1).Info("cancel tracking for data", "gvk", gvk)