diff --git a/knative-operator/pkg/controller/knativeeventing/knativeeventing_controller.go b/knative-operator/pkg/controller/knativeeventing/knativeeventing_controller.go index 2309d534fc..6b6e10dc7a 100644 --- a/knative-operator/pkg/controller/knativeeventing/knativeeventing_controller.go +++ b/knative-operator/pkg/controller/knativeeventing/knativeeventing_controller.go @@ -5,10 +5,8 @@ import ( "fmt" "os" - "github.com/openshift-knative/serverless-operator/knative-operator/pkg/common" - "github.com/openshift-knative/serverless-operator/knative-operator/pkg/monitoring" - "github.com/openshift-knative/serverless-operator/knative-operator/pkg/monitoring/dashboards" - socommon "github.com/openshift-knative/serverless-operator/pkg/common" + monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" + appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/api/errors" @@ -24,6 +22,11 @@ import ( "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/controller-runtime/pkg/source" + + "github.com/openshift-knative/serverless-operator/knative-operator/pkg/common" + "github.com/openshift-knative/serverless-operator/knative-operator/pkg/monitoring" + "github.com/openshift-knative/serverless-operator/knative-operator/pkg/monitoring/dashboards" + socommon "github.com/openshift-knative/serverless-operator/pkg/common" ) const ( @@ -110,7 +113,7 @@ func (r *ReconcileKnativeEventing) Reconcile(ctx context.Context, request reconc } instance := original.DeepCopy() - reconcileErr := r.reconcileKnativeEventing(instance) + reconcileErr := r.reconcileKnativeEventing(ctx, instance) if !equality.Semantic.DeepEqual(original.Status, instance.Status) { if err := r.client.Status().Update(context.TODO(), instance); err != nil { @@ -126,9 +129,10 @@ func (r *ReconcileKnativeEventing) Reconcile(ctx context.Context, request reconc return reconcile.Result{}, reconcileErr } -func (r *ReconcileKnativeEventing) reconcileKnativeEventing(instance *operatorv1alpha1.KnativeEventing) error { +func (r *ReconcileKnativeEventing) reconcileKnativeEventing(ctx context.Context, instance *operatorv1alpha1.KnativeEventing) error { stages := []func(*operatorv1alpha1.KnativeEventing) error{ r.ensureFinalizers, + r.deleteSugar(ctx), r.installDashboards, } for _, stage := range stages { @@ -187,3 +191,42 @@ func (r *ReconcileKnativeEventing) delete(instance *operatorv1alpha1.KnativeEven } return nil } + +func (r *ReconcileKnativeEventing) deleteSugar(ctx context.Context) func(eventing *operatorv1alpha1.KnativeEventing) error { + return func(eventing *operatorv1alpha1.KnativeEventing) error { + ns := "knative-eventing" + nsClient := client.NewNamespacedClient(r.client, ns) + + sugarControllerDeployment := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + Name: "sugar-controller", + }, + } + if err := nsClient.Delete(ctx, sugarControllerDeployment); err != nil && !errors.IsNotFound(err) { + return fmt.Errorf("failed to delete deployment %s/%s: %w", sugarControllerDeployment.Namespace, sugarControllerDeployment.Name, err) + } + + sugarControllerServiceForMonitoring := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + Name: "sugar-controller-sm-service", + }, + } + if err := nsClient.Delete(ctx, sugarControllerServiceForMonitoring); err != nil && !errors.IsNotFound(err) { + return fmt.Errorf("failed to delete service %s/%s: %w", sugarControllerServiceForMonitoring.Namespace, sugarControllerServiceForMonitoring.Name, err) + } + + sugarControllerServiceMonitor := &monitoringv1.ServiceMonitor{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + Name: "sugar-controller-sm", + }, + } + if err := nsClient.Delete(ctx, sugarControllerServiceForMonitoring); err != nil && !errors.IsNotFound(err) { + return fmt.Errorf("failed to delete service monitor %s/%s: %w", sugarControllerServiceMonitor.Namespace, sugarControllerServiceMonitor.Name, err) + } + + return nil + } +} diff --git a/knative-operator/pkg/controller/knativeeventing/knativeeventing_controller_test.go b/knative-operator/pkg/controller/knativeeventing/knativeeventing_controller_test.go index 45aeb39243..13255c505c 100644 --- a/knative-operator/pkg/controller/knativeeventing/knativeeventing_controller_test.go +++ b/knative-operator/pkg/controller/knativeeventing/knativeeventing_controller_test.go @@ -1,42 +1,12 @@ package knativeeventing import ( - "context" "os" - "testing" - "github.com/openshift-knative/serverless-operator/knative-operator/pkg/apis" - "github.com/openshift-knative/serverless-operator/knative-operator/pkg/monitoring/dashboards" - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/scheme" - operatorv1alpha1 "knative.dev/operator/pkg/apis/operator/v1alpha1" - "sigs.k8s.io/controller-runtime/pkg/client/fake" - "sigs.k8s.io/controller-runtime/pkg/reconcile" -) -var ( - ke = &operatorv1alpha1.KnativeEventing{ - ObjectMeta: metav1.ObjectMeta{ - Name: "knative-eventing", - Namespace: "knative-eventing", - }, - } - req = reconcile.Request{ - NamespacedName: types.NamespacedName{Namespace: ke.Namespace, Name: ke.Name}, - } - dashboardNamespace = &corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: dashboards.ConfigManagedNamespace, - }, - } - eventingNamespace = &corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: "knative-eventing", - }, - } + "github.com/openshift-knative/serverless-operator/knative-operator/pkg/apis" + "github.com/openshift-knative/serverless-operator/knative-operator/pkg/monitoring/dashboards" ) func init() { @@ -45,105 +15,3 @@ func init() { apis.AddToScheme(scheme.Scheme) } - -// TestEventingReconcile runs Reconcile to verify if eventing resources are created/deleted. -func TestEventingReconcile(t *testing.T) { - tests := []struct { - name string - ownerName string - ownerNamespace string - deleted bool - }{{ - name: "reconcile request with same KnativeEventing owner", - ownerName: "knative-eventing", - ownerNamespace: "knative-eventing", - deleted: true, - }, { - name: "reconcile request with different KnativeEventing owner name", - ownerName: "FOO", - ownerNamespace: "knative-eventing", - deleted: false, - }, { - name: "reconcile request with different KnativeEventing owner namespace", - ownerName: "knative-eventing", - ownerNamespace: "FOO", - deleted: false, - }} - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - cl := fake.NewClientBuilder().WithObjects(ke, dashboardNamespace, eventingNamespace).Build() - r := &ReconcileKnativeEventing{client: cl, scheme: scheme.Scheme} - // Reconcile to initialize - if _, err := r.Reconcile(context.Background(), req); err != nil { - t.Fatalf("reconcile: (%v)", err) - } - // Check if Eventing dashboard configmaps are available - resourcesCM := &corev1.ConfigMap{} - err := cl.Get(context.TODO(), types.NamespacedName{Name: "grafana-dashboard-definition-knative-eventing-resources", Namespace: dashboardNamespace.Name}, resourcesCM) - if err != nil { - t.Fatalf("get: (%v)", err) - } - brokerCM := &corev1.ConfigMap{} - err = cl.Get(context.TODO(), types.NamespacedName{Name: "grafana-dashboard-definition-knative-eventing-broker", Namespace: dashboardNamespace.Name}, brokerCM) - if err != nil { - t.Fatalf("get: (%v)", err) - } - sourceCM := &corev1.ConfigMap{} - err = cl.Get(context.TODO(), types.NamespacedName{Name: "grafana-dashboard-definition-knative-eventing-source", Namespace: dashboardNamespace.Name}, sourceCM) - if err != nil { - t.Fatalf("get: (%v)", err) - } - channelCM := &corev1.ConfigMap{} - err = cl.Get(context.TODO(), types.NamespacedName{Name: "grafana-dashboard-definition-knative-eventing-channel", Namespace: dashboardNamespace.Name}, channelCM) - if err != nil { - t.Fatalf("get: (%v)", err) - } - // Delete Dashboard configmaps. - err = cl.Delete(context.TODO(), resourcesCM) - if err != nil { - t.Fatalf("delete: (%v)", err) - } - err = cl.Delete(context.TODO(), brokerCM) - if err != nil { - t.Fatalf("delete: (%v)", err) - } - err = cl.Delete(context.TODO(), sourceCM) - if err != nil { - t.Fatalf("delete: (%v)", err) - } - err = cl.Delete(context.TODO(), channelCM) - if err != nil { - t.Fatalf("delete: (%v)", err) - } - // Reconcile again with test requests. - req := reconcile.Request{ - NamespacedName: types.NamespacedName{Namespace: test.ownerNamespace, Name: test.ownerName}, - } - if _, err := r.Reconcile(context.Background(), req); err != nil { - t.Fatalf("reconcile: (%v)", err) - } - var checkError = func(t *testing.T, err error) { - if test.deleted { - if err != nil { - t.Fatalf("get: (%v)", err) - } - } - if !test.deleted { - if !errors.IsNotFound(err) { - t.Fatalf("get: (%v)", err) - } - } - } - // Check again if Eventing dashboard configmaps are available - err = cl.Get(context.TODO(), types.NamespacedName{Name: "grafana-dashboard-definition-knative-eventing-resources", Namespace: dashboardNamespace.Name}, resourcesCM) - checkError(t, err) - err = cl.Get(context.TODO(), types.NamespacedName{Name: "grafana-dashboard-definition-knative-eventing-broker", Namespace: dashboardNamespace.Name}, brokerCM) - checkError(t, err) - err = cl.Get(context.TODO(), types.NamespacedName{Name: "grafana-dashboard-definition-knative-eventing-source", Namespace: dashboardNamespace.Name}, sourceCM) - checkError(t, err) - err = cl.Get(context.TODO(), types.NamespacedName{Name: "grafana-dashboard-definition-knative-eventing-channel", Namespace: dashboardNamespace.Name}, sourceCM) - checkError(t, err) - }) - } -} diff --git a/openshift-knative-operator/pkg/monitoring/monitoring_eventing.go b/openshift-knative-operator/pkg/monitoring/monitoring_eventing.go index 18a7843ad2..0dc327591a 100644 --- a/openshift-knative-operator/pkg/monitoring/monitoring_eventing.go +++ b/openshift-knative-operator/pkg/monitoring/monitoring_eventing.go @@ -10,7 +10,7 @@ import ( ) var ( - eventingDeployments = sets.NewString("eventing-controller", "eventing-webhook", "imc-controller", "imc-dispatcher", "mt-broker-controller", "mt-broker-filter", "mt-broker-ingress", "sugar-controller") + eventingDeployments = sets.NewString("eventing-controller", "eventing-webhook", "imc-controller", "imc-dispatcher", "mt-broker-controller", "mt-broker-filter", "mt-broker-ingress") ) func ReconcileMonitoringForEventing(ctx context.Context, api kubernetes.Interface, ke *operatorv1alpha1.KnativeEventing) error { diff --git a/test/e2e/dashboards.go b/test/e2e/dashboards.go new file mode 100644 index 0000000000..0bec18dc2c --- /dev/null +++ b/test/e2e/dashboards.go @@ -0,0 +1,51 @@ +package e2e + +import ( + "context" + "testing" + "time" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/wait" + + "github.com/openshift-knative/serverless-operator/test" +) + +var ( + EventingDashboards = []string{ + "grafana-dashboard-definition-knative-eventing-resources", + "grafana-dashboard-definition-knative-eventing-broker", + "grafana-dashboard-definition-knative-eventing-kafka-broker", + "grafana-dashboard-definition-knative-eventing-source", + "grafana-dashboard-definition-knative-eventing-channel", + "grafana-dashboard-definition-knative-eventing-kafka-sink", + } +) + +func VerifyDashboards(t *testing.T, caCtx *test.Context, dashboards []string) { + t.Run("Verify dashboards", func(t *testing.T) { + t.Parallel() + + ns := "openshift-config-managed" + ctx := context.Background() + + for _, d := range dashboards { + t.Run(d, func(t *testing.T) { + err := wait.PollImmediate(time.Second, time.Minute, func() (bool, error) { + _, err := caCtx.Clients.Kube.CoreV1().ConfigMaps(ns).Get(ctx, d, metav1.GetOptions{}) + if err != nil && !apierrors.IsNotFound(err) { + return false, err + } + if apierrors.IsNotFound(err) { + return false, nil + } + return true, nil + }) + if err != nil { + t.Error(err) + } + }) + } + }) +} diff --git a/test/e2e/knative_eventing_test.go b/test/e2e/knative_eventing_test.go index 524e83b3cf..da1a0980df 100644 --- a/test/e2e/knative_eventing_test.go +++ b/test/e2e/knative_eventing_test.go @@ -53,10 +53,18 @@ func TestKnativeEventing(t *testing.T) { VerifyNoDisallowedImageReference(t, caCtx, eventingNamespace) }) + t.Run("Verify sugar controller deletion", func(t *testing.T) { + if err := test.CheckNoDeployment(caCtx.Clients.Kube, eventingNamespace, "sugar-controller"); err != nil { + t.Errorf("sugar-controller is still present: %v", err) + } + }) + t.Run("Verify job succeeded", func(t *testing.T) { upgrade.VerifyPostInstallJobs(caCtx, upgrade.VerifyPostJobsConfig{ Namespace: eventingNamespace, FailOnNoJobs: true, }) }) + + VerifyDashboards(t, caCtx, EventingDashboards) } diff --git a/test/service.go b/test/service.go index 61550f4f17..016ef5d807 100644 --- a/test/service.go +++ b/test/service.go @@ -4,13 +4,16 @@ import ( "context" "fmt" "strings" + "time" routev1 "github.com/openshift/api/route/v1" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/client-go/kubernetes" servingv1 "knative.dev/serving/pkg/apis/serving/v1" servingv1alpha1 "knative.dev/serving/pkg/apis/serving/v1alpha1" ) @@ -101,6 +104,20 @@ func CheckDeploymentScale(ctx *Context, ns, name string, scale int) error { return nil } +func CheckNoDeployment(kube *kubernetes.Clientset, ns, name string) error { + return wait.PollImmediate(time.Second, time.Minute, func() (bool, error) { + _, err := kube.AppsV1().Deployments(ns).Get(context.Background(), name, metav1.GetOptions{}) + if err != nil && apierrors.IsNotFound(err) { + return true, nil + } + if err != nil { + return false, err + } + + return false, nil + }) +} + func WaitForServiceState(ctx *Context, name, namespace string, inState func(s *servingv1.Service, err error) (bool, error)) (*servingv1.Service, error) { var ( lastState *servingv1.Service diff --git a/test/upgrade/upgrade_test.go b/test/upgrade/upgrade_test.go index 3c79e182d3..36e7fdf9f2 100644 --- a/test/upgrade/upgrade_test.go +++ b/test/upgrade/upgrade_test.go @@ -144,6 +144,7 @@ func postUpgradeTests(ctx *test.Context) []pkgupgrade.Operation { kafkaupgrade.SourcePostUpgradeTest(), kafkabrokerupgrade.BrokerPostUpgradeTest(), kafkabrokerupgrade.SinkPostUpgradeTest(), + upgrade.VerifySugarControllerDeletion(ctx), ) tests = append(tests, servingupgrade.ServingPostUpgradeTests()...) return tests diff --git a/test/upgrade/verify_upgrade.go b/test/upgrade/verify_upgrade.go new file mode 100644 index 0000000000..c56fae5164 --- /dev/null +++ b/test/upgrade/verify_upgrade.go @@ -0,0 +1,15 @@ +package upgrade + +import ( + "knative.dev/pkg/test/upgrade" + + "github.com/openshift-knative/serverless-operator/test" +) + +func VerifySugarControllerDeletion(ctx *test.Context) upgrade.Operation { + return upgrade.NewOperation("Verify sugar-controller deletion", func(c upgrade.Context) { + if err := test.CheckNoDeployment(ctx.Clients.Kube, "knative-eventing", "sugar-controller"); err != nil { + c.T.Error(err) + } + }) +}