From 09eb0bd05d32bed3e849b7e7b321dfa8e598d80a Mon Sep 17 00:00:00 2001 From: OpenShift Cherrypick Robot Date: Thu, 2 Nov 2023 12:03:44 +0000 Subject: [PATCH] Improve eventing istio resources lifecycle (#2365) Improve handling of the transition enabled <-> disable for the istio feature flag. Signed-off-by: Pierangelo Di Pilato Co-authored-by: Pierangelo Di Pilato --- .../pkg/eventing/extension.go | 34 +++++++++++++++++-- .../pkg/eventing/extension_test.go | 4 +++ test/monitoringe2e/monitoring_test.go | 2 +- 3 files changed, 37 insertions(+), 3 deletions(-) diff --git a/openshift-knative-operator/pkg/eventing/extension.go b/openshift-knative-operator/pkg/eventing/extension.go index 89e11797ea..7ae800ae6d 100644 --- a/openshift-knative-operator/pkg/eventing/extension.go +++ b/openshift-knative-operator/pkg/eventing/extension.go @@ -6,13 +6,20 @@ import ( "os" mf "github.com/manifestival/manifestival" + "go.uber.org/zap" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/dynamic" "k8s.io/client-go/kubernetes" "knative.dev/operator/pkg/apis/operator/base" operatorv1beta1 "knative.dev/operator/pkg/apis/operator/v1beta1" operator "knative.dev/operator/pkg/reconciler/common" kubeclient "knative.dev/pkg/client/injection/kube/client" "knative.dev/pkg/controller" + "knative.dev/pkg/injection/clients/dynamicclient" + "knative.dev/pkg/logging" "knative.dev/pkg/ptr" "github.com/openshift-knative/serverless-operator/openshift-knative-operator/pkg/common" @@ -25,12 +32,16 @@ const requiredNsEnvName = "REQUIRED_EVENTING_NAMESPACE" // NewExtension creates a new extension for a Knative Eventing controller. func NewExtension(ctx context.Context, _ *controller.Impl) operator.Extension { return &extension{ - kubeclient: kubeclient.Get(ctx), + kubeclient: kubeclient.Get(ctx), + dynamicclient: dynamicclient.Get(ctx), + logger: logging.FromContext(ctx), } } type extension struct { - kubeclient kubernetes.Interface + kubeclient kubernetes.Interface + dynamicclient dynamic.Interface + logger *zap.SugaredLogger } func (e *extension) Manifests(ke base.KComponent) ([]mf.Manifest, error) { @@ -44,10 +55,29 @@ func (e *extension) Manifests(ke base.KComponent) ([]mf.Manifest, error) { } if enabled := eventingistio.IsEnabled(ke.GetSpec().GetConfig()); enabled { m = append(m, p) + } else { + // This handles the case when it transitions from "enabled" to "disabled". + e.deleteResourcesSilently(p) } return m, nil } +func (e *extension) deleteResourcesSilently(m mf.Manifest) { + for _, np := range m.Resources() { + r /* plural */, _ /* singular */ := meta.UnsafeGuessKindToResource(np.GroupVersionKind()) + err := e.dynamicclient.Resource(r). + Namespace(np.GetNamespace()). + Delete(context.Background(), np.GetName(), metav1.DeleteOptions{}) + if err != nil && !apierrors.IsNotFound(err) { + // Do not fail completely, just log the error + e.logger.Warnw("Failed to delete resource", + zap.Any("resource", r), + zap.String("namespace", np.GetNamespace()), + zap.String("name", np.GetName())) + } + } +} + func (e *extension) Transformers(ke base.KComponent) []mf.Transformer { tf := []mf.Transformer{ common.InjectCommonLabelIntoNamespace(), diff --git a/openshift-knative-operator/pkg/eventing/extension_test.go b/openshift-knative-operator/pkg/eventing/extension_test.go index 60bd3b9d30..c61d166555 100644 --- a/openshift-knative-operator/pkg/eventing/extension_test.go +++ b/openshift-knative-operator/pkg/eventing/extension_test.go @@ -11,11 +11,13 @@ import ( "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes/scheme" "k8s.io/utils/pointer" "knative.dev/operator/pkg/apis/operator/base" operatorv1beta1 "knative.dev/operator/pkg/apis/operator/v1beta1" "knative.dev/pkg/apis" kubefake "knative.dev/pkg/client/injection/kube/client/fake" + dynamicfake "knative.dev/pkg/injection/clients/dynamicclient/fake" "github.com/openshift-knative/serverless-operator/openshift-knative-operator/pkg/common" "github.com/openshift-knative/serverless-operator/openshift-knative-operator/pkg/monitoring" @@ -138,6 +140,7 @@ func TestReconcile(t *testing.T) { ke := c.in.DeepCopy() ctx, _ := kubefake.With(context.Background(), &eventingNamespace) + ctx, _ = dynamicfake.With(ctx, scheme.Scheme) ext := NewExtension(ctx, nil) ext.Reconcile(context.Background(), ke) @@ -271,6 +274,7 @@ func TestMonitoring(t *testing.T) { c.expected.Namespace = ke.Namespace ctx, _ := ocpfake.With(context.Background(), objs...) ctx, kube := kubefake.With(ctx, &eventingNamespace) + ctx, _ = dynamicfake.With(ctx, scheme.Scheme) ext := NewExtension(ctx, nil) shouldEnableMonitoring, err := c.setupMonitoringToggle() diff --git a/test/monitoringe2e/monitoring_test.go b/test/monitoringe2e/monitoring_test.go index 68bcdfa092..0b704edea9 100644 --- a/test/monitoringe2e/monitoring_test.go +++ b/test/monitoringe2e/monitoring_test.go @@ -35,7 +35,7 @@ func TestKnativeMetrics(t *testing.T) { t.Run("verify Eventing metrics work correctly", func(t *testing.T) { // Eventing control plane metrics should work if err := VerifyMetrics(ctx, eventingMetricQueries); err != nil { - t.Fatal("Failed to verify that Eventing control plane metrics work correctly", err) + t.Fatal("Failed to verify that Eventing metrics work correctly", err) } })