From f1e5b82525a02885453476612f69285fd046cdac Mon Sep 17 00:00:00 2001 From: Coleen Iona Quadros Date: Fri, 1 Dec 2023 16:53:26 +0100 Subject: [PATCH 1/6] add watch logic for addondeploymentconfig Signed-off-by: Coleen Iona Quadros --- .../controllers/placementrule/manifestwork.go | 1 + .../placementrule/placementrule_controller.go | 42 +++++++++++++++++++ .../pkg/config/config.go | 1 + 3 files changed, 44 insertions(+) diff --git a/operators/multiclusterobservability/controllers/placementrule/manifestwork.go b/operators/multiclusterobservability/controllers/placementrule/manifestwork.go index c85289f27..71643218d 100644 --- a/operators/multiclusterobservability/controllers/placementrule/manifestwork.go +++ b/operators/multiclusterobservability/controllers/placementrule/manifestwork.go @@ -326,6 +326,7 @@ func createManifestWorks( } // If ProxyConfig is specified as part of addonConfig, set the proxy envs + //Coleen shouldnt this be out of th loop if clusterName != localClusterName { for i := range spec.Containers { container := &spec.Containers[i] diff --git a/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go b/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go index abe3d4985..d6189aefa 100644 --- a/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go +++ b/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go @@ -539,6 +539,34 @@ func (r *PlacementRuleReconciler) SetupWithManager(mgr ctrl.Manager) error { clusterPred := getClusterPreds() + // Watch chnages for AddonDeploymentConfig + AddonDeploymentPred := predicate.Funcs{ + CreateFunc: func(e event.CreateEvent) bool { + if e.Object.GetName() == defaultAddonDeploymentConfig.Name && + e.Object.GetNamespace() == defaultAddonDeploymentConfig.Namespace { + log.Info("default AddonDeploymentConfig is created") + return true + } + return false + }, + UpdateFunc: func(e event.UpdateEvent) bool { + if e.ObjectNew.GetName() == defaultAddonDeploymentConfig.Name && + e.ObjectNew.GetNamespace() == defaultAddonDeploymentConfig.Namespace { + log.Info("default AddonDeploymentConfig is updated") + return true + } + return false + }, + DeleteFunc: func(e event.DeleteEvent) bool { + if e.Object.GetName() == defaultAddonDeploymentConfig.Name && + e.Object.GetNamespace() == defaultAddonDeploymentConfig.Namespace { + log.Info("default AddonDeploymentConfig is deleted") + return true + } + return false + }, + } + obsAddonPred := predicate.Funcs{ CreateFunc: func(e event.CreateEvent) bool { return false @@ -829,6 +857,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() 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" From 3e62d105a8085e458cfc50a895bb2aa61290a58c Mon Sep 17 00:00:00 2001 From: Coleen Iona Quadros Date: Fri, 1 Dec 2023 16:59:09 +0100 Subject: [PATCH 2/6] lint Signed-off-by: Coleen Iona Quadros --- .../controllers/placementrule/placementrule_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go b/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go index d6189aefa..a43aaaf78 100644 --- a/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go +++ b/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go @@ -539,7 +539,7 @@ func (r *PlacementRuleReconciler) SetupWithManager(mgr ctrl.Manager) error { clusterPred := getClusterPreds() - // Watch chnages for AddonDeploymentConfig + // Watch changes for AddonDeploymentConfig AddonDeploymentPred := predicate.Funcs{ CreateFunc: func(e event.CreateEvent) bool { if e.Object.GetName() == defaultAddonDeploymentConfig.Name && From ff1e8c237b7223b4e334dcc15d311cd8756470b8 Mon Sep 17 00:00:00 2001 From: Coleen Iona Quadros Date: Fri, 1 Dec 2023 21:49:29 +0100 Subject: [PATCH 3/6] enable reconcile Signed-off-by: Coleen Iona Quadros --- .../controllers/placementrule/placementrule_controller.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go b/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go index a43aaaf78..3c1b21b1a 100644 --- a/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go +++ b/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go @@ -979,7 +979,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() || From f2e9ad10a0846565f1c27315b6fc92c61d54efe3 Mon Sep 17 00:00:00 2001 From: Coleen Iona Quadros Date: Tue, 5 Dec 2023 14:34:22 +0100 Subject: [PATCH 4/6] add test case Signed-off-by: Coleen Iona Quadros --- .../controllers/placementrule/manifestwork.go | 52 ++++++------- .../placementrule/placementrule_controller.go | 12 +-- .../placementrule_controller_test.go | 78 ++++++++++++++++++- .../pkg/util/clustermanagementaddon.go | 1 + 4 files changed, 109 insertions(+), 34 deletions(-) diff --git a/operators/multiclusterobservability/controllers/placementrule/manifestwork.go b/operators/multiclusterobservability/controllers/placementrule/manifestwork.go index 71643218d..b8d185664 100644 --- a/operators/multiclusterobservability/controllers/placementrule/manifestwork.go +++ b/operators/multiclusterobservability/controllers/placementrule/manifestwork.go @@ -325,32 +325,6 @@ func createManifestWorks( } } - // If ProxyConfig is specified as part of addonConfig, set the proxy envs - //Coleen shouldnt this be out of th loop - if clusterName != localClusterName { - for i := range spec.Containers { - container := &spec.Containers[i] - if addonConfig.Spec.ProxyConfig.HTTPProxy != "" { - container.Env = append(container.Env, corev1.EnvVar{ - Name: "HTTP_PROXY", - Value: addonConfig.Spec.ProxyConfig.HTTPProxy, - }) - } - if addonConfig.Spec.ProxyConfig.HTTPSProxy != "" { - container.Env = append(container.Env, corev1.EnvVar{ - Name: "HTTPS_PROXY", - Value: addonConfig.Spec.ProxyConfig.HTTPSProxy, - }) - } - if addonConfig.Spec.ProxyConfig.NoProxy != "" { - container.Env = append(container.Env, corev1.EnvVar{ - Name: "NO_PROXY", - Value: addonConfig.Spec.ProxyConfig.NoProxy, - }) - } - } - } - if hasCustomRegistry { oldImage := container.Image newImage, err := imageRegistryClient.Cluster(clusterName).ImageOverride(oldImage) @@ -361,6 +335,32 @@ func createManifestWorks( } } } + for i := range spec.Containers { + if spec.Containers[i].Name == "endpoint-observability-operator" { + container := &spec.Containers[i] + + if clusterName != localClusterName { + if addonConfig.Spec.ProxyConfig.HTTPProxy != "" { + container.Env = append(container.Env, corev1.EnvVar{ + Name: "HTTP_PROXY", + Value: addonConfig.Spec.ProxyConfig.HTTPProxy, + }) + } + if addonConfig.Spec.ProxyConfig.HTTPSProxy != "" { + container.Env = append(container.Env, corev1.EnvVar{ + Name: "HTTPS_PROXY", + Value: addonConfig.Spec.ProxyConfig.HTTPSProxy, + }) + } + if addonConfig.Spec.ProxyConfig.NoProxy != "" { + container.Env = append(container.Env, corev1.EnvVar{ + Name: "NO_PROXY", + Value: addonConfig.Spec.ProxyConfig.NoProxy, + }) + } + } + } + } log.Info(fmt.Sprintf("Cluster: %+v, Spec.NodeSelector (after): %+v", clusterName, spec.NodeSelector)) log.Info(fmt.Sprintf("Cluster: %+v, Spec.Tolerations (after): %+v", clusterName, spec.Tolerations)) dep.Spec.Template.Spec = spec diff --git a/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go b/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go index 3c1b21b1a..952b3b822 100644 --- a/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go +++ b/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go @@ -542,12 +542,12 @@ func (r *PlacementRuleReconciler) SetupWithManager(mgr ctrl.Manager) error { // Watch changes for AddonDeploymentConfig AddonDeploymentPred := predicate.Funcs{ CreateFunc: func(e event.CreateEvent) bool { - if e.Object.GetName() == defaultAddonDeploymentConfig.Name && - e.Object.GetNamespace() == defaultAddonDeploymentConfig.Namespace { - log.Info("default AddonDeploymentConfig is created") - return true - } - return false + //if e.Object.GetName() == defaultAddonDeploymentConfig.Name && + // e.Object.GetNamespace() == defaultAddonDeploymentConfig.Namespace { + // log.Info("default AddonDeploymentConfig is created") + // return true + //} + return true }, UpdateFunc: func(e event.UpdateEvent) bool { if e.ObjectNew.GetName() == defaultAddonDeploymentConfig.Name && diff --git a/operators/multiclusterobservability/controllers/placementrule/placementrule_controller_test.go b/operators/multiclusterobservability/controllers/placementrule/placementrule_controller_test.go index d9f54b721..b9e800346 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}} @@ -234,6 +236,73 @@ func TestObservabilityAddonController(t *testing.T) { 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) + } + + _, err = r.Reconcile(context.TODO(), req) + if err != nil { + t.Fatalf("reconcile after updating addondeploymentconfig: (%v)", 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 { t.Fatalf("Failed to delete mco: (%v)", err) @@ -310,7 +379,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 +622,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/pkg/util/clustermanagementaddon.go b/operators/multiclusterobservability/pkg/util/clustermanagementaddon.go index 20bd844a3..d49bda514 100644 --- a/operators/multiclusterobservability/pkg/util/clustermanagementaddon.go +++ b/operators/multiclusterobservability/pkg/util/clustermanagementaddon.go @@ -40,6 +40,7 @@ func CreateClusterManagementAddon(c client.Client) ( found := &addonv1alpha1.ClusterManagementAddOn{} err = c.Get(context.TODO(), types.NamespacedName{Name: ObservabilityController}, found) if err != nil && errors.IsNotFound(err) { + if err := c.Create(context.TODO(), clusterManagementAddon); err != nil { log.Error(err, "Failed to create observability-controller clustermanagementaddon ") return nil, err From 42ed8e3037ba52d231ee10d72a5b589010cfc54c Mon Sep 17 00:00:00 2001 From: Coleen Iona Quadros Date: Tue, 5 Dec 2023 14:40:36 +0100 Subject: [PATCH 5/6] remove extra reconcile Signed-off-by: Coleen Iona Quadros --- .../placementrule/placementrule_controller_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/operators/multiclusterobservability/controllers/placementrule/placementrule_controller_test.go b/operators/multiclusterobservability/controllers/placementrule/placementrule_controller_test.go index b9e800346..e11113373 100644 --- a/operators/multiclusterobservability/controllers/placementrule/placementrule_controller_test.go +++ b/operators/multiclusterobservability/controllers/placementrule/placementrule_controller_test.go @@ -242,10 +242,10 @@ func TestObservabilityAddonController(t *testing.T) { t.Fatalf("Failed to get addondeploymentconfig %s: (%v)", name, err) } - _, err = r.Reconcile(context.TODO(), req) - if err != nil { - t.Fatalf("reconcile after updating addondeploymentconfig: (%v)", err) - } + //_, err = r.Reconcile(context.TODO(), req) + //if err != nil { + // t.Fatalf("reconcile after updating addondeploymentconfig: (%v)", err) + //} //Change proxyconfig in addondeploymentconfig foundAddonDeploymentConfig.Spec.ProxyConfig = addonv1alpha1.ProxyConfig{ HTTPProxy: "http://test1.com", From 47266f4a28100d49aa9919d23f06d05aae6f7c5e Mon Sep 17 00:00:00 2001 From: Coleen Iona Quadros Date: Tue, 5 Dec 2023 14:47:33 +0100 Subject: [PATCH 6/6] remove comment Signed-off-by: Coleen Iona Quadros --- .../placementrule/placementrule_controller_test.go | 4 ---- operators/multiclusterobservability/tests/manifests | 1 + 2 files changed, 1 insertion(+), 4 deletions(-) create mode 120000 operators/multiclusterobservability/tests/manifests diff --git a/operators/multiclusterobservability/controllers/placementrule/placementrule_controller_test.go b/operators/multiclusterobservability/controllers/placementrule/placementrule_controller_test.go index e11113373..7c884db3a 100644 --- a/operators/multiclusterobservability/controllers/placementrule/placementrule_controller_test.go +++ b/operators/multiclusterobservability/controllers/placementrule/placementrule_controller_test.go @@ -242,10 +242,6 @@ func TestObservabilityAddonController(t *testing.T) { t.Fatalf("Failed to get addondeploymentconfig %s: (%v)", name, err) } - //_, err = r.Reconcile(context.TODO(), req) - //if err != nil { - // t.Fatalf("reconcile after updating addondeploymentconfig: (%v)", err) - //} //Change proxyconfig in addondeploymentconfig foundAddonDeploymentConfig.Spec.ProxyConfig = addonv1alpha1.ProxyConfig{ HTTPProxy: "http://test1.com", diff --git a/operators/multiclusterobservability/tests/manifests b/operators/multiclusterobservability/tests/manifests new file mode 120000 index 000000000..f6ae72e90 --- /dev/null +++ b/operators/multiclusterobservability/tests/manifests @@ -0,0 +1 @@ +/Users/coquadro/work/multicluster-observability-operator/operators/multiclusterobservability/manifests \ No newline at end of file