From 4f47e11f5527ea3e7fd793ba2a93d28da6dae439 Mon Sep 17 00:00:00 2001 From: Song Song Li Date: Thu, 14 May 2020 09:16:57 +0800 Subject: [PATCH] fix code smell (#28) --- cmd/manager/main.go | 27 ++++- .../multiclustermonitoring/default_conf.go | 16 +-- .../default_conf_test.go | 16 +-- .../multiclustermonitoring/grafana.go | 29 +++-- .../multiclustermonitoring_controller.go | 25 ++-- .../multiclustermonitoring/object_storage.go | 33 +++--- .../object_storage_test.go | 40 +++---- .../multiclustermonitoring/observatorium.go | 107 ++++++++++++------ .../observatorium_test.go | 6 +- .../multiclustermonitoring/ocp_monitoring.go | 6 +- pkg/rendering/patching/patcher.go | 32 ++++-- pkg/rendering/renderer.go | 48 ++++++-- pkg/rendering/templates/templates.go | 8 +- 13 files changed, 261 insertions(+), 132 deletions(-) diff --git a/cmd/manager/main.go b/cmd/manager/main.go index f486b761f..a5d5fce61 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -118,7 +118,12 @@ func main() { } // Setup Scheme for observatorium resources - schemeBuilder := &scheme.Builder{GroupVersion: schema.GroupVersion{Group: "core.observatorium.io", Version: "v1alpha1"}} + schemeBuilder := &scheme.Builder{ + GroupVersion: schema.GroupVersion{ + Group: "core.observatorium.io", + Version: "v1alpha1", + }, + } schemeBuilder.Register(&observatoriumAPIs.Observatorium{}, &observatoriumAPIs.ObservatoriumList{}) if err := schemeBuilder.AddToScheme(mgr.GetScheme()); err != nil { log.Error(err, "") @@ -156,8 +161,24 @@ func addMetrics(ctx context.Context, cfg *rest.Config, namespace string) { // Add to the below struct any other metrics ports you want to expose. servicePorts := []v1.ServicePort{ - {Port: metricsPort, Name: metrics.OperatorPortName, Protocol: v1.ProtocolTCP, TargetPort: intstr.IntOrString{Type: intstr.Int, IntVal: metricsPort}}, - {Port: operatorMetricsPort, Name: metrics.CRPortName, Protocol: v1.ProtocolTCP, TargetPort: intstr.IntOrString{Type: intstr.Int, IntVal: operatorMetricsPort}}, + { + Port: metricsPort, + Name: metrics.OperatorPortName, + Protocol: v1.ProtocolTCP, + TargetPort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: metricsPort, + }, + }, + { + Port: operatorMetricsPort, + Name: metrics.CRPortName, + Protocol: v1.ProtocolTCP, + TargetPort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: operatorMetricsPort, + }, + }, } // Create Service object to expose the metrics port(s). diff --git a/pkg/controller/multiclustermonitoring/default_conf.go b/pkg/controller/multiclustermonitoring/default_conf.go index 69e6dbc41..425db6be6 100644 --- a/pkg/controller/multiclustermonitoring/default_conf.go +++ b/pkg/controller/multiclustermonitoring/default_conf.go @@ -13,19 +13,19 @@ import ( ) const ( - DEFAULT_VERSION = "latest" - DEFAULT_IMG_REPO = "quay.io/open-cluster-management" - DEFAULT_IMG_PULL_SECRET = "quay-secret" - DEFAULT_STORAGE_CLASS = "gp2" + defaultVersion = "latest" + defaultImgRepo = "quay.io/open-cluster-management" + defaultImgPullSecret = "quay-secret" + defaultStorageClass = "gp2" ) func addDefaultConfig(c client.Client, mcm *monitoringv1alpha1.MultiClusterMonitoring) (*reconcile.Result, error) { if mcm.Spec.Version == "" { - mcm.Spec.Version = DEFAULT_VERSION + mcm.Spec.Version = defaultVersion } if mcm.Spec.ImageRepository == "" { - mcm.Spec.ImageRepository = DEFAULT_IMG_REPO + mcm.Spec.ImageRepository = defaultImgRepo } if string(mcm.Spec.ImagePullPolicy) == "" { @@ -33,7 +33,7 @@ func addDefaultConfig(c client.Client, mcm *monitoringv1alpha1.MultiClusterMonit } if mcm.Spec.ImagePullSecret == "" { - mcm.Spec.ImagePullSecret = DEFAULT_IMG_PULL_SECRET + mcm.Spec.ImagePullSecret = defaultImgPullSecret } if mcm.Spec.NodeSelector == nil { @@ -41,7 +41,7 @@ func addDefaultConfig(c client.Client, mcm *monitoringv1alpha1.MultiClusterMonit } if mcm.Spec.StorageClass == "" { - mcm.Spec.StorageClass = DEFAULT_STORAGE_CLASS + mcm.Spec.StorageClass = defaultStorageClass } if mcm.Spec.Observatorium == nil { diff --git a/pkg/controller/multiclustermonitoring/default_conf_test.go b/pkg/controller/multiclustermonitoring/default_conf_test.go index 113ec7532..de9e99d2a 100644 --- a/pkg/controller/multiclustermonitoring/default_conf_test.go +++ b/pkg/controller/multiclustermonitoring/default_conf_test.go @@ -23,28 +23,28 @@ func TestAddDefaultConfig(t *testing.T) { t.Errorf("Should return nil for result (%v) and err (%v)", result, err) } - if mcm.Spec.Version != DEFAULT_VERSION { - t.Errorf("Version (%v) is not the expected (%v)", mcm.Spec.Version, DEFAULT_VERSION) + if mcm.Spec.Version != defaultVersion { + t.Errorf("Version (%v) is not the expected (%v)", mcm.Spec.Version, defaultVersion) } - if mcm.Spec.ImageRepository != DEFAULT_IMG_REPO { - t.Errorf("ImageRepository (%v) is not the expected (%v)", mcm.Spec.ImageRepository, DEFAULT_IMG_REPO) + if mcm.Spec.ImageRepository != defaultImgRepo { + t.Errorf("ImageRepository (%v) is not the expected (%v)", mcm.Spec.ImageRepository, defaultImgRepo) } if string(mcm.Spec.ImagePullPolicy) != string(corev1.PullAlways) { t.Errorf("ImagePullPolicy (%v) is not the expected (%v)", mcm.Spec.ImagePullPolicy, corev1.PullAlways) } - if mcm.Spec.ImagePullSecret != DEFAULT_IMG_PULL_SECRET { - t.Errorf("ImagePullSecret (%v) is not the expected (%v)", mcm.Spec.ImagePullSecret, DEFAULT_IMG_PULL_SECRET) + if mcm.Spec.ImagePullSecret != defaultImgPullSecret { + t.Errorf("ImagePullSecret (%v) is not the expected (%v)", mcm.Spec.ImagePullSecret, defaultImgPullSecret) } if mcm.Spec.NodeSelector == nil { t.Errorf("NodeSelector (%v) is not the expected (non-nil)", mcm.Spec.NodeSelector) } - if mcm.Spec.StorageClass != DEFAULT_STORAGE_CLASS { - t.Errorf("StorageClass (%v) is not the expected (%v)", mcm.Spec.StorageClass, DEFAULT_STORAGE_CLASS) + if mcm.Spec.StorageClass != defaultStorageClass { + t.Errorf("StorageClass (%v) is not the expected (%v)", mcm.Spec.StorageClass, defaultStorageClass) } if mcm.Spec.Observatorium == nil { diff --git a/pkg/controller/multiclustermonitoring/grafana.go b/pkg/controller/multiclustermonitoring/grafana.go index 46452ba13..1d28bd734 100644 --- a/pkg/controller/multiclustermonitoring/grafana.go +++ b/pkg/controller/multiclustermonitoring/grafana.go @@ -38,7 +38,10 @@ type GrafanaDatasource struct { // GenerateGrafanaDataSource is used to generate the GrafanaDatasource as a secret. // the GrafanaDatasource points to observatorium api gateway service -func GenerateGrafanaDataSource(client client.Client, scheme *runtime.Scheme, monitoring *monitoringv1alpha1.MultiClusterMonitoring) (*reconcile.Result, error) { +func GenerateGrafanaDataSource( + client client.Client, + scheme *runtime.Scheme, + monitoring *monitoringv1alpha1.MultiClusterMonitoring) (*reconcile.Result, error) { grafanaDatasources, err := json.MarshalIndent(GrafanaDatasources{ APIVersion: 1, @@ -47,7 +50,7 @@ func GenerateGrafanaDataSource(client client.Client, scheme *runtime.Scheme, mon Name: "Observatorium", Type: "prometheus", Access: "proxy", - URL: "http://" + monitoring.Name + observatoriumPartoOfName + "-observatorium-api:8080/api/metrics/v1", + URL: "http://" + monitoring.Name + obsPartoOfName + "-observatorium-api:8080/api/metrics/v1", }, }, }, "", " ") @@ -55,7 +58,7 @@ func GenerateGrafanaDataSource(client client.Client, scheme *runtime.Scheme, mon return &reconcile.Result{}, err } - grafanaDataSourceSecret := &corev1.Secret{ + dsSecret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "grafana-datasources", Namespace: monitoring.Namespace, @@ -67,16 +70,28 @@ func GenerateGrafanaDataSource(client client.Client, scheme *runtime.Scheme, mon } // Set MultiClusterMonitoring instance as the owner and controller - if err = controllerutil.SetControllerReference(monitoring, grafanaDataSourceSecret, scheme); err != nil { + if err = controllerutil.SetControllerReference(monitoring, dsSecret, scheme); err != nil { return &reconcile.Result{}, err } // Check if this already exists grafanaDSFound := &corev1.Secret{} - err = client.Get(context.TODO(), types.NamespacedName{Name: grafanaDataSourceSecret.Name, Namespace: grafanaDataSourceSecret.Namespace}, grafanaDSFound) + err = client.Get( + context.TODO(), + types.NamespacedName{ + Name: dsSecret.Name, + Namespace: dsSecret.Namespace, + }, + grafanaDSFound, + ) + if err != nil && errors.IsNotFound(err) { - log.Info("Creating a new grafana datasource secret", "grafanaDataSourceSecret.Namespace", grafanaDataSourceSecret.Namespace, "grafanaDataSourceSecret.Name", grafanaDataSourceSecret.Name) - err = client.Create(context.TODO(), grafanaDataSourceSecret) + log.Info("Creating a new grafana datasource secret", + "dsSecret.Namespace", dsSecret.Namespace, + "dsSecret.Name", dsSecret.Name, + ) + + err = client.Create(context.TODO(), dsSecret) if err != nil { return &reconcile.Result{}, err } diff --git a/pkg/controller/multiclustermonitoring/multiclustermonitoring_controller.go b/pkg/controller/multiclustermonitoring/multiclustermonitoring_controller.go index b81ff698f..a12fdef4f 100644 --- a/pkg/controller/multiclustermonitoring/multiclustermonitoring_controller.go +++ b/pkg/controller/multiclustermonitoring/multiclustermonitoring_controller.go @@ -35,15 +35,19 @@ var log = logf.Log.WithName("controller_multiclustermonitoring") * business logic. Delete these comments after modifying this file.* */ -// Add creates a new MultiClusterMonitoring Controller and adds it to the Manager. The Manager will set fields on the Controller -// and Start it when the Manager is Started. +// Add creates a new MultiClusterMonitoring Controller and adds it to the Manager. The Manager will set fields on +// the Controller and Start it when the Manager is Started. func Add(mgr manager.Manager) error { return add(mgr, newReconciler(mgr)) } // newReconciler returns a new reconcile.Reconciler func newReconciler(mgr manager.Manager) reconcile.Reconciler { - return &ReconcileMultiClusterMonitoring{client: mgr.GetClient(), apiReader: mgr.GetAPIReader(), scheme: mgr.GetScheme()} + return &ReconcileMultiClusterMonitoring{ + client: mgr.GetClient(), + apiReader: mgr.GetAPIReader(), + scheme: mgr.GetScheme(), + } } // add adds a new Controller to mgr with r as the reconcile.Reconciler @@ -109,9 +113,9 @@ type ReconcileMultiClusterMonitoring struct { scheme *runtime.Scheme } -// Reconcile reads that state of the cluster for a MultiClusterMonitoring object and makes changes based on the state read -// and what is in the MultiClusterMonitoring.Spec -// TODO(user): Modify this Reconcile function to implement your Controller logic. This example creates +// Reconcile reads that state of the cluster for a MultiClusterMonitoring object and makes changes +// based on the state read and what is in the MultiClusterMonitoring.Spec +// Modify this Reconcile function to implement your Controller logic. This example creates // a Pod as an example // Note: // The Controller will requeue the Request to be processed again if the returned error is non-nil or @@ -193,7 +197,9 @@ func (r *ReconcileMultiClusterMonitoring) Reconcile(request reconcile.Request) ( return reconcile.Result{Requeue: true}, nil } -func (r *ReconcileMultiClusterMonitoring) UpdateStatus(mcm *monitoringv1alpha1.MultiClusterMonitoring) (*reconcile.Result, error) { +func (r *ReconcileMultiClusterMonitoring) UpdateStatus( + mcm *monitoringv1alpha1.MultiClusterMonitoring) (*reconcile.Result, error) { + reqLogger := log.WithValues("Request.Namespace", mcm.Namespace, "Request.Name", mcm.Name) deployList := &appsv1.DeploymentList{} @@ -203,7 +209,10 @@ func (r *ReconcileMultiClusterMonitoring) UpdateStatus(mcm *monitoringv1alpha1.M } err := r.client.List(context.TODO(), deployList, listOpts...) if err != nil { - reqLogger.Error(err, "Failed to list deployments.", "MultiClusterMonitoring.Namespace", mcm.Namespace, "MemcaMultiClusterMonitoringched.Name", mcm.Name) + reqLogger.Error(err, "Failed to list deployments.", + "MultiClusterMonitoring.Namespace", mcm.Namespace, + "MemcaMultiClusterMonitoringched.Name", mcm.Name, + ) return &reconcile.Result{}, err } diff --git a/pkg/controller/multiclustermonitoring/object_storage.go b/pkg/controller/multiclustermonitoring/object_storage.go index 1a5a04031..458669b2b 100644 --- a/pkg/controller/multiclustermonitoring/object_storage.go +++ b/pkg/controller/multiclustermonitoring/object_storage.go @@ -17,29 +17,32 @@ import ( ) const ( - DEFAULT_OBJ_STORAGE_TYPE = "minio" - DEFAULT_OBJ_STORAGE_BUCKET = "thanos" - DEFAULT_OBJ_STORAGE_ENDPOINT = "minio:9000" - DEFAULT_OBJ_STORAGE_INSECURE = true - DEFAULT_OBJ_STORAGE_ACCESSKEY = "minio" - DEFAULT_OBJ_STORAGE_SECRETKEY = "minio123" - DEFAULT_OBJ_STORAGE_STORAGE = "1Gi" + defaultObjStorageType = "minio" + defaultObjStorageBucket = "thanos" + defaultObjStorageEndpoint = "minio:9000" + defaultObjStorageInsecure = true + defaultObjStorageAccesskey = "minio" + defaultObjStorageSecretkey = "minio123" + defaultObjStorageStorage = "1Gi" ) func newDefaultObjectStorageConfigSpec() *monitoringv1alpha1.ObjectStorageConfigSpec { spec := &monitoringv1alpha1.ObjectStorageConfigSpec{} - spec.Type = DEFAULT_OBJ_STORAGE_TYPE - spec.Config.Bucket = DEFAULT_OBJ_STORAGE_BUCKET - spec.Config.Endpoint = DEFAULT_OBJ_STORAGE_ENDPOINT - spec.Config.Insecure = DEFAULT_OBJ_STORAGE_INSECURE - spec.Config.AccessKey = DEFAULT_OBJ_STORAGE_ACCESSKEY - spec.Config.SecretKey = DEFAULT_OBJ_STORAGE_SECRETKEY - spec.Config.Storage = DEFAULT_OBJ_STORAGE_STORAGE + spec.Type = defaultObjStorageType + spec.Config.Bucket = defaultObjStorageBucket + spec.Config.Endpoint = defaultObjStorageEndpoint + spec.Config.Insecure = defaultObjStorageInsecure + spec.Config.AccessKey = defaultObjStorageAccesskey + spec.Config.SecretKey = defaultObjStorageSecretkey + spec.Config.Storage = defaultObjStorageStorage return spec } -func updateObjStorageConfig(c client.Client, mcm *monitoringv1alpha1.MultiClusterMonitoring) (*reconcile.Result, error) { +func updateObjStorageConfig( + c client.Client, + mcm *monitoringv1alpha1.MultiClusterMonitoring) (*reconcile.Result, error) { + // Check valid object storage type if mcm.Spec.ObjectStorageConfigSpec != nil { objStorageType := mcm.Spec.ObjectStorageConfigSpec.Type diff --git a/pkg/controller/multiclustermonitoring/object_storage_test.go b/pkg/controller/multiclustermonitoring/object_storage_test.go index b770ddb57..eca6c8bba 100644 --- a/pkg/controller/multiclustermonitoring/object_storage_test.go +++ b/pkg/controller/multiclustermonitoring/object_storage_test.go @@ -24,32 +24,32 @@ func NewFakeClient(mcm *monitoringv1alpha1.MultiClusterMonitoring) client.Client func TestNewDefaultObjectStorageConfigSpec(t *testing.T) { spec := newDefaultObjectStorageConfigSpec() - if spec.Type != DEFAULT_OBJ_STORAGE_TYPE { - t.Errorf("Type (%v) is not the expected (%v)", spec.Type, DEFAULT_OBJ_STORAGE_TYPE) + if spec.Type != defaultObjStorageType { + t.Errorf("Type (%v) is not the expected (%v)", spec.Type, defaultObjStorageType) } - if spec.Config.Bucket != DEFAULT_OBJ_STORAGE_BUCKET { - t.Errorf("Bucket (%v) is not the expected (%v)", spec.Config.Bucket, DEFAULT_OBJ_STORAGE_BUCKET) + if spec.Config.Bucket != defaultObjStorageBucket { + t.Errorf("Bucket (%v) is not the expected (%v)", spec.Config.Bucket, defaultObjStorageBucket) } - if spec.Config.Endpoint != DEFAULT_OBJ_STORAGE_ENDPOINT { - t.Errorf("Endpoint (%v) is not the expected (%v)", spec.Config.Endpoint, DEFAULT_OBJ_STORAGE_ENDPOINT) + if spec.Config.Endpoint != defaultObjStorageEndpoint { + t.Errorf("Endpoint (%v) is not the expected (%v)", spec.Config.Endpoint, defaultObjStorageEndpoint) } - if spec.Config.Insecure != DEFAULT_OBJ_STORAGE_INSECURE { - t.Errorf("Insecure (%v) is not the expected (%v)", spec.Config.Insecure, DEFAULT_OBJ_STORAGE_INSECURE) + if spec.Config.Insecure != defaultObjStorageInsecure { + t.Errorf("Insecure (%v) is not the expected (%v)", spec.Config.Insecure, defaultObjStorageInsecure) } - if spec.Config.AccessKey != DEFAULT_OBJ_STORAGE_ACCESSKEY { - t.Errorf("AccessKey (%v) is not the expected (%v)", spec.Config.AccessKey, DEFAULT_OBJ_STORAGE_ACCESSKEY) + if spec.Config.AccessKey != defaultObjStorageAccesskey { + t.Errorf("AccessKey (%v) is not the expected (%v)", spec.Config.AccessKey, defaultObjStorageAccesskey) } - if spec.Config.SecretKey != DEFAULT_OBJ_STORAGE_SECRETKEY { - t.Errorf("SecretKey (%v) is not the expected (%v)", spec.Config.SecretKey, DEFAULT_OBJ_STORAGE_SECRETKEY) + if spec.Config.SecretKey != defaultObjStorageSecretkey { + t.Errorf("SecretKey (%v) is not the expected (%v)", spec.Config.SecretKey, defaultObjStorageSecretkey) } - if spec.Config.Storage != DEFAULT_OBJ_STORAGE_STORAGE { - t.Errorf("Storage (%v) is not the expected (%v)", spec.Config.Storage, DEFAULT_OBJ_STORAGE_STORAGE) + if spec.Config.Storage != defaultObjStorageStorage { + t.Errorf("Storage (%v) is not the expected (%v)", spec.Config.Storage, defaultObjStorageStorage) } } @@ -74,10 +74,10 @@ func TestCheckObjStorageConfig(t *testing.T) { t.Errorf("Failed to check valid object storage type: result: (%v) err: (%v)", result, err) } - mcm.Spec.ObjectStorageConfigSpec.Type = DEFAULT_OBJ_STORAGE_TYPE + mcm.Spec.ObjectStorageConfigSpec.Type = defaultObjStorageType result, err = updateObjStorageConfig(NewFakeClient(mcm), mcm) if result != nil || err != nil { - t.Errorf("(%v) should be a valid type", DEFAULT_OBJ_STORAGE_TYPE) + t.Errorf("(%v) should be a valid type", defaultObjStorageType) } mcm.Spec.ObjectStorageConfigSpec.Type = "s3" @@ -87,12 +87,12 @@ func TestCheckObjStorageConfig(t *testing.T) { } updateObjStorageConfig(NewFakeClient(mcm), mcm) - if mcm.Spec.ObjectStorageConfigSpec.Config.Bucket != DEFAULT_OBJ_STORAGE_BUCKET { - t.Errorf("Bucket (%v) is not the expected (%v)", mcm.Spec.ObjectStorageConfigSpec.Config.Bucket, DEFAULT_OBJ_STORAGE_BUCKET) + if mcm.Spec.ObjectStorageConfigSpec.Config.Bucket != defaultObjStorageBucket { + t.Errorf("Bucket (%v) is not the expected (%v)", mcm.Spec.ObjectStorageConfigSpec.Config.Bucket, defaultObjStorageBucket) } mcm.Spec.ObjectStorageConfigSpec = newDefaultObjectStorageConfigSpec() - if mcm.Spec.ObjectStorageConfigSpec.Config.Endpoint != DEFAULT_OBJ_STORAGE_ENDPOINT { - t.Errorf("Endpoint (%v) is not the expected (%v)", mcm.Spec.ObjectStorageConfigSpec.Config.Endpoint, DEFAULT_OBJ_STORAGE_ENDPOINT) + if mcm.Spec.ObjectStorageConfigSpec.Config.Endpoint != defaultObjStorageEndpoint { + t.Errorf("Endpoint (%v) is not the expected (%v)", mcm.Spec.ObjectStorageConfigSpec.Config.Endpoint, defaultObjStorageEndpoint) } } diff --git a/pkg/controller/multiclustermonitoring/observatorium.go b/pkg/controller/multiclustermonitoring/observatorium.go index e26ad2662..2eb742d18 100644 --- a/pkg/controller/multiclustermonitoring/observatorium.go +++ b/pkg/controller/multiclustermonitoring/observatorium.go @@ -28,30 +28,34 @@ import ( ) const ( - observatoriumPartoOfName = "-observatorium" - observatoriumAPIGatewayName = "observatorium-api" + obsPartoOfName = "-observatorium" + obsAPIGateway = "observatorium-api" ) const ( - DEFAULT_STORAGE_SIZE = "1Gi" + defaultStorageSize = "1Gi" - DEFAULT_RETENTIONRESOLUTION1H = "1s" - DEFAULT_RETENTIONRESOLUTION5M = "1s" - DEFAULT_RETENTIONRESOLUTIONRAW = "14d" + retentionResolution1h = "1s" + retentionResolution5m = "1s" + retentionResolutionRaw = "14d" - DEFAULT_THANOS_IMAGE = "quay.io/thanos/thanos:v0.12.0" - DEFAULT_THANOS_VERSION = "v0.12.0" + defaultThanosImage = "quay.io/thanos/thanos:v0.12.0" + defaultThanosVersion = "v0.12.0" ) // GenerateObservatoriumCR returns Observatorium cr defined in MultiClusterMonitoring -func GenerateObservatoriumCR(client client.Client, scheme *runtime.Scheme, monitoring *monitoringv1alpha1.MultiClusterMonitoring) (*reconcile.Result, error) { +func GenerateObservatoriumCR( + client client.Client, + scheme *runtime.Scheme, + monitoring *monitoringv1alpha1.MultiClusterMonitoring) (*reconcile.Result, error) { + labels := map[string]string{ "app": monitoring.Name, } observatoriumCR := &observatoriumv1alpha1.Observatorium{ ObjectMeta: metav1.ObjectMeta{ - Name: monitoring.Name + observatoriumPartoOfName, + Name: monitoring.Name + obsPartoOfName, Namespace: monitoring.Namespace, Labels: labels, }, @@ -65,9 +69,20 @@ func GenerateObservatoriumCR(client client.Client, scheme *runtime.Scheme, monit // Check if this Observatorium CR already exists observatoriumCRFound := &observatoriumv1alpha1.Observatorium{} - err := client.Get(context.TODO(), types.NamespacedName{Name: observatoriumCR.Name, Namespace: observatoriumCR.Namespace}, observatoriumCRFound) + err := client.Get( + context.TODO(), + types.NamespacedName{ + Name: observatoriumCR.Name, + Namespace: observatoriumCR.Namespace, + }, + observatoriumCRFound, + ) + if err != nil && errors.IsNotFound(err) { - log.Info("Creating a new observatorium CR", "observatorium.Namespace", observatoriumCR.Namespace, "observatorium.Name", observatoriumCR.Name) + log.Info("Creating a new observatorium CR", + "observatorium.Namespace", observatoriumCR.Namespace, + "observatorium.Name", observatoriumCR.Name, + ) err = client.Create(context.TODO(), observatoriumCR) if err != nil { return &reconcile.Result{}, err @@ -110,8 +125,17 @@ func createKubeClient() (kubernetes.Interface, error) { return kubeClient, err } -func GenerateAPIGatewayRoute(client client.Client, scheme *runtime.Scheme, monitoring *monitoringv1alpha1.MultiClusterMonitoring) (*reconcile.Result, error) { - labelSelector := fmt.Sprintf("app.kubernetes.io/component=%s, app.kubernetes.io/instance=%s", "api", monitoring.Name+observatoriumPartoOfName) +func GenerateAPIGatewayRoute( + client client.Client, + scheme *runtime.Scheme, + monitoring *monitoringv1alpha1.MultiClusterMonitoring) (*reconcile.Result, error) { + + labelSelector := fmt.Sprintf( + "app.kubernetes.io/component=%s, app.kubernetes.io/instance=%s", + "api", + monitoring.Name+obsPartoOfName, + ) + listOptions := metav1.ListOptions{ LabelSelector: labelSelector, } @@ -125,7 +149,7 @@ func GenerateAPIGatewayRoute(client client.Client, scheme *runtime.Scheme, monit if err == nil && len(apiGatewayServices.Items) > 0 { apiGateway := &routev1.Route{ ObjectMeta: metav1.ObjectMeta{ - Name: observatoriumAPIGatewayName, + Name: obsAPIGateway, Namespace: monitoring.Namespace, }, Spec: routev1.RouteSpec{ @@ -138,9 +162,15 @@ func GenerateAPIGatewayRoute(client client.Client, scheme *runtime.Scheme, monit }, }, } - err = client.Get(context.TODO(), types.NamespacedName{Name: apiGateway.Name, Namespace: apiGateway.Namespace}, &routev1.Route{}) + err = client.Get( + context.TODO(), + types.NamespacedName{Name: apiGateway.Name, Namespace: apiGateway.Namespace}, + &routev1.Route{}) if err != nil && errors.IsNotFound(err) { - log.Info("Creating a new route to expose observatorium api", "apiGateway.Namespace", apiGateway.Namespace, "apiGateway.Name", apiGateway.Name) + log.Info("Creating a new route to expose observatorium api", + "apiGateway.Namespace", apiGateway.Namespace, + "apiGateway.Name", apiGateway.Name, + ) err = client.Create(context.TODO(), apiGateway) if err != nil { return &reconcile.Result{}, err @@ -148,7 +178,10 @@ func GenerateAPIGatewayRoute(client client.Client, scheme *runtime.Scheme, monit } } else if err == nil && len(apiGatewayServices.Items) == 0 { - log.Info("Cannot find the service ", "serviceName", monitoring.Name+observatoriumPartoOfName+"-"+observatoriumAPIGatewayName) + log.Info("Cannot find the service ", + "serviceName", + monitoring.Name+obsPartoOfName+"-"+obsAPIGateway, + ) return &reconcile.Result{RequeueAfter: time.Second * 10}, nil } else { return &reconcile.Result{}, err @@ -162,8 +195,8 @@ func newDefaultObservatoriumSpec() *observatoriumv1alpha1.ObservatoriumSpec { obs.API.Image = "quay.io/observatorium/observatorium:master-2020-04-29-v0.1.1-14-gceac185" obs.API.Version = "master-2020-04-29-v0.1.1-14-gceac185" - obs.APIQuery.Image = DEFAULT_THANOS_IMAGE - obs.APIQuery.Version = DEFAULT_THANOS_VERSION + obs.APIQuery.Image = defaultThanosImage + obs.APIQuery.Version = defaultThanosVersion obs.Compact = newCompactSpec() @@ -174,8 +207,8 @@ func newDefaultObservatoriumSpec() *observatoriumv1alpha1.ObservatoriumSpec { obs.ObjectStorageConfig.Name = "thanos-objectstorage" obs.ObjectStorageConfig.Key = "thanos.yaml" - obs.Query.Image = DEFAULT_THANOS_IMAGE - obs.Query.Version = DEFAULT_THANOS_VERSION + obs.Query.Image = defaultThanosImage + obs.Query.Version = defaultThanosVersion replicas := int32(1) obs.QueryCache.Image = "quay.io/cortexproject/cortex:master-fdcd992f" @@ -193,18 +226,18 @@ func newDefaultObservatoriumSpec() *observatoriumv1alpha1.ObservatoriumSpec { func newReceiversSpec() observatoriumv1alpha1.ReceiversSpec { receSpec := observatoriumv1alpha1.ReceiversSpec{} - receSpec.Image = DEFAULT_THANOS_IMAGE - receSpec.Version = DEFAULT_THANOS_VERSION - receSpec.VolumeClaimTemplate = newVolumeClaimTemplate(DEFAULT_STORAGE_SIZE) + receSpec.Image = defaultThanosImage + receSpec.Version = defaultThanosVersion + receSpec.VolumeClaimTemplate = newVolumeClaimTemplate(defaultStorageSize) return receSpec } func newRuleSpec() observatoriumv1alpha1.RuleSpec { ruleSpec := observatoriumv1alpha1.RuleSpec{} - ruleSpec.Image = DEFAULT_THANOS_IMAGE - ruleSpec.Version = DEFAULT_THANOS_VERSION - ruleSpec.VolumeClaimTemplate = newVolumeClaimTemplate(DEFAULT_STORAGE_SIZE) + ruleSpec.Image = defaultThanosImage + ruleSpec.Version = defaultThanosVersion + ruleSpec.VolumeClaimTemplate = newVolumeClaimTemplate(defaultStorageSize) return ruleSpec } @@ -212,9 +245,9 @@ func newRuleSpec() observatoriumv1alpha1.RuleSpec { func newStoreSpec() observatoriumv1alpha1.StoreSpec { storeSpec := observatoriumv1alpha1.StoreSpec{} - storeSpec.Image = DEFAULT_THANOS_IMAGE - storeSpec.Version = DEFAULT_THANOS_VERSION - storeSpec.VolumeClaimTemplate = newVolumeClaimTemplate(DEFAULT_STORAGE_SIZE) + storeSpec.Image = defaultThanosImage + storeSpec.Version = defaultThanosVersion + storeSpec.VolumeClaimTemplate = newVolumeClaimTemplate(defaultStorageSize) shards := int32(1) storeSpec.Shards = &shards storeSpec.Cache = newStoreCacheSpec() @@ -238,12 +271,12 @@ func newStoreCacheSpec() observatoriumv1alpha1.StoreCacheSpec { func newCompactSpec() observatoriumv1alpha1.CompactSpec { compactSpec := observatoriumv1alpha1.CompactSpec{} - compactSpec.Image = DEFAULT_THANOS_IMAGE - compactSpec.Version = DEFAULT_THANOS_VERSION - compactSpec.RetentionResolutionRaw = DEFAULT_RETENTIONRESOLUTIONRAW - compactSpec.RetentionResolution5m = DEFAULT_RETENTIONRESOLUTION5M - compactSpec.RetentionResolution1h = DEFAULT_RETENTIONRESOLUTION1H - compactSpec.VolumeClaimTemplate = newVolumeClaimTemplate(DEFAULT_STORAGE_SIZE) + compactSpec.Image = defaultThanosImage + compactSpec.Version = defaultThanosVersion + compactSpec.RetentionResolutionRaw = retentionResolutionRaw + compactSpec.RetentionResolution5m = retentionResolution5m + compactSpec.RetentionResolution1h = retentionResolution1h + compactSpec.VolumeClaimTemplate = newVolumeClaimTemplate(defaultStorageSize) return compactSpec } diff --git a/pkg/controller/multiclustermonitoring/observatorium_test.go b/pkg/controller/multiclustermonitoring/observatorium_test.go index 692f291a5..b588c2bef 100644 --- a/pkg/controller/multiclustermonitoring/observatorium_test.go +++ b/pkg/controller/multiclustermonitoring/observatorium_test.go @@ -12,15 +12,15 @@ import ( func TestNewVolumeClaimTemplate(t *testing.T) { vct := newVolumeClaimTemplate("1Gi") if vct.Spec.AccessModes[0] != v1.ReadWriteOnce || - vct.Spec.Resources.Requests[v1.ResourceStorage] != resource.MustParse(DEFAULT_STORAGE_SIZE) { + vct.Spec.Resources.Requests[v1.ResourceStorage] != resource.MustParse(defaultStorageSize) { t.Errorf("Failed to newVolumeClaimTemplate") } } func TestNewDefaultObservatorium(t *testing.T) { obs := newDefaultObservatoriumSpec() - if obs.Query.Image != DEFAULT_THANOS_IMAGE || - obs.Query.Version != DEFAULT_THANOS_VERSION { + if obs.Query.Image != defaultThanosImage || + obs.Query.Version != defaultThanosVersion { t.Errorf("Failed to newDefaultObservatorium") } } diff --git a/pkg/controller/multiclustermonitoring/ocp_monitoring.go b/pkg/controller/multiclustermonitoring/ocp_monitoring.go index 2d984f0e9..cde441a82 100644 --- a/pkg/controller/multiclustermonitoring/ocp_monitoring.go +++ b/pkg/controller/multiclustermonitoring/ocp_monitoring.go @@ -35,16 +35,16 @@ func UpdateOCPMonitoringCM(monitoring *monitoringv1alpha1.MultiClusterMonitoring } // Try to get route instance - obsRoute, err := routev1Client.RouteV1().Routes(monitoring.Namespace).Get(observatoriumAPIGatewayName, metav1.GetOptions{}) + obsRoute, err := routev1Client.RouteV1().Routes(monitoring.Namespace).Get(obsAPIGateway, metav1.GetOptions{}) if err != nil { - log.Error(err, "Failed to get route", observatoriumAPIGatewayName) + log.Error(err, "Failed to get route", obsAPIGateway) return &reconcile.Result{}, err } remoteRriteURL := obsRoute.Spec.Host if remoteRriteURL == "" { - remoteRriteURL = monitoring.Name + observatoriumPartoOfName + "-" + observatoriumAPIGatewayName + "." + monitoring.Namespace + ".svc" + remoteRriteURL = monitoring.Name + obsPartoOfName + "-" + obsAPIGateway + "." + monitoring.Namespace + ".svc" } err = util.UpdateHubClusterMonitoringConfig(remoteRriteURL) diff --git a/pkg/rendering/patching/patcher.go b/pkg/rendering/patching/patcher.go index 31e817327..6624aa570 100644 --- a/pkg/rendering/patching/patcher.go +++ b/pkg/rendering/patching/patcher.go @@ -15,16 +15,18 @@ import ( monitoringv1alpha1 "github.com/open-cluster-management/multicluster-monitoring-operator/pkg/apis/monitoring/v1alpha1" ) -type patchGenerateFn func(res *resource.Resource, multipleClusterMonitoring *monitoringv1alpha1.MultiClusterMonitoring) (ifc.Kunstructured, error) +type patchGenerateFn func( + res *resource.Resource, + mcm *monitoringv1alpha1.MultiClusterMonitoring) (ifc.Kunstructured, error) -func ApplyGlobalPatches(res *resource.Resource, multipleClusterMonitoring *monitoringv1alpha1.MultiClusterMonitoring) error { +func ApplyGlobalPatches(res *resource.Resource, mcm *monitoringv1alpha1.MultiClusterMonitoring) error { for _, generate := range []patchGenerateFn{ //generateImagePatch, //generateImagePullSecretsPatch, generateNodeSelectorPatch, } { - patch, err := generate(res, multipleClusterMonitoring) + patch, err := generate(res, mcm) if err != nil { return err } @@ -38,13 +40,15 @@ func ApplyGlobalPatches(res *resource.Resource, multipleClusterMonitoring *monit return nil } -func generateImagePatch(res *resource.Resource, mch *monitoringv1alpha1.MultiClusterMonitoring) (ifc.Kunstructured, error) { +func generateImagePatch( + res *resource.Resource, + mcm *monitoringv1alpha1.MultiClusterMonitoring) (ifc.Kunstructured, error) { imageFromTemplate, err := res.GetString("spec.template.spec.containers[0].image") // need to loop through all images if err != nil { return nil, err } - imageRepo := mch.Spec.ImageRepository - imageTagSuffix := mch.Spec.ImageTagSuffix + imageRepo := mcm.Spec.ImageRepository + imageTagSuffix := mcm.Spec.ImageTagSuffix if imageTagSuffix != "" { imageTagSuffix = "-" + imageTagSuffix } @@ -53,7 +57,7 @@ func generateImagePatch(res *resource.Resource, mch *monitoringv1alpha1.MultiClu container, _ := res.GetFieldValue("spec.template.spec.containers[0]") // need to loop through all images containerMap, _ := container.(map[string]interface{}) containerMap["image"] = generatedImage - containerMap["imagePullPolicy"] = mch.Spec.ImagePullPolicy + containerMap["imagePullPolicy"] = mcm.Spec.ImagePullPolicy return kunstruct.NewKunstructuredFactoryImpl().FromMap(map[string]interface{}{ "spec": map[string]interface{}{ @@ -76,8 +80,11 @@ spec: - name: __pullsecrets__ ` -func generateImagePullSecretsPatch(res *resource.Resource, mch *monitoringv1alpha1.MultiClusterMonitoring) (ifc.Kunstructured, error) { - pullSecret := mch.Spec.ImagePullSecret +func generateImagePullSecretsPatch( + res *resource.Resource, + mcm *monitoringv1alpha1.MultiClusterMonitoring) (ifc.Kunstructured, error) { + + pullSecret := mcm.Spec.ImagePullSecret if pullSecret == "" { return nil, nil } @@ -100,8 +107,11 @@ spec: nodeSelector: {__selector__} ` -func generateNodeSelectorPatch(res *resource.Resource, mch *monitoringv1alpha1.MultiClusterMonitoring) (ifc.Kunstructured, error) { - nodeSelectorOptions := mch.Spec.NodeSelector +func generateNodeSelectorPatch( + res *resource.Resource, + mcm *monitoringv1alpha1.MultiClusterMonitoring) (ifc.Kunstructured, error) { + + nodeSelectorOptions := mcm.Spec.NodeSelector if nodeSelectorOptions == nil { return nil, nil } diff --git a/pkg/rendering/renderer.go b/pkg/rendering/renderer.go index 744249aac..80840c8f9 100644 --- a/pkg/rendering/renderer.go +++ b/pkg/rendering/renderer.go @@ -229,14 +229,42 @@ func stringValueReplace(toReplace string, cr *monitoringv1.MultiClusterMonitorin replaced = strings.ReplaceAll(replaced, "{{MULTICLUSTERMONITORING_CR_NAME}}", string(cr.Name)) // Object storage config - replaced = strings.ReplaceAll(replaced, "{{OBJ_STORAGE_BUCKET}}", string(cr.Spec.ObjectStorageConfigSpec.Config.Bucket)) - replaced = strings.ReplaceAll(replaced, "{{OBJ_STORAGE_ENDPOINT}}", string(cr.Spec.ObjectStorageConfigSpec.Config.Endpoint)) - replaced = strings.ReplaceAll(replaced, "{{OBJ_STORAGE_INSECURE}}", strconv.FormatBool(cr.Spec.ObjectStorageConfigSpec.Config.Insecure)) - replaced = strings.ReplaceAll(replaced, "{{OBJ_STORAGE_ACCESSKEY}}", string(cr.Spec.ObjectStorageConfigSpec.Config.AccessKey)) - replaced = strings.ReplaceAll(replaced, "{{OBJ_STORAGE_SECRETKEY}}", string(cr.Spec.ObjectStorageConfigSpec.Config.SecretKey)) + replaced = strings.ReplaceAll( + replaced, + "{{OBJ_STORAGE_BUCKET}}", + string(cr.Spec.ObjectStorageConfigSpec.Config.Bucket), + ) + + replaced = strings.ReplaceAll( + replaced, + "{{OBJ_STORAGE_ENDPOINT}}", + string(cr.Spec.ObjectStorageConfigSpec.Config.Endpoint), + ) + + replaced = strings.ReplaceAll( + replaced, + "{{OBJ_STORAGE_INSECURE}}", + strconv.FormatBool(cr.Spec.ObjectStorageConfigSpec.Config.Insecure), + ) + + replaced = strings.ReplaceAll( + replaced, + "{{OBJ_STORAGE_ACCESSKEY}}", + string(cr.Spec.ObjectStorageConfigSpec.Config.AccessKey), + ) + + replaced = strings.ReplaceAll( + replaced, + "{{OBJ_STORAGE_SECRETKEY}}", + string(cr.Spec.ObjectStorageConfigSpec.Config.SecretKey), + ) if cr.Spec.ObjectStorageConfigSpec.Type == "minio" { - replaced = strings.ReplaceAll(replaced, "{{OBJ_STORAGE_STORAGE}}", string(cr.Spec.ObjectStorageConfigSpec.Config.Storage)) + replaced = strings.ReplaceAll( + replaced, + "{{OBJ_STORAGE_STORAGE}}", + string(cr.Spec.ObjectStorageConfigSpec.Config.Storage), + ) } return replaced @@ -244,7 +272,10 @@ func stringValueReplace(toReplace string, cr *monitoringv1.MultiClusterMonitorin func replaceInValues(values map[string]interface{}, cr *monitoringv1.MultiClusterMonitoring) error { for inKey := range values { - isPrimitiveType := reflect.TypeOf(values[inKey]).String() == "string" || reflect.TypeOf(values[inKey]).String() == "bool" || reflect.TypeOf(values[inKey]).String() == "int" + isPrimitiveType := reflect.TypeOf(values[inKey]).String() == "string" || + reflect.TypeOf(values[inKey]).String() == "bool" || + reflect.TypeOf(values[inKey]).String() == "int" + if isPrimitiveType { if reflect.TypeOf(values[inKey]).String() == "string" { values[inKey] = stringValueReplace(values[inKey].(string), cr) @@ -252,7 +283,8 @@ func replaceInValues(values map[string]interface{}, cr *monitoringv1.MultiCluste } else if reflect.TypeOf(values[inKey]).Kind().String() == "slice" { stringSlice := values[inKey].([]interface{}) for i := range stringSlice { - stringSlice[i] = stringValueReplace(stringSlice[i].(string), cr) // assumes only slices of strings, which is OK for now + // assumes only slices of strings, which is OK for now + stringSlice[i] = stringValueReplace(stringSlice[i].(string), cr) } } else { // reflect.TypeOf(values[inKey]).Kind().String() == "map" inValue, ok := values[inKey].(map[string]interface{}) diff --git a/pkg/rendering/templates/templates.go b/pkg/rendering/templates/templates.go index 915fcf146..8b99e847e 100644 --- a/pkg/rendering/templates/templates.go +++ b/pkg/rendering/templates/templates.go @@ -91,7 +91,13 @@ func (r *TemplateRenderer) addTemplateFromPath(kustomizationPath string, resourc } func (r *TemplateRenderer) render(kustomizationPath string) (resmap.ResMap, error) { - ldr, err := loader.NewLoader(loader.RestrictionRootOnly, validator.NewKustValidator(), kustomizationPath, fs.MakeFsOnDisk()) + ldr, err := loader.NewLoader( + loader.RestrictionRootOnly, + validator.NewKustValidator(), + kustomizationPath, + fs.MakeFsOnDisk(), + ) + if err != nil { return nil, err }