From ae7d158d14b70bf72356b04add3692e6ee32bf04 Mon Sep 17 00:00:00 2001 From: Subbarao Meduri <smeduri@redhat.com> Date: Wed, 24 Jan 2024 13:34:26 -0500 Subject: [PATCH] add watch logic for addondeployment (#1289) (#1301) * add watch logic for addondeployment * make sonarcloud happy * lint * refactor predicate * lint * fix test case * refactor --------- Signed-off-by: Coleen Iona Quadros <coleen.quadros27@gmail.com> Co-authored-by: Coleen Iona Quadros <coleen.quadros27@gmail.com> --- .../controllers/placementrule/manifestwork.go | 1 - .../placementrule/placementrule_controller.go | 20 ++++- .../placementrule_controller_test.go | 73 ++++++++++++++++++- .../placementrule/predicate_func.go | 22 ++++++ .../placementrule/predicate_func_test.go | 70 ++++++++++++++++++ .../pkg/config/config.go | 1 + 6 files changed, 183 insertions(+), 4 deletions(-) diff --git a/operators/multiclusterobservability/controllers/placementrule/manifestwork.go b/operators/multiclusterobservability/controllers/placementrule/manifestwork.go index 908a399db..8fc6f8de4 100644 --- a/operators/multiclusterobservability/controllers/placementrule/manifestwork.go +++ b/operators/multiclusterobservability/controllers/placementrule/manifestwork.go @@ -326,7 +326,6 @@ func createManifestWorks( container.Env[j].Value = strconv.FormatBool(installProm) } } - // If ProxyConfig is specified as part of addonConfig, set the proxy envs if clusterName != localClusterName { for i := range spec.Containers { diff --git a/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go b/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go index abe3d4985..318f015f7 100644 --- a/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go +++ b/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go @@ -539,6 +539,9 @@ func (r *PlacementRuleReconciler) SetupWithManager(mgr ctrl.Manager) error { clusterPred := getClusterPreds() + // Watch changes for AddonDeploymentConfig + AddonDeploymentPred := GetAddOnDeploymentPredicates() + obsAddonPred := predicate.Funcs{ CreateFunc: func(e event.CreateEvent) bool { return false @@ -829,6 +832,20 @@ func (r *PlacementRuleReconciler) SetupWithManager(mgr ctrl.Manager) error { // secondary watch for alertmanager accessor serviceaccount Watches(&source.Kind{Type: &corev1.ServiceAccount{}}, &handler.EnqueueRequestForObject{}, builder.WithPredicates(amAccessorSAPred)) + // watch for AddonDeploymentConfig + if _, err := r.RESTMapper.RESTMapping(schema.GroupKind{Group: addonv1alpha1.GroupVersion.Group, Kind: "AddOnDeploymentConfig"}, addonv1alpha1.GroupVersion.Version); err == nil { + ctrBuilder = ctrBuilder.Watches( + &source.Kind{Type: &addonv1alpha1.AddOnDeploymentConfig{}}, + handler.EnqueueRequestsFromMapFunc(func(obj client.Object) []reconcile.Request { + return []reconcile.Request{ + {NamespacedName: types.NamespacedName{ + Name: config.AddonDeploymentConfigUpdateName, + }}, + } + }), + builder.WithPredicates(AddonDeploymentPred), + ) + } manifestWorkGroupKind := schema.GroupKind{Group: workv1.GroupVersion.Group, Kind: "ManifestWork"} if _, err := r.RESTMapper.RESTMapping(manifestWorkGroupKind, workv1.GroupVersion.Version); err == nil { workPred := getManifestworkPred() @@ -937,7 +954,8 @@ func StartPlacementController(mgr manager.Manager, crdMap map[string]bool) error func isReconcileRequired(request ctrl.Request, managedCluster string) bool { if request.Name == config.MCOUpdatedRequestName || request.Name == config.MCHUpdatedRequestName || - request.Name == config.ClusterManagementAddOnUpdateName { + request.Name == config.ClusterManagementAddOnUpdateName || + request.Name == config.AddonDeploymentConfigUpdateName { return true } if request.Namespace == config.GetDefaultNamespace() || diff --git a/operators/multiclusterobservability/controllers/placementrule/placementrule_controller_test.go b/operators/multiclusterobservability/controllers/placementrule/placementrule_controller_test.go index d9f54b721..48f6e30e1 100644 --- a/operators/multiclusterobservability/controllers/placementrule/placementrule_controller_test.go +++ b/operators/multiclusterobservability/controllers/placementrule/placementrule_controller_test.go @@ -11,6 +11,8 @@ import ( "strings" "testing" + appsv1 "k8s.io/api/apps/v1" + ocinfrav1 "github.com/openshift/api/config/v1" operatorv1 "github.com/openshift/api/operator/v1" routev1 "github.com/openshift/api/route/v1" @@ -177,7 +179,7 @@ func TestObservabilityAddonController(t *testing.T) { }, } objs := []runtime.Object{mco, pull, newConsoleRoute(), newTestObsApiRoute(), newTestAlertmanagerRoute(), newTestIngressController(), newTestRouteCASecret(), newCASecret(), newCertSecret(mcoNamespace), NewMetricsAllowListCM(), - NewAmAccessorSA(), NewAmAccessorTokenSecret(), newManagedClusterAddon(), deprecatedRole, newClusterMgmtAddon(), + NewAmAccessorSA(), NewAmAccessorTokenSecret(), deprecatedRole, newClusterMgmtAddon(), newAddonDeploymentConfig(defaultAddonConfigName, namespace), newAddonDeploymentConfig(addonConfigName, namespace)} c := fake.NewClientBuilder().WithRuntimeObjects(objs...).Build() r := &PlacementRuleReconciler{Client: c, Scheme: s, CRDMap: map[string]bool{config.IngressControllerCRD: true}} @@ -233,6 +235,68 @@ func TestObservabilityAddonController(t *testing.T) { if err != nil { t.Fatalf("reconcile: (%v)", err) } + foundAddonDeploymentConfig := &addonv1alpha1.AddOnDeploymentConfig{} + err = c.Get(context.TODO(), types.NamespacedName{Namespace: namespace, Name: defaultAddonConfigName}, foundAddonDeploymentConfig) + if err != nil { + t.Fatalf("Failed to get addondeploymentconfig %s: (%v)", name, err) + } + + //Change proxyconfig in addondeploymentconfig + foundAddonDeploymentConfig.Spec.ProxyConfig = addonv1alpha1.ProxyConfig{ + HTTPProxy: "http://test1.com", + HTTPSProxy: "https://test1.com", + NoProxy: "test.com", + } + + err = c.Update(context.TODO(), foundAddonDeploymentConfig) + if err != nil { + t.Fatalf("Failed to update addondeploymentconfig %s: (%v)", name, err) + } + + req = ctrl.Request{ + NamespacedName: types.NamespacedName{ + Name: config.AddonDeploymentConfigUpdateName, + }, + } + + _, err = r.Reconcile(context.TODO(), req) + if err != nil { + t.Fatalf("reconcile after updating addondeploymentconfig: (%v)", err) + } + + foundManifestwork := &workv1.ManifestWork{} + err = c.Get(context.TODO(), types.NamespacedName{Name: namespace + workNameSuffix, Namespace: namespace}, foundManifestwork) + if err != nil { + t.Fatalf("Failed to get manifestwork %s: (%v)", namespace, err) + } + for _, manifest := range foundManifestwork.Spec.Workload.Manifests { + obj, _ := util.GetObject(manifest.RawExtension) + if obj.GetObjectKind().GroupVersionKind().Kind == "Deployment" { + //Check the proxy env variables + deployment := obj.(*appsv1.Deployment) + spec := deployment.Spec.Template.Spec + for _, c := range spec.Containers { + if c.Name == "endpoint-observability-operator" { + env := c.Env + for _, e := range env { + if e.Name == "HTTP_PROXY" { + if e.Value != "http://test1.com" { + t.Fatalf("HTTP_PROXY is not set correctly: expected %s, got %s", "http://test1.com", e.Value) + } + } else if e.Name == "HTTPS_PROXY" { + if e.Value != "https://test1.com" { + t.Fatalf("HTTPS_PROXY is not set correctly: expected %s, got %s", "https://test1.com", e.Value) + } + } else if e.Name == "NO_PROXY" { + if e.Value != "test.com" { + t.Fatalf("NO_PROXY is not set correctly: expected %s, got %s", "test.com", e.Value) + } + } + } + } + } + } + } err = c.Delete(context.TODO(), mco) if err != nil { @@ -310,7 +374,7 @@ func TestObservabilityAddonController(t *testing.T) { // test mco-disable-alerting annotation // 1. Verify that alertmanager-endpoint in secret hub-info-secret in the ManifestWork is not null t.Logf("check alertmanager endpoint is not null") - foundManifestwork := &workv1.ManifestWork{} + foundManifestwork = &workv1.ManifestWork{} err = c.Get(context.TODO(), types.NamespacedName{Name: namespace + workNameSuffix, Namespace: namespace}, foundManifestwork) if err != nil { t.Fatalf("Failed to get manifestwork %s: (%v)", namespace, err) @@ -553,6 +617,11 @@ func newAddonDeploymentConfig(name, namespace string) *addonv1alpha1.AddOnDeploy "kubernetes.io/os": "linux", }, }, + ProxyConfig: addonv1alpha1.ProxyConfig{ + HTTPProxy: "http://foo.com", + HTTPSProxy: "https://foo.com", + NoProxy: "bar.com", + }, }, } } diff --git a/operators/multiclusterobservability/controllers/placementrule/predicate_func.go b/operators/multiclusterobservability/controllers/placementrule/predicate_func.go index c6346d64b..a19af143f 100644 --- a/operators/multiclusterobservability/controllers/placementrule/predicate_func.go +++ b/operators/multiclusterobservability/controllers/placementrule/predicate_func.go @@ -5,6 +5,9 @@ package placementrule import ( + "reflect" + + addonv1alpha1 "open-cluster-management.io/api/addon/v1alpha1" "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/predicate" ) @@ -73,3 +76,22 @@ func getClusterPreds() predicate.Funcs { DeleteFunc: deleteFunc, } } + +func GetAddOnDeploymentPredicates() predicate.Funcs { + return predicate.Funcs{ + CreateFunc: func(e event.CreateEvent) bool { + return true + }, + UpdateFunc: func(e event.UpdateEvent) bool { + if !reflect.DeepEqual(e.ObjectNew.(*addonv1alpha1.AddOnDeploymentConfig).Spec.ProxyConfig, + e.ObjectOld.(*addonv1alpha1.AddOnDeploymentConfig).Spec.ProxyConfig) { + log.Info("AddonDeploymentConfig is updated", e.ObjectNew.GetName(), "name", e.ObjectNew.GetNamespace(), "namespace") + return true + } + return false + }, + DeleteFunc: func(e event.DeleteEvent) bool { + return true + }, + } +} diff --git a/operators/multiclusterobservability/controllers/placementrule/predicate_func_test.go b/operators/multiclusterobservability/controllers/placementrule/predicate_func_test.go index bfa6a751e..6ae231f98 100644 --- a/operators/multiclusterobservability/controllers/placementrule/predicate_func_test.go +++ b/operators/multiclusterobservability/controllers/placementrule/predicate_func_test.go @@ -8,6 +8,8 @@ import ( "testing" "time" + addonv1alpha1 "open-cluster-management.io/api/addon/v1alpha1" + appsv1 "k8s.io/api/apps/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/event" @@ -146,3 +148,71 @@ func TestClusterPred(t *testing.T) { }) } } + +func TestAddonDeploymentPredicate(t *testing.T) { + name := "test-obj" + caseList := []struct { + caseName string + namespace string + expectedCreate bool + expectedUpdate bool + expectedDelete bool + }{ + { + caseName: "Create AddonDeploymentConfig", + namespace: testNamespace, + expectedCreate: true, + expectedDelete: true, + expectedUpdate: true, + }, + } + + defaultAddonDeploymentConfig = &addonv1alpha1.AddOnDeploymentConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: addonv1alpha1.AddOnDeploymentConfigSpec{ + ProxyConfig: addonv1alpha1.ProxyConfig{ + HTTPProxy: "http://foo.com", + HTTPSProxy: "https://foo.com", + NoProxy: "bar.com", + }, + }, + } + for _, c := range caseList { + t.Run(c.caseName, func(t *testing.T) { + pred := GetAddOnDeploymentPredicates() + createEvent := event.CreateEvent{ + Object: defaultAddonDeploymentConfig, + } + + if c.expectedCreate { + if !pred.CreateFunc(createEvent) { + t.Fatalf("pre func return false on applied createevent in case: (%v)", c.caseName) + } + } + + newDefaultAddonDeploymentConfig := defaultAddonDeploymentConfig.DeepCopy() + newDefaultAddonDeploymentConfig.Spec.ProxyConfig.HTTPProxy = "http://bar1.com" + updateEvent := event.UpdateEvent{ + ObjectOld: defaultAddonDeploymentConfig, + ObjectNew: newDefaultAddonDeploymentConfig, + } + if c.expectedUpdate { + if !pred.UpdateFunc(updateEvent) { + t.Fatalf("pre func return false on applied update event in case: (%v)", c.caseName) + } + } + + deleteEvent := event.DeleteEvent{ + Object: defaultAddonDeploymentConfig, + } + if c.expectedDelete { + if !pred.DeleteFunc(deleteEvent) { + t.Fatalf("pre func return false on applied delete event in case: (%v)", c.caseName) + } + } + }) + } +} diff --git a/operators/multiclusterobservability/pkg/config/config.go b/operators/multiclusterobservability/pkg/config/config.go index 7f9efe9da..d1440bc7e 100644 --- a/operators/multiclusterobservability/pkg/config/config.go +++ b/operators/multiclusterobservability/pkg/config/config.go @@ -56,6 +56,7 @@ const ( MCHUpdatedRequestName = "mch-updated-request" MCOUpdatedRequestName = "mco-updated-request" ClusterManagementAddOnUpdateName = "clustermgmtaddon-updated-request" + AddonDeploymentConfigUpdateName = "addondc-updated-request" MulticloudConsoleRouteName = "multicloud-console" ImageManifestConfigMapNamePrefix = "mch-image-manifest-" OCMManifestConfigMapTypeLabelKey = "ocm-configmap-type"