Skip to content

Commit

Permalink
Improve KnativeEventing finalization for TLS resources (knative#1590)
Browse files Browse the repository at this point in the history
For optional resources like cert-manager's Certificates and Issuers, we don't
want to fail finalization when such operators are not installed, so during
finalization, we split the resources in
- optional resources (TLS resources, etc)
- resources (core k8s resources)

Then, we delete `resources` first and after we delete optional resources
while also ignoring errors returned when such operators are not instaled.

Signed-off-by: Pierangelo Di Pilato <[email protected]>
  • Loading branch information
pierDipi committed Oct 9, 2023
1 parent 4525683 commit 6b8df46
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 3 deletions.
6 changes: 4 additions & 2 deletions pkg/reconciler/knativeeventing/eventing_tls.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,17 @@ import (
"knative.dev/operator/pkg/apis/operator/v1beta1"
)

var (
tlsResourcesPred = byGroup("cert-manager.io")
)

func (r *Reconciler) handleTLSResources(ctx context.Context, manifests *mf.Manifest, comp base.KComponent) error {
instance := comp.(*v1beta1.KnativeEventing)

if isTLSEnabled(instance) {
return nil
}

tlsResourcesPred := byGroup("cert-manager.io")

// Delete TLS resources (if present)
toBeDeleted := manifests.Filter(tlsResourcesPred)
if err := toBeDeleted.Delete(mf.IgnoreNotFound(true)); err != nil && !meta.IsNoMatchError(err) {
Expand Down
21 changes: 20 additions & 1 deletion pkg/reconciler/knativeeventing/knativeeventing.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"fmt"

mf "github.com/manifestival/manifestival"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes"

Expand Down Expand Up @@ -90,9 +91,27 @@ func (r *Reconciler) FinalizeKind(ctx context.Context, original *v1beta1.Knative
return nil
}

if err = common.Uninstall(manifest); err != nil {
// For optional resources like cert-manager's Certificates and Issuers, we don't want to fail
// finalization when such operator is not installed, so we split the resources in
// - optional resources (TLS resources, etc)
// - resources (core k8s resources)
//
// Then, we delete `resources` first and after we delete optional resources while also ignoring
// errors returned when such operators are not installed.

optionalResourcesPred := mf.Any(tlsResourcesPred)

optionalResources := manifest.Filter(optionalResourcesPred)
resources := manifest.Filter(mf.Not(optionalResourcesPred))

if err = common.Uninstall(&resources); err != nil {
logger.Error("Failed to finalize platform resources", err)
}

if err := common.Uninstall(&optionalResources); err != nil && !meta.IsNoMatchError(err) {
logger.Error("Failed to finalize platform resources", err)
}

return nil
}

Expand Down

0 comments on commit 6b8df46

Please sign in to comment.