diff --git a/operators/multiclusterobservability/api/shared/multiclusterobservability_shared.go b/operators/multiclusterobservability/api/shared/multiclusterobservability_shared.go index e06575070b..7a79487c2b 100644 --- a/operators/multiclusterobservability/api/shared/multiclusterobservability_shared.go +++ b/operators/multiclusterobservability/api/shared/multiclusterobservability_shared.go @@ -9,10 +9,22 @@ package shared import ( + "net/url" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +// URL is kubebuilder type that validates the containing string is an URL. +// +kubebuilder:validation:Pattern=`^https?:\/\/` +// +kubebuilder:validation:MaxLength=2083 +type URL string + +func (u URL) Validate() error { + _, err := url.Parse(string(u)) + return err +} + // ObservabilityAddonSpec is the spec of observability addon. type ObservabilityAddonSpec struct { // EnableMetrics indicates the observability addon push metrics to hub server. diff --git a/operators/multiclusterobservability/api/v1beta2/multiclusterobservability_types.go b/operators/multiclusterobservability/api/v1beta2/multiclusterobservability_types.go index f125a017fc..20d2573393 100644 --- a/operators/multiclusterobservability/api/v1beta2/multiclusterobservability_types.go +++ b/operators/multiclusterobservability/api/v1beta2/multiclusterobservability_types.go @@ -42,6 +42,11 @@ type MultiClusterObservabilitySpec struct { } type AdvancedConfig struct { + // CustomObservabilityHubURL overrides the endpoint used by the metrics-collector to send + // metrics to the hub server. + // For the metrics-collector that runs in the hub this setting has no effect. + // +optional + CustomObservabilityHubURL observabilityshared.URL `json:"customObservabilityHubURL,omitempty"` // The spec of the data retention configurations // +optional RetentionConfig *RetentionConfig `json:"retentionConfig,omitempty"` diff --git a/operators/multiclusterobservability/bundle/manifests/observability.open-cluster-management.io_multiclusterobservabilities.yaml b/operators/multiclusterobservability/bundle/manifests/observability.open-cluster-management.io_multiclusterobservabilities.yaml index be086e562e..004f881f9e 100644 --- a/operators/multiclusterobservability/bundle/manifests/observability.open-cluster-management.io_multiclusterobservabilities.yaml +++ b/operators/multiclusterobservability/bundle/manifests/observability.open-cluster-management.io_multiclusterobservabilities.yaml @@ -1148,6 +1148,11 @@ spec: description: Annotations is an unstructured key value map stored with a service account type: object type: object + customObservabilityHubURL: + description: CustomObservabilityHubURL overrides the endpoint used by the metrics-collector to send metrics to the hub server. For the metrics-collector that runs in the hub this setting has no effect. + maxLength: 2083 + pattern: ^https?:\/\/ + type: string grafana: description: The spec of grafana properties: diff --git a/operators/multiclusterobservability/config/crd/bases/observability.open-cluster-management.io_multiclusterobservabilities.yaml b/operators/multiclusterobservability/config/crd/bases/observability.open-cluster-management.io_multiclusterobservabilities.yaml index 3c5aee75b5..4092422786 100644 --- a/operators/multiclusterobservability/config/crd/bases/observability.open-cluster-management.io_multiclusterobservabilities.yaml +++ b/operators/multiclusterobservability/config/crd/bases/observability.open-cluster-management.io_multiclusterobservabilities.yaml @@ -1761,6 +1761,14 @@ spec: stored with a service account type: object type: object + customObservabilityHubURL: + description: CustomObservabilityHubURL overrides the endpoint + used by the metrics-collector to send metrics to the hub server. + For the metrics-collector that runs in the hub this setting + has no effect. + maxLength: 2083 + pattern: ^https?:\/\/ + type: string grafana: description: The spec of grafana properties: diff --git a/operators/multiclusterobservability/controllers/multiclusterobservability/grafana.go b/operators/multiclusterobservability/controllers/multiclusterobservability/grafana.go index 271b3b3f3a..c50107253d 100644 --- a/operators/multiclusterobservability/controllers/multiclusterobservability/grafana.go +++ b/operators/multiclusterobservability/controllers/multiclusterobservability/grafana.go @@ -8,6 +8,7 @@ import ( "bytes" "context" "fmt" + "reflect" oauthv1 "github.com/openshift/api/oauth/v1" routev1 "github.com/openshift/api/route/v1" @@ -31,6 +32,9 @@ const ( defaultReplicas int32 = 1 restartLabel = "datasource/time-restarted" datasourceKey = "datasources.yaml" + + haProxyRouterTimeoutKey = "haproxy.router.openshift.io/timeout" + defaultHaProxyRouterTimeout = "300s" ) type GrafanaDatasources struct { @@ -194,7 +198,7 @@ func GenerateGrafanaRoute( Name: config.GrafanaRouteName, Namespace: config.GetDefaultNamespace(), Annotations: map[string]string{ - "haproxy.router.openshift.io/timeout": "300s", + haProxyRouterTimeoutKey: defaultHaProxyRouterTimeout, }, }, Spec: routev1.RouteSpec{ @@ -237,6 +241,34 @@ func GenerateGrafanaRoute( } return nil, nil } + + // if no annotations are set, set the default timeout + if found.Annotations == nil { + found.Annotations = map[string]string{} + found.Annotations[haProxyRouterTimeoutKey] = defaultHaProxyRouterTimeout + } + + // if some annotations are set, but the timeout is not set, set the default timeout + // otherwise, use the existing timeout which allows for custom timeouts. + // we do not want to overwrite other labels that may be set. + if _, ok := found.Annotations[haProxyRouterTimeoutKey]; !ok { + found.Annotations[haProxyRouterTimeoutKey] = defaultHaProxyRouterTimeout + } + + if !reflect.DeepEqual(found.Spec, grafanaRoute.Spec) { + found.Spec = grafanaRoute.Spec + } + + err = c.Update(context.TODO(), found) + if err != nil { + log.Error( + err, + "failed update for Grafana Route", + "grafanaRoute.Name", + grafanaRoute.Name, + ) + return &ctrl.Result{}, err + } return nil, nil } diff --git a/operators/multiclusterobservability/controllers/multiclusterobservability/grafana_test.go b/operators/multiclusterobservability/controllers/multiclusterobservability/grafana_test.go index d278b737d6..3bcd0d8014 100644 --- a/operators/multiclusterobservability/controllers/multiclusterobservability/grafana_test.go +++ b/operators/multiclusterobservability/controllers/multiclusterobservability/grafana_test.go @@ -5,21 +5,130 @@ package multiclusterobservability import ( + "context" + "reflect" "testing" + + routev1 "github.com/openshift/api/route/v1" + mcov1beta2 "github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/api/v1beta2" + "github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/pkg/config" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/client-go/kubernetes/scheme" + + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" ) -func TestUpdateGrafanaSpec(t *testing.T) { - // mco := &mcov1beta2.MultiClusterObservability{ - // Spec: mcov1beta2.MultiClusterObservabilitySpec{ - // Grafana: &mcov1beta2.GrafanaSpec{ - // Hostport: defaultHostport, - // }, - // }, - // } +func TestGenerateGrafanaRoute(t *testing.T) { + instance := &mcov1beta2.MultiClusterObservability{} + s := scheme.Scheme + s.AddKnownTypes(mcov1beta2.GroupVersion) + if err := mcov1beta2.AddToScheme(s); err != nil { + t.Fatalf("Unable to add scheme: (%v)", err) + } - // updateGrafanaConfig(mco) + clientScheme := runtime.NewScheme() + if err := routev1.AddToScheme(clientScheme); err != nil { + t.Fatalf("Unable to add route scheme: (%v)", err) + } - // if mco.Spec.Grafana.Replicas != 1 { - // t.Errorf("Replicas (%v) is not the expected (%v)", mco.Spec.Grafana.Replicas, defaultReplicas) - // } + tests := []struct { + name string + want routev1.Route + c client.WithWatch + }{ + { + name: "Test create a Route if it does not exist", + want: routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: config.GrafanaRouteName, + Namespace: config.GetDefaultNamespace(), + Annotations: map[string]string{ + haProxyRouterTimeoutKey: defaultHaProxyRouterTimeout, + }, + }, + Spec: routev1.RouteSpec{ + Port: &routev1.RoutePort{ + TargetPort: intstr.FromString("oauth-proxy"), + }, + To: routev1.RouteTargetReference{ + Kind: "Service", + Name: config.GrafanaServiceName, + }, + TLS: &routev1.TLSConfig{ + Termination: routev1.TLSTerminationReencrypt, + InsecureEdgeTerminationPolicy: routev1.InsecureEdgeTerminationPolicyRedirect, + }, + }, + }, + c: fake.NewClientBuilder().WithScheme(clientScheme).Build(), + }, + { + name: "Test update a Route if it has been modified", + want: routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: config.GrafanaRouteName, + Namespace: config.GetDefaultNamespace(), + Annotations: map[string]string{ + haProxyRouterTimeoutKey: defaultHaProxyRouterTimeout, + }, + }, + Spec: routev1.RouteSpec{ + Port: &routev1.RoutePort{ + TargetPort: intstr.FromString("oauth-proxy"), + }, + To: routev1.RouteTargetReference{ + Kind: "Service", + Name: config.GrafanaServiceName, + }, + TLS: &routev1.TLSConfig{ + Termination: routev1.TLSTerminationReencrypt, + InsecureEdgeTerminationPolicy: routev1.InsecureEdgeTerminationPolicyRedirect, + }, + }, + }, + c: fake.NewClientBuilder().WithScheme(clientScheme).WithObjects(&routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: config.GrafanaRouteName, + Namespace: config.GetDefaultNamespace(), + Annotations: map[string]string{}, + }, + Spec: routev1.RouteSpec{ + Port: &routev1.RoutePort{ + TargetPort: intstr.FromString("oauth-proxy"), + }, + To: routev1.RouteTargetReference{ + Kind: "Service", + Name: "modified", + }, + TLS: &routev1.TLSConfig{ + Termination: routev1.TLSTerminationReencrypt, + InsecureEdgeTerminationPolicy: routev1.InsecureEdgeTerminationPolicyRedirect, + }, + }, + }).Build(), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := GenerateGrafanaRoute(tt.c, s, instance) + if err != nil { + t.Errorf("GenerateGrafanaDataSource() error = %v", err) + return + } + list := &routev1.RouteList{} + if err := tt.c.List(context.Background(), list); err != nil { + t.Fatalf("Unable to list routes: (%v)", err) + } + if len(list.Items) != 1 { + t.Fatalf("Expected 1 route, got %d", len(list.Items)) + } + if !reflect.DeepEqual(list.Items[0].Spec, tt.want.Spec) { + t.Fatalf("Expected route spec: %v, got %v", tt.want.Spec, list.Items[0].Spec) + } + }) + } } diff --git a/operators/multiclusterobservability/controllers/multiclusterobservability/multiclusterobservability_controller.go b/operators/multiclusterobservability/controllers/multiclusterobservability/multiclusterobservability_controller.go index fa7962bd22..dfc499a715 100644 --- a/operators/multiclusterobservability/controllers/multiclusterobservability/multiclusterobservability_controller.go +++ b/operators/multiclusterobservability/controllers/multiclusterobservability/multiclusterobservability_controller.go @@ -300,7 +300,7 @@ func (r *MultiClusterObservabilityReconciler) Reconcile(ctx context.Context, req } // expose observatorium api gateway - result, err = GenerateAPIGatewayRoute(r.Client, r.Scheme, instance) + result, err = GenerateAPIGatewayRoute(ctx, r.Client, r.Scheme, instance) if result != nil { return *result, err } diff --git a/operators/multiclusterobservability/controllers/multiclusterobservability/multiclusterobservability_controller_test.go b/operators/multiclusterobservability/controllers/multiclusterobservability/multiclusterobservability_controller_test.go index 9d4726d92d..042e53dd77 100644 --- a/operators/multiclusterobservability/controllers/multiclusterobservability/multiclusterobservability_controller_test.go +++ b/operators/multiclusterobservability/controllers/multiclusterobservability/multiclusterobservability_controller_test.go @@ -12,12 +12,19 @@ import ( "testing" "time" - monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" - oauthv1 "github.com/openshift/api/oauth/v1" routev1 "github.com/openshift/api/route/v1" + monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" + + mcoshared "github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/api/shared" + mcov1beta2 "github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/api/v1beta2" + "github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/pkg/config" + "github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/pkg/rendering/templates" + mchv1 "github.com/stolostron/multiclusterhub-operator/api/v1" observatoriumv1alpha1 "github.com/stolostron/observatorium-operator/api/v1alpha1" + "gopkg.in/yaml.v2" + appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" @@ -27,20 +34,14 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/scheme" _ "k8s.io/client-go/plugin/pkg/client/auth/gcp" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client/fake" - "sigs.k8s.io/controller-runtime/pkg/log/zap" - migrationv1alpha1 "sigs.k8s.io/kube-storage-version-migrator/pkg/apis/migration/v1alpha1" - - mchv1 "github.com/stolostron/multiclusterhub-operator/api/v1" addonv1alpha1 "open-cluster-management.io/api/addon/v1alpha1" clusterv1 "open-cluster-management.io/api/cluster/v1" - mcoshared "github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/api/shared" - mcov1beta2 "github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/api/v1beta2" - "github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/pkg/config" - "github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/pkg/rendering/templates" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/log/zap" + migrationv1alpha1 "sigs.k8s.io/kube-storage-version-migrator/pkg/apis/migration/v1alpha1" ) func init() { diff --git a/operators/multiclusterobservability/controllers/multiclusterobservability/observatorium.go b/operators/multiclusterobservability/controllers/multiclusterobservability/observatorium.go index 4dc2ded057..9dfae3f859 100644 --- a/operators/multiclusterobservability/controllers/multiclusterobservability/observatorium.go +++ b/operators/multiclusterobservability/controllers/multiclusterobservability/observatorium.go @@ -47,7 +47,9 @@ const ( endpointsConfigName = "observability-remotewrite-endpoints" endpointsKey = "endpoints.yaml" - obsAPIGateway = "observatorium-api" + obsAPIGateway = "observatorium-api" + obsApiGatewayTargetPort = "public" + obsCRConfigHashLabelName = "config-hash" readOnlyRoleName = "read-only-metrics" @@ -252,6 +254,7 @@ func updateTenantID( // GenerateAPIGatewayRoute defines aaa func GenerateAPIGatewayRoute( + ctx context.Context, runclient client.Client, scheme *runtime.Scheme, mco *mcov1beta2.MultiClusterObservability) (*ctrl.Result, error) { @@ -262,11 +265,11 @@ func GenerateAPIGatewayRoute( }, Spec: routev1.RouteSpec{ Port: &routev1.RoutePort{ - TargetPort: intstr.FromString("public"), + TargetPort: intstr.FromString(obsApiGatewayTargetPort), }, To: routev1.RouteTargetReference{ Kind: "Service", - Name: mcoconfig.GetOperandNamePrefix() + "observatorium-api", + Name: mcoconfig.GetOperandNamePrefix() + obsAPIGateway, }, TLS: &routev1.TLSConfig{ Termination: routev1.TLSTerminationPassthrough, @@ -280,10 +283,8 @@ func GenerateAPIGatewayRoute( return &ctrl.Result{}, err } - err := runclient.Get( - context.TODO(), - types.NamespacedName{Name: apiGateway.Name, Namespace: apiGateway.Namespace}, - &routev1.Route{}) + found := &routev1.Route{} + err := runclient.Get(ctx, types.NamespacedName{Namespace: apiGateway.Namespace, Name: apiGateway.Name}, found) if err != nil && k8serrors.IsNotFound(err) { log.Info("Creating a new route to expose observatorium api", "apiGateway.Namespace", apiGateway.Namespace, @@ -293,6 +294,43 @@ func GenerateAPIGatewayRoute( if err != nil { return &ctrl.Result{}, err } + return nil, nil + } + + var needsUpdate bool + if found.Spec.TLS != nil { + if found.Spec.TLS.Termination != routev1.TLSTerminationPassthrough { + needsUpdate = true + found.Spec.TLS.Termination = routev1.TLSTerminationPassthrough + } + + if found.Spec.TLS.InsecureEdgeTerminationPolicy != routev1.InsecureEdgeTerminationPolicyNone { + needsUpdate = true + found.Spec.TLS.InsecureEdgeTerminationPolicy = routev1.InsecureEdgeTerminationPolicyNone + } + } + + if found.Spec.Port != nil && found.Spec.Port.TargetPort.String() != obsApiGatewayTargetPort { + needsUpdate = true + found.Spec.Port.TargetPort = intstr.FromString(obsApiGatewayTargetPort) + } + + if found.Spec.To.Name != mcoconfig.GetOperandNamePrefix()+obsAPIGateway { + needsUpdate = true + found.Spec.To.Name = mcoconfig.GetOperandNamePrefix() + obsAPIGateway + } + + if needsUpdate { + log.Info("Updating Route for observatorium api", + "apiGateway.Namespace", apiGateway.Namespace, + "apiGateway.Name", apiGateway.Name, + ) + err = runclient.Update(context.TODO(), found) + if err != nil { + log.Error(err, "failed update Route for observatorium api gateway", + "apiGateway.Name", apiGateway.Name) + return &ctrl.Result{}, err + } } return nil, nil diff --git a/operators/multiclusterobservability/controllers/multiclusterobservability/observatorium_test.go b/operators/multiclusterobservability/controllers/multiclusterobservability/observatorium_test.go index ecb52aa685..993a9e0064 100644 --- a/operators/multiclusterobservability/controllers/multiclusterobservability/observatorium_test.go +++ b/operators/multiclusterobservability/controllers/multiclusterobservability/observatorium_test.go @@ -11,22 +11,26 @@ import ( "reflect" "testing" + routev1 "github.com/openshift/api/route/v1" + + mcoshared "github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/api/shared" + mcov1beta2 "github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/api/v1beta2" + mcoconfig "github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/pkg/config" + mcoutil "github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/pkg/util" + observatoriumv1alpha1 "github.com/stolostron/observatorium-operator/api/v1alpha1" + "gopkg.in/yaml.v2" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/client-go/kubernetes/scheme" - "sigs.k8s.io/controller-runtime/pkg/client/fake" - - mcoshared "github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/api/shared" - mcov1beta2 "github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/api/v1beta2" - - observatoriumv1alpha1 "github.com/stolostron/observatorium-operator/api/v1alpha1" - mcoconfig "github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/pkg/config" - mcoutil "github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/pkg/util" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" ) var ( @@ -629,3 +633,94 @@ func TestObservatoriumCustomArgs(t *testing.T) { t.Errorf("Failed to propagate custom args to QueryFrontend Observatorium spec") } } + +func TestGenerateAPIGatewayRoute(t *testing.T) { + ctx := context.Background() + s := scheme.Scheme + s.AddKnownTypes(mcov1beta2.GroupVersion) + if err := mcov1beta2.AddToScheme(s); err != nil { + t.Fatalf("Unable to add scheme: (%v)", err) + } + + clientScheme := runtime.NewScheme() + if err := routev1.AddToScheme(clientScheme); err != nil { + t.Fatalf("Unable to add route scheme: (%v)", err) + } + + want := routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: obsAPIGateway, + Namespace: mcoconfig.GetDefaultNamespace(), + }, + Spec: routev1.RouteSpec{ + Port: &routev1.RoutePort{ + TargetPort: intstr.FromString(obsApiGatewayTargetPort), + }, + To: routev1.RouteTargetReference{ + Kind: "Service", + Name: mcoconfig.GetOperandNamePrefix() + obsAPIGateway, + }, + TLS: &routev1.TLSConfig{ + Termination: routev1.TLSTerminationPassthrough, + InsecureEdgeTerminationPolicy: routev1.InsecureEdgeTerminationPolicyNone, + }, + }, + } + + tests := []struct { + name string + want routev1.Route + c client.WithWatch + instance *mcov1beta2.MultiClusterObservability + }{ + { + name: "Test create a Route if it does not exist", + want: want, + c: fake.NewClientBuilder().WithScheme(clientScheme).Build(), + instance: &mcov1beta2.MultiClusterObservability{}, + }, + { + name: "Test update a Route if it has been modified", + want: want, + instance: &mcov1beta2.MultiClusterObservability{}, + c: fake.NewClientBuilder().WithScheme(clientScheme).WithObjects(&routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: obsAPIGateway, + Namespace: mcoconfig.GetDefaultNamespace(), + }, + Spec: routev1.RouteSpec{ + Port: &routev1.RoutePort{ + TargetPort: intstr.FromString("oauth-proxy"), + }, + To: routev1.RouteTargetReference{ + Kind: "Service", + Name: "modified", + }, + TLS: &routev1.TLSConfig{ + Termination: routev1.TLSTerminationReencrypt, + InsecureEdgeTerminationPolicy: routev1.InsecureEdgeTerminationPolicyRedirect, + }, + }, + }).Build(), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := GenerateAPIGatewayRoute(ctx, tt.c, s, tt.instance) + if err != nil { + t.Errorf("GenerateAPIGatewayRoute() error = %v", err) + return + } + list := &routev1.RouteList{} + if err := tt.c.List(context.Background(), list); err != nil { + t.Fatalf("Unable to list routes: (%v)", err) + } + if len(list.Items) != 1 { + t.Fatalf("Expected 1 route, got %d", len(list.Items)) + } + if !reflect.DeepEqual(list.Items[0].Spec, tt.want.Spec) { + t.Fatalf("Expected route spec: %v, got %v", tt.want.Spec, list.Items[0].Spec) + } + }) + } +} diff --git a/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go b/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go index b4b80e4a59..8b42ccefb3 100644 --- a/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go +++ b/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go @@ -371,11 +371,6 @@ func createAllRelatedRes( } } - currentClusters := []string{} - for _, ep := range obsAddonList.Items { - currentClusters = append(currentClusters, ep.Namespace) - } - // need to reload the template and update the the corresponding resources // the loadTemplates method is now lightweight operations as we have cache the templates in memory. log.Info("load and update templates for managedcluster resources") @@ -398,7 +393,6 @@ func createAllRelatedRes( failedCreateManagedClusterRes := false managedClusterListMutex.RLock() for managedCluster, openshiftVersion := range managedClusterList { - currentClusters = commonutil.Remove(currentClusters, managedCluster) if isReconcileRequired(request, managedCluster) { log.Info( "Monitoring operator should be installed in cluster", @@ -443,10 +437,20 @@ func createAllRelatedRes( } } } + + // Look through the obsAddonList items and find clusters + // which are no longer to be managed and therefore needs deletion + clustersToCleanup := []string{} + for _, ep := range obsAddonList.Items { + if _, ok := managedClusterList[ep.Namespace]; !ok { + clustersToCleanup = append(clustersToCleanup, ep.Namespace) + } + } + managedClusterListMutex.RUnlock() failedDeleteOba := false - for _, cluster := range currentClusters { + for _, cluster := range clustersToCleanup { if cluster != config.GetDefaultNamespace() { err = deleteObsAddon(c, cluster) if err != nil { diff --git a/operators/multiclusterobservability/pkg/config/config.go b/operators/multiclusterobservability/pkg/config/config.go index 73759b5fda..3c0c5922c2 100644 --- a/operators/multiclusterobservability/pkg/config/config.go +++ b/operators/multiclusterobservability/pkg/config/config.go @@ -482,6 +482,22 @@ func GetDefaultTenantName() string { // GetObsAPIHost is used to get the URL for observartium api gateway. func GetObsAPIHost(client client.Client, namespace string) (string, error) { + mco := &observabilityv1beta2.MultiClusterObservability{} + err := client.Get(context.TODO(), + types.NamespacedName{ + Name: GetMonitoringCRName(), + }, mco) + if err != nil && !errors.IsNotFound(err) { + return "", err + } + advancedConfig := mco.Spec.AdvancedConfig + if advancedConfig != nil && advancedConfig.CustomObservabilityHubURL != "" { + err := advancedConfig.CustomObservabilityHubURL.Validate() + if err != nil { + return "", err + } + return string(advancedConfig.CustomObservabilityHubURL), nil + } return GetRouteHost(client, obsAPIGateway, namespace) } diff --git a/operators/multiclusterobservability/pkg/config/config_test.go b/operators/multiclusterobservability/pkg/config/config_test.go index cf4318652f..3ad4552a5d 100644 --- a/operators/multiclusterobservability/pkg/config/config_test.go +++ b/operators/multiclusterobservability/pkg/config/config_test.go @@ -18,7 +18,6 @@ import ( "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/kubectl/pkg/scheme" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -264,16 +263,43 @@ func TestGetObsAPIHost(t *testing.T) { } scheme := runtime.NewScheme() scheme.AddKnownTypes(routev1.GroupVersion, route) + scheme.AddKnownTypes(mcov1beta2.GroupVersion, &mcov1beta2.MultiClusterObservability{}) client := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(route).Build() host, _ := GetObsAPIHost(client, "default") if host == apiServerURL { t.Errorf("Should not get route host in default namespace") } + host, _ = GetObsAPIHost(client, "test") if host != apiServerURL { t.Errorf("Observatorium api (%v) is not the expected (%v)", host, apiServerURL) } + + customBaseURL := "https://custom.base/url" + mco := &mcov1beta2.MultiClusterObservability{ + ObjectMeta: metav1.ObjectMeta{ + Name: GetMonitoringCRName(), + }, + Spec: mcov1beta2.MultiClusterObservabilitySpec{ + AdvancedConfig: &mcov1beta2.AdvancedConfig{ + CustomObservabilityHubURL: mcoshared.URL(customBaseURL), + }, + }, + } + client = fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(route, mco).Build() + host, _ = GetObsAPIHost(client, "test") + if host != customBaseURL { + t.Errorf("Observatorium api (%v) is not the expected (%v)", host, customBaseURL) + } + + mco.Spec.AdvancedConfig.CustomObservabilityHubURL = "httpa://foob ar.c" + client = fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(route, mco).Build() + _, err := GetObsAPIHost(client, "test") + if err == nil { + t.Errorf("expected error when parsing URL '%v', but got none", mco.Spec.AdvancedConfig.CustomObservabilityHubURL) + } + } func TestIsPaused(t *testing.T) { @@ -325,7 +351,7 @@ func TestIsPaused(t *testing.T) { func NewFakeClient(mco *mcov1beta2.MultiClusterObservability, obs *observatoriumv1alpha1.Observatorium) client.Client { - s := scheme.Scheme + s := runtime.NewScheme() s.AddKnownTypes(mcov1beta2.GroupVersion, mco) s.AddKnownTypes(observatoriumv1alpha1.GroupVersion, obs) objs := []runtime.Object{mco, obs} @@ -446,7 +472,9 @@ func TestReadImageManifestConfigMap(t *testing.T) { } func Test_checkIsIBMCloud(t *testing.T) { - s := scheme.Scheme + s := runtime.NewScheme() + s.AddKnownTypes(corev1.SchemeGroupVersion, &corev1.Node{}) + s.AddKnownTypes(corev1.SchemeGroupVersion, &corev1.NodeList{}) nodeIBM := &corev1.Node{ Spec: corev1.NodeSpec{ ProviderID: "ibm", @@ -891,7 +919,7 @@ func TestGetOperandName(t *testing.T) { name: "Have Observatorium CR without ownerreference", componentName: Alertmanager, prepare: func() { - //clean the operandNames map + // clean the operandNames map CleanUpOperandNames() mco := &mcov1beta2.MultiClusterObservability{ TypeMeta: metav1.TypeMeta{Kind: "MultiClusterObservability"}, @@ -916,7 +944,7 @@ func TestGetOperandName(t *testing.T) { } // Register operator types with the runtime scheme. - s := scheme.Scheme + s := runtime.NewScheme() mcov1beta2.SchemeBuilder.AddToScheme(s) observatoriumv1alpha1.AddToScheme(s) client := fake.NewClientBuilder().WithScheme(s).WithRuntimeObjects(mco, observatorium).Build() @@ -933,7 +961,7 @@ func TestGetOperandName(t *testing.T) { name: "Have Observatorium CR (observability-observatorium) with ownerreference", componentName: Alertmanager, prepare: func() { - //clean the operandNames map + // clean the operandNames map CleanUpOperandNames() mco := &mcov1beta2.MultiClusterObservability{ TypeMeta: metav1.TypeMeta{Kind: "MultiClusterObservability"}, @@ -964,7 +992,7 @@ func TestGetOperandName(t *testing.T) { } // Register operator types with the runtime scheme. - s := scheme.Scheme + s := runtime.NewScheme() mcov1beta2.SchemeBuilder.AddToScheme(s) observatoriumv1alpha1.AddToScheme(s) client := fake.NewClientBuilder().WithScheme(s).WithRuntimeObjects(mco, observatorium).Build() @@ -982,7 +1010,7 @@ func TestGetOperandName(t *testing.T) { name: "Have Observatorium CR (observability) with ownerreference", componentName: Alertmanager, prepare: func() { - //clean the operandNames map + // clean the operandNames map CleanUpOperandNames() mco := &mcov1beta2.MultiClusterObservability{ TypeMeta: metav1.TypeMeta{Kind: "MultiClusterObservability"}, @@ -1013,7 +1041,7 @@ func TestGetOperandName(t *testing.T) { } // Register operator types with the runtime scheme. - s := scheme.Scheme + s := runtime.NewScheme() mcov1beta2.SchemeBuilder.AddToScheme(s) observatoriumv1alpha1.AddToScheme(s) client := fake.NewClientBuilder().WithScheme(s).WithRuntimeObjects(mco, observatorium).Build()