Do not reconcile images and builds being deleted

[danail-branekov]

When an image is being deleted, k8s does not delete the object immediately, instead it sets the deletion timestamp awaiting finalization. See
k8s docs for details

If the kpack image (or its builds) has a finalizer being added by an external component (such as
Korifi), then when the client request the object to be deleted, the image/build reconcilers keep reconciling the image/build causing new build pods to be created as the image is being deleted. Even though eventually the image gets deleted, it takes significant amount of time.

In order to address this, this PR "short-circuits" the image and build reconcilers to immediately return whenever an image or a build has their deletion timestamp set, thus preventing builds and build pods being rescheduled.

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 13.33333% with 39 lines in your changes missing coverage. Please review.

Project coverage is 35.29%. Comparing base (49014e2) to head (e4ef85e).
Report is 101 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/reconciler/buildpack/buildpack.go	0.00%	6 Missing ⚠️
...kg/reconciler/clusterbuildpack/clusterbuildpack.go	0.00%	5 Missing ⚠️
pkg/reconciler/build/build.go	0.00%	4 Missing ⚠️
pkg/reconciler/builder/builder.go	0.00%	4 Missing ⚠️
pkg/reconciler/clusterbuilder/clusterbuilder.go	0.00%	4 Missing ⚠️
pkg/reconciler/clusterstack/clusterstack.go	0.00%	4 Missing ⚠️
pkg/reconciler/clusterstore/clusterstore.go	0.00%	4 Missing ⚠️
pkg/reconciler/image/image.go	0.00%	4 Missing ⚠️
pkg/reconciler/sourceresolver/sourceresolver.go	0.00%	4 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #1820       +/-   ##
===========================================
- Coverage   67.34%   35.29%   -32.05%     
===========================================
  Files         140      258      +118     
  Lines        8886    20001    +11115     
===========================================
+ Hits         5984     7060     +1076     
- Misses       2393    12339     +9946     
- Partials      509      602       +93     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[chenbh]

It feels kinda awkward to have this short circuit on only the Build and Image, I wonder if we should just add this to all our reconcilers.

I imagine we can make use of the FilterFunc to skip any resource with non-nil DeletionTimestamps

kpack/pkg/reconciler/image/image.go

Lines 66 to 71 in d3e70fe

	imageInformer.Informer().AddEventHandler(controller.HandleAll(impl.Enqueue))
	buildInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{
	FilterFunc: controller.FilterControllerGK(buildapi.SchemeGroupVersion.WithKind(Kind).GroupKind()),
	Handler: controller.HandleAll(impl.EnqueueControllerOf),
	})

Something like pkg/reconciler/filter.go:

func FilterDeletionTimestamp(obj interface{}) bool {
	object, ok := obj.(metav1.Object)
	if !ok {
		return false
	}

	return object.GetDeletionTimestamp() == nil
}

And in each reconciler:

- informer.Informer().AddEventHandler(controller.HandleAll(impl.Enqueue))

+ informer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{ 
+  	FilterFunc: reconciler.FilterDeletionTimestamp, 
+  	Handler:    controller.HandleAll(impl.Enqueue), 
+  }) 

This way we can also get away without modifying all the reconciler tests

[georgethebeatle]

@chenbh thanks for the suggestion. We just pushed a new commit, is that what you meant?