diff --git a/cmd/aro/operator.go b/cmd/aro/operator.go index 979f15b74fd..2b96ad32ff1 100644 --- a/cmd/aro/operator.go +++ b/cmd/aro/operator.go @@ -36,7 +36,6 @@ import ( "github.com/Azure/ARO-RP/pkg/operator/controllers/machine" "github.com/Azure/ARO-RP/pkg/operator/controllers/machinehealthcheck" "github.com/Azure/ARO-RP/pkg/operator/controllers/machineset" - "github.com/Azure/ARO-RP/pkg/operator/controllers/monitoring" "github.com/Azure/ARO-RP/pkg/operator/controllers/muo" "github.com/Azure/ARO-RP/pkg/operator/controllers/node" "github.com/Azure/ARO-RP/pkg/operator/controllers/previewfeature" @@ -128,11 +127,6 @@ func operator(ctx context.Context, log *logrus.Entry) error { client, dh)).SetupWithManager(mgr); err != nil { return fmt.Errorf("unable to create controller %s: %v", routefix.ControllerName, err) } - if err = (monitoring.NewReconciler( - log.WithField("controller", monitoring.ControllerName), - client)).SetupWithManager(mgr); err != nil { - return fmt.Errorf("unable to create controller %s: %v", monitoring.ControllerName, err) - } if err = (rbac.NewReconciler( log.WithField("controller", rbac.ControllerName), client, dh)).SetupWithManager(mgr); err != nil { diff --git a/pkg/operator/controllers/monitoring/monitoring_controller.go b/pkg/operator/controllers/monitoring/monitoring_controller.go deleted file mode 100644 index 1cc31dd5675..00000000000 --- a/pkg/operator/controllers/monitoring/monitoring_controller.go +++ /dev/null @@ -1,224 +0,0 @@ -package monitoring - -// Copyright (c) Microsoft Corporation. -// Licensed under the Apache License 2.0. - -import ( - "context" - "encoding/json" - - "github.com/sirupsen/logrus" - "github.com/ugorji/go/codec" - corev1 "k8s.io/api/core/v1" - kerrors "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/labels" - "k8s.io/apimachinery/pkg/types" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/builder" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/handler" - "sigs.k8s.io/controller-runtime/pkg/predicate" - "sigs.k8s.io/controller-runtime/pkg/reconcile" - "sigs.k8s.io/controller-runtime/pkg/source" - "sigs.k8s.io/yaml" - - "github.com/Azure/ARO-RP/pkg/api" - "github.com/Azure/ARO-RP/pkg/operator" - arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1" - "github.com/Azure/ARO-RP/pkg/operator/controllers/base" - "github.com/Azure/ARO-RP/pkg/operator/predicates" -) - -const ( - ControllerName = "Monitoring" -) - -var ( - monitoringName = types.NamespacedName{Name: "cluster-monitoring-config", Namespace: "openshift-monitoring"} - prometheusLabels = "app=prometheus,prometheus=k8s" -) - -// Config represents cluster monitoring stack configuration. -// Reconciler reconciles retention and storage settings, -// MissingFields are used to preserve settings configured by user. -type Config struct { - api.MissingFields - PrometheusK8s struct { - api.MissingFields - Retention string `json:"retention,omitempty"` - VolumeClaimTemplate *json.RawMessage `json:"volumeClaimTemplate,omitempty"` - } `json:"prometheusK8s,omitempty"` - AlertManagerMain struct { - api.MissingFields - VolumeClaimTemplate *json.RawMessage `json:"volumeClaimTemplate,omitempty"` - } `json:"alertmanagerMain,omitempty"` -} - -type MonitoringReconciler struct { - base.AROController - - jsonHandle *codec.JsonHandle -} - -func NewReconciler(log *logrus.Entry, client client.Client) *MonitoringReconciler { - return &MonitoringReconciler{ - AROController: base.AROController{ - Log: log, - Client: client, - Name: ControllerName, - }, - jsonHandle: new(codec.JsonHandle), - } -} - -func (r *MonitoringReconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.Result, error) { - instance, err := r.GetCluster(ctx) - if err != nil { - return reconcile.Result{}, err - } - - if !instance.Spec.OperatorFlags.GetSimpleBoolean(operator.MonitoringEnabled) { - r.Log.Debug("controller is disabled") - return reconcile.Result{}, nil - } - - r.Log.Debug("running") - for _, f := range []func(context.Context) (ctrl.Result, error){ - r.reconcileConfiguration, - r.reconcilePVC, // TODO(mj): This should be removed once we don't have PVC anymore - } { - result, err := f(ctx) - if err != nil { - r.Log.Error(err) - r.SetDegraded(ctx, err) - return result, err - } - } - - return reconcile.Result{}, nil -} - -func (r *MonitoringReconciler) reconcilePVC(ctx context.Context) (ctrl.Result, error) { - pvcList := &corev1.PersistentVolumeClaimList{} - selector, _ := labels.Parse(prometheusLabels) - err := r.Client.List(ctx, pvcList, &client.ListOptions{ - Namespace: monitoringName.Namespace, - LabelSelector: selector, - }) - if err != nil { - return reconcile.Result{}, err - } - - for _, pvc := range pvcList.Items { - err = r.Client.Delete(ctx, &pvc) - if err != nil { - return reconcile.Result{}, err - } - } - return reconcile.Result{}, nil -} - -func (r *MonitoringReconciler) reconcileConfiguration(ctx context.Context) (ctrl.Result, error) { - cm, isCreate, err := r.monitoringConfigMap(ctx) - if err != nil { - return reconcile.Result{}, err - } - - if cm.Data == nil { - cm.Data = map[string]string{} - } - - configDataJSON, err := yaml.YAMLToJSON([]byte(cm.Data["config.yaml"])) - if err != nil { - return reconcile.Result{}, err - } - - var configData Config - err = codec.NewDecoderBytes(configDataJSON, r.jsonHandle).Decode(&configData) - if err != nil { - return reconcile.Result{}, err - } - - changed := false - - // Nil out the fields we don't want set - if configData.AlertManagerMain.VolumeClaimTemplate != nil { - configData.AlertManagerMain.VolumeClaimTemplate = nil - changed = true - } - - if configData.PrometheusK8s.Retention != "" { - configData.PrometheusK8s.Retention = "" - changed = true - } - - if configData.PrometheusK8s.VolumeClaimTemplate != nil { - configData.PrometheusK8s.VolumeClaimTemplate = nil - changed = true - } - - if !isCreate && !changed { - return reconcile.Result{}, nil - } - - var b []byte - err = codec.NewEncoderBytes(&b, r.jsonHandle).Encode(configData) - if err != nil { - return reconcile.Result{}, err - } - - cmYaml, err := yaml.JSONToYAML(b) - if err != nil { - return reconcile.Result{}, err - } - cm.Data["config.yaml"] = string(cmYaml) - - if isCreate { - r.Log.Infof("re-creating monitoring configmap. %s", monitoringName.Name) - err = r.Client.Create(ctx, cm) - } else { - r.Log.Infof("updating monitoring configmap. %s", monitoringName.Name) - err = r.Client.Update(ctx, cm) - } - return reconcile.Result{}, err -} - -func (r *MonitoringReconciler) monitoringConfigMap(ctx context.Context) (*corev1.ConfigMap, bool, error) { - cm := &corev1.ConfigMap{} - err := r.Client.Get(ctx, monitoringName, cm) - if kerrors.IsNotFound(err) { - return &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: monitoringName.Name, - Namespace: monitoringName.Namespace, - }, - Data: nil, - }, true, nil - } - if err != nil { - return nil, false, err - } - return cm, false, nil -} - -// SetupWithManager setup the manager -func (r *MonitoringReconciler) SetupWithManager(mgr ctrl.Manager) error { - r.Log.Info("starting cluster monitoring controller") - - monitoringConfigMapPredicate := predicate.NewPredicateFuncs(func(o client.Object) bool { - return o.GetName() == monitoringName.Name && o.GetNamespace() == monitoringName.Namespace - }) - - return ctrl.NewControllerManagedBy(mgr). - For(&arov1alpha1.Cluster{}, builder.WithPredicates(predicate.And(predicates.AROCluster, predicate.GenerationChangedPredicate{}))). - // https://github.com/kubernetes-sigs/controller-runtime/issues/1173 - // equivalent to For(&v1.ConfigMap{}, ...)., but can't call For multiple times on one builder - Watches( - &source.Kind{Type: &corev1.ConfigMap{}}, - &handler.EnqueueRequestForObject{}, - builder.WithPredicates(monitoringConfigMapPredicate), - ). - Named(ControllerName). - Complete(r) -} diff --git a/pkg/operator/controllers/monitoring/monitoring_controller_test.go b/pkg/operator/controllers/monitoring/monitoring_controller_test.go deleted file mode 100644 index 1b0065efc46..00000000000 --- a/pkg/operator/controllers/monitoring/monitoring_controller_test.go +++ /dev/null @@ -1,330 +0,0 @@ -package monitoring - -// Copyright (c) Microsoft Corporation. -// Licensed under the Apache License 2.0. - -import ( - "context" - "reflect" - "strings" - "testing" - - operatorv1 "github.com/openshift/api/operator/v1" - "github.com/sirupsen/logrus" - "github.com/ugorji/go/codec" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" - ctrlfake "sigs.k8s.io/controller-runtime/pkg/client/fake" - - "github.com/Azure/ARO-RP/pkg/operator" - arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1" - "github.com/Azure/ARO-RP/pkg/operator/controllers/base" - "github.com/Azure/ARO-RP/pkg/util/cmp" - _ "github.com/Azure/ARO-RP/pkg/util/scheme" - utilconditions "github.com/Azure/ARO-RP/test/util/conditions" -) - -var ( - cmMetadata = metav1.ObjectMeta{Name: "cluster-monitoring-config", Namespace: "openshift-monitoring"} -) - -func TestReconcileMonitoringConfig(t *testing.T) { - defaultAvailable := utilconditions.ControllerDefaultAvailable(ControllerName) - defaultProgressing := utilconditions.ControllerDefaultProgressing(ControllerName) - defaultDegraded := utilconditions.ControllerDefaultDegraded(ControllerName) - defaultConditions := []operatorv1.OperatorCondition{defaultAvailable, defaultProgressing, defaultDegraded} - log := logrus.NewEntry(logrus.StandardLogger()) - type test struct { - name string - configMap *corev1.ConfigMap - wantConfig string - wantConditions []operatorv1.OperatorCondition - } - - for _, tt := range []*test{ - { - name: "ConfigMap does not exist - enable", - wantConfig: `{}`, - wantConditions: defaultConditions, - }, - { - name: "empty config.yaml", - configMap: &corev1.ConfigMap{ - ObjectMeta: cmMetadata, - Data: map[string]string{ - "config.yaml": ``, - }, - }, - wantConfig: ``, - wantConditions: defaultConditions, - }, - { - name: "settings restored to default and extra fields are preserved", - configMap: &corev1.ConfigMap{ - ObjectMeta: cmMetadata, - Data: map[string]string{ - "config.yaml": ` -prometheusK8s: - extraField: prometheus - retention: 1d - volumeClaimTemplate: - metadata: - name: meh - spec: - resources: - requests: - storage: 50Gi - storageClassName: fast - volumeMode: Filesystem -alertmanagerMain: - extraField: yeet - volumeClaimTemplate: - metadata: - name: slowest-storage - spec: - resources: - requests: - storage: 50Gi - storageClassName: snail-mail - volumeMode: Filesystem -`, - }, - }, - wantConfig: ` -alertmanagerMain: - extraField: yeet -prometheusK8s: - extraField: prometheus -`, - wantConditions: defaultConditions, - }, - { - name: "empty volumeClaimTemplate struct is cleared out", - configMap: &corev1.ConfigMap{ - ObjectMeta: cmMetadata, - Data: map[string]string{ - "config.yaml": ` -alertmanagerMain: - volumeClaimTemplate: {} - extraField: alertmanager -prometheusK8s: - volumeClaimTemplate: {} - bugs: not-here -`, - }, - }, - wantConfig: ` -alertmanagerMain: - extraField: alertmanager -prometheusK8s: - bugs: not-here -`, - wantConditions: defaultConditions, - }, - { - name: "other monitoring components are configured", - configMap: &corev1.ConfigMap{ - ObjectMeta: cmMetadata, - Data: map[string]string{ - "config.yaml": ` -alertmanagerMain: - nodeSelector: - foo: bar -somethingElse: - configured: true -`, - }, - }, - wantConfig: ` -alertmanagerMain: - nodeSelector: - foo: bar -somethingElse: - configured: true -`, - wantConditions: defaultConditions, - }, - } { - t.Run(tt.name, func(t *testing.T) { - ctx := context.Background() - - instance := &arov1alpha1.Cluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: arov1alpha1.SingletonClusterName, - }, - Spec: arov1alpha1.ClusterSpec{ - OperatorFlags: arov1alpha1.OperatorFlags{ - operator.MonitoringEnabled: operator.FlagTrue, - }, - }, - } - - clientBuilder := ctrlfake.NewClientBuilder().WithObjects(instance) - if tt.configMap != nil { - clientBuilder.WithObjects(tt.configMap) - } - - r := &MonitoringReconciler{ - AROController: base.AROController{ - Log: log, - Client: clientBuilder.Build(), - }, - jsonHandle: new(codec.JsonHandle), - } - request := ctrl.Request{} - request.Name = "cluster-monitoring-config" - request.Namespace = "openshift-monitoring" - - _, err := r.Reconcile(ctx, request) - if err != nil { - t.Fatal(err) - } - - cm := &corev1.ConfigMap{} - err = r.Client.Get(ctx, types.NamespacedName{Namespace: "openshift-monitoring", Name: "cluster-monitoring-config"}, cm) - if err != nil { - t.Fatal(err) - } - - if strings.TrimSpace(cm.Data["config.yaml"]) != strings.TrimSpace(tt.wantConfig) { - t.Error(cm.Data["config.yaml"]) - } - }) - } -} - -func TestReconcilePVC(t *testing.T) { - defaultAvailable := utilconditions.ControllerDefaultAvailable(ControllerName) - defaultProgressing := utilconditions.ControllerDefaultProgressing(ControllerName) - defaultDegraded := utilconditions.ControllerDefaultDegraded(ControllerName) - defaultConditions := []operatorv1.OperatorCondition{defaultAvailable, defaultProgressing, defaultDegraded} - volumeMode := corev1.PersistentVolumeFilesystem - tests := []struct { - name string - pvcs []client.Object - want []corev1.PersistentVolumeClaim - wantConditions []operatorv1.OperatorCondition - }{ - { - name: "Should delete the prometheus PVCs", - pvcs: []client.Object{ - &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: "prometheus-k8s-db-prometheus-k8s-0", - Namespace: "openshift-monitoring", - Labels: map[string]string{ - "app": "prometheus", - "prometheus": "k8s", - }, - }, - }, - &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: "prometheus-k8s-db-prometheus-k8s-1", - Namespace: "openshift-monitoring", - Labels: map[string]string{ - "app": "prometheus", - "prometheus": "k8s", - }, - }, - }, - }, - want: []corev1.PersistentVolumeClaim{}, - wantConditions: defaultConditions, - }, - { - name: "Should preserve 1 pvc", - pvcs: []client.Object{ - &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: "prometheus-k8s-db-prometheus-k8s-0", - Namespace: "openshift-monitoring", - Labels: map[string]string{ - "app": "prometheus", - "prometheus": "k8s", - }, - }, - }, - &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: "random-pvc", - Namespace: "openshift-monitoring", - Labels: map[string]string{ - "app": "random", - }, - ResourceVersion: "1", - }, - }, - }, - want: []corev1.PersistentVolumeClaim{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "random-pvc", - Namespace: "openshift-monitoring", - Labels: map[string]string{ - "app": "random", - }, - ResourceVersion: "1", - }, - Spec: corev1.PersistentVolumeClaimSpec{ - VolumeMode: &volumeMode, - }, - Status: corev1.PersistentVolumeClaimStatus{ - Phase: corev1.ClaimPending, - }, - }, - }, - wantConditions: defaultConditions, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - ctx := context.Background() - - instance := &arov1alpha1.Cluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: arov1alpha1.SingletonClusterName, - }, - Spec: arov1alpha1.ClusterSpec{ - OperatorFlags: arov1alpha1.OperatorFlags{ - operator.MonitoringEnabled: operator.FlagTrue, - }, - }, - } - - clientFake := ctrlfake.NewClientBuilder().WithObjects(instance).WithObjects(tt.pvcs...).Build() - - r := &MonitoringReconciler{ - AROController: base.AROController{ - Log: logrus.NewEntry(logrus.StandardLogger()), - Client: clientFake, - }, - jsonHandle: new(codec.JsonHandle), - } - request := ctrl.Request{} - request.Name = "cluster-monitoring-config" - request.Namespace = "openshift-monitoring" - - _, err := r.Reconcile(ctx, request) - if err != nil { - t.Fatal(err) - } - - pvcList := &corev1.PersistentVolumeClaimList{} - err = r.Client.List(ctx, pvcList, &client.ListOptions{ - Namespace: monitoringName.Namespace, - }) - if err != nil { - t.Fatalf("Unexpected error during list of PVCs: %v", err) - } - - if !reflect.DeepEqual(pvcList.Items, tt.want) { - t.Error(cmp.Diff(pvcList.Items, tt.want)) - } - }) - } -} diff --git a/test/e2e/operator.go b/test/e2e/operator.go index 097ac9d149d..00a9bef1acd 100644 --- a/test/e2e/operator.go +++ b/test/e2e/operator.go @@ -22,20 +22,17 @@ import ( machinev1beta1 "github.com/openshift/api/machine/v1beta1" cov1Helpers "github.com/openshift/library-go/pkg/config/clusteroperator/v1helpers" mcv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" - "github.com/ugorji/go/codec" corev1 "k8s.io/api/core/v1" kerrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/discovery" "k8s.io/client-go/util/retry" - "sigs.k8s.io/yaml" apisubnet "github.com/Azure/ARO-RP/pkg/api/util/subnet" "github.com/Azure/ARO-RP/pkg/operator" arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1" cpcController "github.com/Azure/ARO-RP/pkg/operator/controllers/cloudproviderconfig" imageController "github.com/Azure/ARO-RP/pkg/operator/controllers/imageconfig" - "github.com/Azure/ARO-RP/pkg/operator/controllers/monitoring" subnetController "github.com/Azure/ARO-RP/pkg/operator/controllers/subnets" "github.com/Azure/ARO-RP/pkg/util/conditions" "github.com/Azure/ARO-RP/pkg/util/ready" @@ -179,45 +176,6 @@ var _ = Describe("ARO Operator - Geneva Logging", func() { }) }) -var _ = Describe("ARO Operator - Cluster Monitoring ConfigMap", func() { - It("must not have persistent volume set", func(ctx context.Context) { - var cm *corev1.ConfigMap - getFunc := clients.Kubernetes.CoreV1().ConfigMaps("openshift-monitoring").Get - - By("waiting for the ConfigMap to make sure it exists") - cm = GetK8sObjectWithRetry(ctx, getFunc, "cluster-monitoring-config", metav1.GetOptions{}) - - By("unmarshalling the config from the ConfigMap data") - var configData monitoring.Config - configDataJSON, err := yaml.YAMLToJSON([]byte(cm.Data["config.yaml"])) - Expect(err).NotTo(HaveOccurred()) - - err = codec.NewDecoderBytes(configDataJSON, &codec.JsonHandle{}).Decode(&configData) - if err != nil { - log.Warn(err) - } - - By("checking config correctness") - Expect(configData.PrometheusK8s.Retention).To(BeEmpty()) - Expect(configData.PrometheusK8s.VolumeClaimTemplate).To(BeNil()) - Expect(configData.AlertManagerMain.VolumeClaimTemplate).To(BeNil()) - }) - - It("must be restored if deleted", func(ctx context.Context) { - getFunc := clients.Kubernetes.CoreV1().ConfigMaps("openshift-monitoring").Get - deleteFunc := clients.Kubernetes.CoreV1().ConfigMaps("openshift-monitoring").Delete - - By("waiting for the ConfigMap to make sure it exists") - GetK8sObjectWithRetry(ctx, getFunc, "cluster-monitoring-config", metav1.GetOptions{}) - - By("deleting for the ConfigMap") - DeleteK8sObjectWithRetry(ctx, deleteFunc, "cluster-monitoring-config", metav1.DeleteOptions{}) - - By("waiting for the ConfigMap to make sure it was restored") - GetK8sObjectWithRetry(ctx, getFunc, "cluster-monitoring-config", metav1.GetOptions{}) - }) -}) - var _ = Describe("ARO Operator - RBAC", func() { It("must restore system:aro-sre ClusterRole if deleted", func(ctx context.Context) { getFunc := clients.Kubernetes.RbacV1().ClusterRoles().Get