Merge remote-tracking branch 'origin/main' into add_mc_e2e

stolostron · Apr 30, 2024 · 3e8327e · 3e8327e
2 parents 63dbb4b + b78fc3b
commit 3e8327e
Show file tree

Hide file tree

Showing 13 changed files with 410 additions and 57 deletions.
diff --git 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
@@ -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/...undle/manifests/observability.open-cluster-management.io_multiclusterobservabilities.yaml b/...undle/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/...onfig/crd/bases/observability.open-cluster-management.io_multiclusterobservabilities.yaml b/...onfig/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
@@ -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
@@ -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/...servability/controllers/multiclusterobservability/multiclusterobservability_controller.go b/...servability/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/...bility/controllers/multiclusterobservability/multiclusterobservability_controller_test.go b/...bility/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() {