diff --git a/pkg/reconciler/v1alpha1/autoscaling/hpa/hpa.go b/pkg/reconciler/v1alpha1/autoscaling/hpa/hpa.go index 34616836e33c..b3ed040310c8 100644 --- a/pkg/reconciler/v1alpha1/autoscaling/hpa/hpa.go +++ b/pkg/reconciler/v1alpha1/autoscaling/hpa/hpa.go @@ -131,6 +131,10 @@ func (c *Reconciler) Reconcile(ctx context.Context, key string) error { func (c *Reconciler) reconcile(ctx context.Context, key string, pa *pav1alpha1.PodAutoscaler) error { logger := logging.FromContext(ctx) + if pa.GetDeletionTimestamp() != nil { + return nil + } + // We may be reading a version of the object that was stored at an older version // and may not have had all of the assumed defaults specified. This won't result // in this getting written back to the API Server, but lets downstream logic make diff --git a/pkg/reconciler/v1alpha1/autoscaling/hpa/hpa_test.go b/pkg/reconciler/v1alpha1/autoscaling/hpa/hpa_test.go index 69485d646322..e5c8aef414c0 100644 --- a/pkg/reconciler/v1alpha1/autoscaling/hpa/hpa_test.go +++ b/pkg/reconciler/v1alpha1/autoscaling/hpa/hpa_test.go @@ -55,6 +55,13 @@ func TestReconcile(t *testing.T) { pa(testRevision, testNamespace, WithKPAClass), }, Key: key(testRevision, testNamespace), + }, { + Name: "nop deletion reconcile", + // Test that with a DeletionTimestamp we do nothing. + Objects: []runtime.Object{ + pa(testRevision, testNamespace, WithHPAClass, WithPADeletionTimestamp), + }, + Key: key(testRevision, testNamespace), }, { Name: "delete when pa does not exist", Objects: []runtime.Object{}, diff --git a/pkg/reconciler/v1alpha1/autoscaling/kpa/kpa.go b/pkg/reconciler/v1alpha1/autoscaling/kpa/kpa.go index 9a38eeabe4d9..498a8504d4f0 100644 --- a/pkg/reconciler/v1alpha1/autoscaling/kpa/kpa.go +++ b/pkg/reconciler/v1alpha1/autoscaling/kpa/kpa.go @@ -173,6 +173,10 @@ func (c *Reconciler) Reconcile(ctx context.Context, key string) error { func (c *Reconciler) reconcile(ctx context.Context, pa *pav1alpha1.PodAutoscaler) error { logger := logging.FromContext(ctx) + if pa.GetDeletionTimestamp() != nil { + return nil + } + // We may be reading a version of the object that was stored at an older version // and may not have had all of the assumed defaults specified. This won't result // in this getting written back to the API Server, but lets downstream logic make diff --git a/pkg/reconciler/v1alpha1/clusteringress/clusteringress.go b/pkg/reconciler/v1alpha1/clusteringress/clusteringress.go index 0fce784eb0b8..0b7bb48bd0f6 100644 --- a/pkg/reconciler/v1alpha1/clusteringress/clusteringress.go +++ b/pkg/reconciler/v1alpha1/clusteringress/clusteringress.go @@ -174,6 +174,9 @@ func (c *Reconciler) updateStatus(desired *v1alpha1.ClusterIngress) (*v1alpha1.C func (c *Reconciler) reconcile(ctx context.Context, ci *v1alpha1.ClusterIngress) error { logger := logging.FromContext(ctx) + if ci.GetDeletionTimestamp() != nil { + return nil + } // We may be reading a version of the object that was stored at an older version // and may not have had all of the assumed defaults specified. This won't result diff --git a/pkg/reconciler/v1alpha1/configuration/configuration.go b/pkg/reconciler/v1alpha1/configuration/configuration.go index 9ddada2a3f23..42d92a84a009 100644 --- a/pkg/reconciler/v1alpha1/configuration/configuration.go +++ b/pkg/reconciler/v1alpha1/configuration/configuration.go @@ -149,6 +149,9 @@ func (c *Reconciler) Reconcile(ctx context.Context, key string) error { func (c *Reconciler) reconcile(ctx context.Context, config *v1alpha1.Configuration) error { logger := logging.FromContext(ctx) + if config.GetDeletionTimestamp() != nil { + return nil + } // We may be reading a version of the object that was stored at an older version // and may not have had all of the assumed defaults specified. This won't result diff --git a/pkg/reconciler/v1alpha1/configuration/configuration_test.go b/pkg/reconciler/v1alpha1/configuration/configuration_test.go index 1a8982f84d85..67d58bed58cd 100644 --- a/pkg/reconciler/v1alpha1/configuration/configuration_test.go +++ b/pkg/reconciler/v1alpha1/configuration/configuration_test.go @@ -60,6 +60,13 @@ func TestReconcile(t *testing.T) { }, { Name: "key not found", Key: "foo/not-found", + }, { + Name: "nop deletion reconcile", + // Test that with a DeletionTimestamp we do nothing. + Objects: []runtime.Object{ + cfg("foo", "delete-pending", 1234, WithConfigDeletionTimestamp), + }, + Key: "foo/delete-pending", }, { Name: "create revision matching generation", Objects: []runtime.Object{ diff --git a/pkg/reconciler/v1alpha1/revision/revision.go b/pkg/reconciler/v1alpha1/revision/revision.go index 0bc0157cc85a..09fce3728134 100644 --- a/pkg/reconciler/v1alpha1/revision/revision.go +++ b/pkg/reconciler/v1alpha1/revision/revision.go @@ -350,6 +350,9 @@ func (c *Reconciler) reconcileDigest(ctx context.Context, rev *v1alpha1.Revision func (c *Reconciler) reconcile(ctx context.Context, rev *v1alpha1.Revision) error { logger := commonlogging.FromContext(ctx) + if rev.GetDeletionTimestamp() != nil { + return nil + } // We may be reading a version of the object that was stored at an older version // and may not have had all of the assumed defaults specified. This won't result diff --git a/pkg/reconciler/v1alpha1/revision/table_test.go b/pkg/reconciler/v1alpha1/revision/table_test.go index d7768189504d..a5603435b397 100644 --- a/pkg/reconciler/v1alpha1/revision/table_test.go +++ b/pkg/reconciler/v1alpha1/revision/table_test.go @@ -52,6 +52,13 @@ func TestReconcile(t *testing.T) { Name: "key not found", // Make sure Reconcile handles good keys that don't exist. Key: "foo/not-found", + }, { + Name: "nop deletion reconcile", + // Test that with a DeletionTimestamp we do nothing. + Objects: []runtime.Object{ + rev("foo", "delete-pending", WithRevisionDeletionTimestamp), + }, + Key: "foo/delete-pending", }, { Name: "first revision reconciliation", // Test the simplest successful reconciliation flow. diff --git a/pkg/reconciler/v1alpha1/route/route.go b/pkg/reconciler/v1alpha1/route/route.go index 75a5f58c8899..6953ee1741b0 100644 --- a/pkg/reconciler/v1alpha1/route/route.go +++ b/pkg/reconciler/v1alpha1/route/route.go @@ -222,7 +222,6 @@ func (c *Reconciler) Reconcile(ctx context.Context, key string) error { func (c *Reconciler) reconcile(ctx context.Context, r *v1alpha1.Route) error { logger := logging.FromContext(ctx) - if r.GetDeletionTimestamp() != nil { // Check for a DeletionTimestamp. If present, elide the normal reconcile logic. return c.reconcileDeletion(ctx, r) diff --git a/pkg/reconciler/v1alpha1/service/service.go b/pkg/reconciler/v1alpha1/service/service.go index 5f9e341d25c5..20500b61aa3b 100644 --- a/pkg/reconciler/v1alpha1/service/service.go +++ b/pkg/reconciler/v1alpha1/service/service.go @@ -160,6 +160,9 @@ func (c *Reconciler) Reconcile(ctx context.Context, key string) error { func (c *Reconciler) reconcile(ctx context.Context, service *v1alpha1.Service) error { logger := logging.FromContext(ctx) + if service.GetDeletionTimestamp() != nil { + return nil + } // We may be reading a version of the object that was stored at an older version // and may not have had all of the assumed defaults specified. This won't result diff --git a/pkg/reconciler/v1alpha1/service/service_test.go b/pkg/reconciler/v1alpha1/service/service_test.go index 370970f9e781..515ec6757da6 100644 --- a/pkg/reconciler/v1alpha1/service/service_test.go +++ b/pkg/reconciler/v1alpha1/service/service_test.go @@ -44,6 +44,13 @@ func TestReconcile(t *testing.T) { }, { Name: "key not found", Key: "foo/not-found", + }, { + Name: "nop deletion reconcile", + // Test that with a DeletionTimestamp we do nothing. + Objects: []runtime.Object{ + svc("delete-pending", "foo", WithServiceDeletionTimestamp), + }, + Key: "foo/delete-pending", }, { Name: "incomplete service", Objects: []runtime.Object{ diff --git a/pkg/reconciler/v1alpha1/testing/functional.go b/pkg/reconciler/v1alpha1/testing/functional.go index 29d107840a9b..1783ccc3c560 100644 --- a/pkg/reconciler/v1alpha1/testing/functional.go +++ b/pkg/reconciler/v1alpha1/testing/functional.go @@ -97,6 +97,12 @@ var ( } ) +// WithServiceDeletionTimestamp will set the DeletionTimestamp on the Service. +func WithServiceDeletionTimestamp(r *v1alpha1.Service) { + t := metav1.NewTime(time.Unix(1e9, 0)) + r.ObjectMeta.SetDeletionTimestamp(&t) +} + // WithRunLatestRollout configures the Service to use a "runLatest" rollout. func WithRunLatestRollout(s *v1alpha1.Service) { s.Spec = v1alpha1.ServiceSpec{ @@ -422,6 +428,12 @@ func WithRouteLabel(key, value string) RouteOption { // ConfigOption enables further configuration of a Configuration. type ConfigOption func(*v1alpha1.Configuration) +// WithConfigDeletionTimestamp will set the DeletionTimestamp on the Config. +func WithConfigDeletionTimestamp(r *v1alpha1.Configuration) { + t := metav1.NewTime(time.Unix(1e9, 0)) + r.ObjectMeta.SetDeletionTimestamp(&t) +} + // WithBuild adds a Build to the provided Configuration. func WithBuild(cfg *v1alpha1.Configuration) { cfg.Spec.Build = &v1alpha1.RawExtension{ @@ -517,6 +529,12 @@ func WithConfigLabel(key, value string) ConfigOption { // RevisionOption enables further configuration of a Revision. type RevisionOption func(*v1alpha1.Revision) +// WithRevisionDeletionTimestamp will set the DeletionTimestamp on the Revision. +func WithRevisionDeletionTimestamp(r *v1alpha1.Revision) { + t := metav1.NewTime(time.Unix(1e9, 0)) + r.ObjectMeta.SetDeletionTimestamp(&t) +} + // WithInitRevConditions calls .Status.InitializeConditions() on a Revision. func WithInitRevConditions(r *v1alpha1.Revision) { r.Status.InitializeConditions() @@ -722,6 +740,12 @@ func WithNoTraffic(reason, message string) PodAutoscalerOption { } } +// WithPADeletionTimestamp will set the DeletionTimestamp on the PodAutoscaler. +func WithPADeletionTimestamp(r *autoscalingv1alpha1.PodAutoscaler) { + t := metav1.NewTime(time.Unix(1e9, 0)) + r.ObjectMeta.SetDeletionTimestamp(&t) +} + // WithHPAClass updates the PA to add the hpa class annotation. func WithHPAClass(pa *autoscalingv1alpha1.PodAutoscaler) { if pa.Annotations == nil { diff --git a/test/clients.go b/test/clients.go index 7a30c8000188..43e9fee93104 100644 --- a/test/clients.go +++ b/test/clients.go @@ -131,6 +131,11 @@ func (clients *ServingClients) Delete(routes []string, configs []string, service {clients.Services, services}, } + propPolicy := v1.DeletePropagationForeground + dopt := &v1.DeleteOptions{ + PropagationPolicy: &propPolicy, + } + for _, deletion := range deletions { if deletion.client == nil { continue @@ -141,7 +146,7 @@ func (clients *ServingClients) Delete(routes []string, configs []string, service continue } - if err := deletion.client.Delete(item, nil); err != nil { + if err := deletion.client.Delete(item, dopt); err != nil { return err } }