Ignore resources when DeletionTimestamp is set.

When we had finalizers previously we would race with K8s GC to recreate our children as K8s reaped them.

The simplest way to test this is to enable "foreground" deletion in our e2e tests, which is implemented as a finalizer.

Fixes: knative#2678

pkg/reconciler/v1alpha1/autoscaling/hpa/hpa.go

@@ -139,6 +139,9 @@ func (c *Reconciler) reconcile(ctx context.Context, key string, pa *pav1alpha1.P
 
 	pa.Status.InitializeConditions()
 	logger.Debug("PA exists")
+	if pa.GetDeletionTimestamp() != nil {
+		return nil
+	}
 
 	// HPA-class PAs don't yet support scale-to-zero
 	pa.Status.MarkActive()

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, WithKPAClass, WithPADeletionTimestamp),
+		},
+		Key: key(testRevision, testNamespace),
 	}, {
 		Name:    "delete when pa does not exist",
 		Objects: []runtime.Object{},

pkg/reconciler/v1alpha1/autoscaling/kpa/kpa.go

@@ -181,6 +181,9 @@ func (c *Reconciler) reconcile(ctx context.Context, pa *pav1alpha1.PodAutoscaler
 
 	pa.Status.InitializeConditions()
 	logger.Debug("PA exists")
+	if pa.GetDeletionTimestamp() != nil {
+		return nil
+	}
 
 	desiredMetric := resources.MakeMetric(ctx, pa, c.dynConfig.Current())
 	metric, err := c.kpaMetrics.Get(ctx, desiredMetric.Namespace, desiredMetric.Name)

pkg/reconciler/v1alpha1/clusteringress/clusteringress.go

@@ -182,6 +182,10 @@ func (c *Reconciler) reconcile(ctx context.Context, ci *v1alpha1.ClusterIngress)
 	ci.SetDefaults()
 
 	ci.Status.InitializeConditions()
+	if ci.GetDeletionTimestamp() != nil {
+		return nil
+	}
+
 	vs := resources.MakeVirtualService(ci, gatewayNamesFromContext(ctx, ci))
 
 	logger.Infof("Reconciling clusterIngress :%v", ci)

pkg/reconciler/v1alpha1/configuration/configuration.go

@@ -157,6 +157,9 @@ func (c *Reconciler) reconcile(ctx context.Context, config *v1alpha1.Configurati
 	config.SetDefaults()
 
 	config.Status.InitializeConditions()
+	if config.GetDeletionTimestamp() != nil {
+		return nil
+	}
 
 	// First, fetch the revision that should exist for the current generation.
 	revName := resourcenames.DeprecatedRevision(config)

pkg/reconciler/v1alpha1/configuration/configuration_test.go

@@ -60,6 +60,12 @@ 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),
+		},
 	}, {
 		Name: "create revision matching generation",
 		Objects: []runtime.Object{

pkg/reconciler/v1alpha1/revision/revision.go

@@ -358,6 +358,10 @@ func (c *Reconciler) reconcile(ctx context.Context, rev *v1alpha1.Revision) erro
 	rev.SetDefaults()
 
 	rev.Status.InitializeConditions()
+	if rev.GetDeletionTimestamp() != nil {
+		return nil
+	}
+
 	c.updateRevisionLoggingURL(ctx, rev)
 
 	if err := c.reconcileBuild(ctx, rev); err != nil {

pkg/reconciler/v1alpha1/revision/table_test.go

@@ -52,6 +52,12 @@ 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),
+		},
 	}, {
 		Name: "first revision reconciliation",
 		// Test the simplest successful reconciliation flow.

pkg/reconciler/v1alpha1/service/service.go

@@ -168,6 +168,9 @@ func (c *Reconciler) reconcile(ctx context.Context, service *v1alpha1.Service) e
 	service.SetDefaults()
 
 	service.Status.InitializeConditions()
+	if service.GetDeletionTimestamp() != nil {
+		return nil
+	}
 
 	configName := resourcenames.Configuration(service)
 	config, err := c.configurationLister.Configurations(service.Namespace).Get(configName)

pkg/reconciler/v1alpha1/service/service_test.go

@@ -44,6 +44,12 @@ 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("foo", "delete-pending", WithServiceDeletionTimestamp),
+		},
 	}, {
 		Name: "incomplete service",
 		Objects: []runtime.Object{

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 {

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
 			}
 		}