From 8e824946f651b399421505882db337c23b22ab92 Mon Sep 17 00:00:00 2001 From: Thibault Mange <22740367+thibaultmg@users.noreply.github.com> Date: Mon, 29 Apr 2024 11:51:04 +0200 Subject: [PATCH] ACM-10828: sort resources for microshift install (#1395) * sort resources Signed-off-by: Thibault Mange <22740367+thibaultmg@users.noreply.github.com> * add addon finalizers perm Signed-off-by: Thibault Mange <22740367+thibaultmg@users.noreply.github.com> * fix permissions and watcher Signed-off-by: Thibault Mange <22740367+thibaultmg@users.noreply.github.com> * add roles exception for ownerref Signed-off-by: Thibault Mange <22740367+thibaultmg@users.noreply.github.com> * fix cache noisy logs Signed-off-by: Thibault Mange <22740367+thibaultmg@users.noreply.github.com> * fix globalRes Signed-off-by: Thibault Mange <22740367+thibaultmg@users.noreply.github.com> * clean some logs Signed-off-by: Thibault Mange <22740367+thibaultmg@users.noreply.github.com> * update depoyer update logs Signed-off-by: Thibault Mange <22740367+thibaultmg@users.noreply.github.com> --------- Signed-off-by: Thibault Mange <22740367+thibaultmg@users.noreply.github.com> --- .../observabilityaddon_controller.go | 35 +++++++++++++------ operators/endpointmetrics/main.go | 8 +++-- .../prometheus/prometheus-operator-role.yaml | 1 + .../endpointmetrics/pkg/rendering/renderer.go | 19 ++++++++++ .../pkg/rendering/renderer_test.go | 8 +++++ .../endpoint-observability/role.yaml | 6 ++++ operators/pkg/deploying/deployer.go | 34 +++++++++--------- 7 files changed, 83 insertions(+), 28 deletions(-) diff --git a/operators/endpointmetrics/controllers/observabilityendpoint/observabilityaddon_controller.go b/operators/endpointmetrics/controllers/observabilityendpoint/observabilityaddon_controller.go index 13a1f27d2..571ae5700 100644 --- a/operators/endpointmetrics/controllers/observabilityendpoint/observabilityaddon_controller.go +++ b/operators/endpointmetrics/controllers/observabilityendpoint/observabilityaddon_controller.go @@ -240,14 +240,24 @@ func (r *ObservabilityAddonReconciler) Reconcile(ctx context.Context, req ctrl.R } deployer := deploying.NewDeployer(r.Client) for _, res := range toDeploy { + if res.GetNamespace() != namespace { + globalRes = append(globalRes, res) + } + if !isHubMetricsCollector { // For kind tests we need to deploy prometheus in hub but cannot set controller // reference as there is no observabilityaddon - if err := controllerutil.SetControllerReference(obsAddon, res, r.Scheme); err != nil { - log.Info("Failed to set controller reference", "resource", res.GetName()) - globalRes = append(globalRes, res) + + // skip setting controller reference for resources that don't need it + // and for which we lack permission to set it + skipResources := []string{"Role", "RoleBinding", "ClusterRole", "ClusterRoleBinding"} + if !slices.Contains(skipResources, res.GetKind()) { + if err := controllerutil.SetControllerReference(obsAddon, res, r.Scheme); err != nil { + log.Info("Failed to set controller reference", "resource", res.GetName(), "kind", res.GetKind(), "error", err.Error()) + } } } + if err := deployer.Deploy(res); err != nil { return ctrl.Result{}, fmt.Errorf("failed to deploy %s %s/%s: %w", res.GetKind(), namespace, res.GetName(), err) } @@ -475,15 +485,20 @@ func (r *ObservabilityAddonReconciler) SetupWithManager(mgr ctrl.Manager) error if os.Getenv("NAMESPACE") != "" { namespace = os.Getenv("NAMESPACE") } - return ctrl.NewControllerManagedBy(mgr). - For( - &oav1beta1.ObservabilityAddon{}, - builder.WithPredicates(getPred(obAddonName, namespace, true, true, true)), - ). - Watches( + + ctrlBuilder := ctrl.NewControllerManagedBy(mgr).For( + &oav1beta1.ObservabilityAddon{}, + builder.WithPredicates(getPred(obAddonName, namespace, true, true, true)), + ) + + if isHubMetricsCollector { + ctrlBuilder = ctrlBuilder.Watches( &source.Kind{Type: &oav1beta2.MultiClusterObservability{}}, &handler.EnqueueRequestForObject{}, - ). + ) + } + + return ctrlBuilder. Watches( &source.Kind{Type: &corev1.Secret{}}, &handler.EnqueueRequestForObject{}, diff --git a/operators/endpointmetrics/main.go b/operators/endpointmetrics/main.go index d15076214..6aa9ba5ec 100644 --- a/operators/endpointmetrics/main.go +++ b/operators/endpointmetrics/main.go @@ -99,9 +99,13 @@ func main() { oav1beta1.GroupVersion.WithKind("ObservabilityAddon"): { {FieldSelector: namespaceSelector}, }, - oav1beta2.GroupVersion.WithKind("MultiClusterObservability"): { + } + + // Only watch MCO CRs in the hub cluster to avoid noisy log messages + if os.Getenv("HUB_ENDPOINT_OPERATOR") == "true" { + gvkLabelMap[oav1beta2.GroupVersion.WithKind("MultiClusterObservability")] = []filteredcache.Selector{ {FieldSelector: "metadata.name!=null"}, - }, + } } mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ diff --git a/operators/endpointmetrics/manifests/prometheus/prometheus-operator-role.yaml b/operators/endpointmetrics/manifests/prometheus/prometheus-operator-role.yaml index 5565735e1..fc7b87a6c 100644 --- a/operators/endpointmetrics/manifests/prometheus/prometheus-operator-role.yaml +++ b/operators/endpointmetrics/manifests/prometheus/prometheus-operator-role.yaml @@ -16,6 +16,7 @@ rules: - alertmanagerconfigs - prometheuses - prometheuses/finalizers + - prometheuses/status - thanosrulers - thanosrulers/finalizers - servicemonitors diff --git a/operators/endpointmetrics/pkg/rendering/renderer.go b/operators/endpointmetrics/pkg/rendering/renderer.go index 0d23a9188..254361968 100644 --- a/operators/endpointmetrics/pkg/rendering/renderer.go +++ b/operators/endpointmetrics/pkg/rendering/renderer.go @@ -12,6 +12,7 @@ import ( "strings" prometheusv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" + "golang.org/x/exp/slices" v1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -250,9 +251,27 @@ func Render( } } + // Ordering resources to ensure they are applied in the correct order + slices.SortFunc(resources, func(a, b *unstructured.Unstructured) bool { + return (resourcePriority(a) - resourcePriority(b)) < 0 + }) + return resources, nil } +func resourcePriority(resource *unstructured.Unstructured) int { + switch resource.GetKind() { + case "Role", "ClusterRole": + return 1 + case "RoleBinding", "ClusterRoleBinding": + return 2 + case "CustomResourceDefinition": + return 3 + default: + return 4 + } +} + func getDisabledMetrics(c runtimeclient.Client) (string, error) { cm := &corev1.ConfigMap{} err := c.Get(context.TODO(), types.NamespacedName{Name: operatorconfig.AllowlistConfigMapName, diff --git a/operators/endpointmetrics/pkg/rendering/renderer_test.go b/operators/endpointmetrics/pkg/rendering/renderer_test.go index e0126ba41..e922ef0b7 100644 --- a/operators/endpointmetrics/pkg/rendering/renderer_test.go +++ b/operators/endpointmetrics/pkg/rendering/renderer_test.go @@ -58,6 +58,14 @@ func TestRender(t *testing.T) { } printObjs(t, objs) + + // ensure that objects are sorted + for i := 0; i < len(objs)-1; i++ { + if resourcePriority(objs[i]) > resourcePriority(objs[i+1]) { + t.Errorf("objects are not sorted") + } + } + } func printObjs(t *testing.T, objs []*unstructured.Unstructured) { diff --git a/operators/multiclusterobservability/manifests/endpoint-observability/role.yaml b/operators/multiclusterobservability/manifests/endpoint-observability/role.yaml index 6e63e7da9..e7f38eae3 100644 --- a/operators/multiclusterobservability/manifests/endpoint-observability/role.yaml +++ b/operators/multiclusterobservability/manifests/endpoint-observability/role.yaml @@ -63,6 +63,12 @@ rules: verbs: - get - update +- apiGroups: + - observability.open-cluster-management.io + resources: + - observabilityaddons/finalizers + verbs: + - update - apiGroups: - config.openshift.io resources: diff --git a/operators/pkg/deploying/deployer.go b/operators/pkg/deploying/deployer.go index 2b2bac700..f71d64a4d 100644 --- a/operators/pkg/deploying/deployer.go +++ b/operators/pkg/deploying/deployer.go @@ -66,7 +66,7 @@ func (d *Deployer) Deploy(obj *unstructured.Unstructured) error { ) if err != nil { if errors.IsNotFound(err) { - log.Info("Create", "Kind:", obj.GroupVersionKind(), "Name:", obj.GetName()) + log.Info("Create", "Kind", obj.GroupVersionKind(), "Name", obj.GetName()) return d.client.Create(context.TODO(), obj) } return err @@ -78,7 +78,7 @@ func (d *Deployer) Deploy(obj *unstructured.Unstructured) error { annotations, ok := metadata["annotations"].(map[string]interface{}) if ok && annotations != nil && annotations[config.AnnotationSkipCreation] != nil { if strings.ToLower(annotations[config.AnnotationSkipCreation].(string)) == "true" { - log.Info("Skip creation", "Kind:", obj.GroupVersionKind(), "Name:", obj.GetName()) + log.Info("Skip creation", "Kind", obj.GroupVersionKind(), "Name", obj.GetName()) return nil } } @@ -88,7 +88,7 @@ func (d *Deployer) Deploy(obj *unstructured.Unstructured) error { if ok { return deployerFn(obj, found) } else { - log.Info("deployerFn not found", "kind:", found.GetKind()) + log.Info("deployerFn not found", "kind", found.GetKind()) } return nil } @@ -109,7 +109,7 @@ func (d *Deployer) updateDeployment(desiredObj, runtimeObj *unstructured.Unstruc } if !apiequality.Semantic.DeepDerivative(desiredDepoly.Spec, runtimeDepoly.Spec) { - log.Info("Update", "Kind:", runtimeObj.GroupVersionKind(), "Name:", runtimeObj.GetName()) + logUpdateInfo(runtimeObj) return d.client.Update(context.TODO(), desiredDepoly) } @@ -133,7 +133,7 @@ func (d *Deployer) updateStatefulSet(desiredObj, runtimeObj *unstructured.Unstru if !apiequality.Semantic.DeepDerivative(desiredDepoly.Spec.Template, runtimeDepoly.Spec.Template) || !apiequality.Semantic.DeepDerivative(desiredDepoly.Spec.Replicas, runtimeDepoly.Spec.Replicas) { - log.Info("Update", "Kind:", runtimeObj.GroupVersionKind(), "Name:", runtimeObj.GetName()) + logUpdateInfo(runtimeObj) runtimeDepoly.Spec.Replicas = desiredDepoly.Spec.Replicas runtimeDepoly.Spec.Template = desiredDepoly.Spec.Template return d.client.Update(context.TODO(), runtimeDepoly) @@ -160,7 +160,7 @@ func (d *Deployer) updateService(desiredObj, runtimeObj *unstructured.Unstructur if !apiequality.Semantic.DeepDerivative(desiredService.Spec, runtimeService.Spec) { desiredService.ObjectMeta.ResourceVersion = runtimeService.ObjectMeta.ResourceVersion desiredService.Spec.ClusterIP = runtimeService.Spec.ClusterIP - log.Info("Update", "Kind:", runtimeObj.GroupVersionKind(), "Name:", runtimeObj.GetName()) + logUpdateInfo(runtimeObj) return d.client.Update(context.TODO(), desiredService) } @@ -183,7 +183,7 @@ func (d *Deployer) updateConfigMap(desiredObj, runtimeObj *unstructured.Unstruct } if !apiequality.Semantic.DeepDerivative(desiredConfigMap.Data, runtimeConfigMap.Data) { - log.Info("Update", "Kind:", runtimeObj.GroupVersionKind(), "Name:", runtimeObj.GetName()) + logUpdateInfo(runtimeObj) return d.client.Update(context.TODO(), desiredConfigMap) } @@ -207,7 +207,7 @@ func (d *Deployer) updateSecret(desiredObj, runtimeObj *unstructured.Unstructure if desiredSecret.Data == nil || !apiequality.Semantic.DeepDerivative(desiredSecret.Data, runtimeSecret.Data) { - log.Info("Update", "Kind:", desiredObj.GroupVersionKind(), "Name:", desiredObj.GetName()) + logUpdateInfo(desiredObj) return d.client.Update(context.TODO(), desiredSecret) } return nil @@ -230,7 +230,7 @@ func (d *Deployer) updateClusterRole(desiredObj, runtimeObj *unstructured.Unstru if !apiequality.Semantic.DeepDerivative(desiredClusterRole.Rules, runtimeClusterRole.Rules) || !apiequality.Semantic.DeepDerivative(desiredClusterRole.AggregationRule, runtimeClusterRole.AggregationRule) { - log.Info("Update", "Kind:", desiredObj.GroupVersionKind(), "Name:", desiredObj.GetName()) + logUpdateInfo(desiredObj) return d.client.Update(context.TODO(), desiredClusterRole) } return nil @@ -253,7 +253,7 @@ func (d *Deployer) updateClusterRoleBinding(desiredObj, runtimeObj *unstructured if !apiequality.Semantic.DeepDerivative(desiredClusterRoleBinding.Subjects, runtimeClusterRoleBinding.Subjects) || !apiequality.Semantic.DeepDerivative(desiredClusterRoleBinding.RoleRef, runtimeClusterRoleBinding.RoleRef) { - log.Info("Update", "Kind:", desiredObj.GroupVersionKind(), "Name:", desiredObj.GetName()) + logUpdateInfo(desiredObj) return d.client.Update(context.TODO(), desiredClusterRoleBinding) } return nil @@ -276,7 +276,7 @@ func (d *Deployer) updateCRD(desiredObj, runtimeObj *unstructured.Unstructured) desiredCRD.ObjectMeta.ResourceVersion = runtimeCRD.ObjectMeta.ResourceVersion if !apiequality.Semantic.DeepDerivative(desiredCRD.Spec, runtimeCRD.Spec) { - log.Info("Update", "Kind:", runtimeObj.GroupVersionKind(), "Name:", runtimeObj.GetName()) + logUpdateInfo(runtimeObj) return d.client.Update(context.TODO(), desiredCRD) } @@ -306,8 +306,6 @@ func (d *Deployer) updatePrometheus(desiredObj, runtimeObj *unstructured.Unstruc // 2. delete endpoint operator pod // inherit resource version if not specified - log.Info("Desired Prometheus", "resourceVersion", desiredPrometheus.ResourceVersion) - log.Info("Runtime Prometheus", "resourceVersion", runtimePrometheus.ResourceVersion) if desiredPrometheus.ResourceVersion != runtimePrometheus.ResourceVersion { desiredPrometheus.ResourceVersion = runtimePrometheus.ResourceVersion } @@ -327,7 +325,7 @@ func (d *Deployer) updatePrometheus(desiredObj, runtimeObj *unstructured.Unstruc } if !apiequality.Semantic.DeepDerivative(desiredPrometheus.Spec, runtimePrometheus.Spec) { - log.Info("Update", "Kind:", runtimeObj.GroupVersionKind(), "Name:", runtimeObj.GetName()) + logUpdateInfo(runtimeObj) return d.client.Update(context.TODO(), desiredPrometheus) } else { log.Info("Runtime Prometheus and Desired Prometheus are semantically equal!") @@ -351,7 +349,7 @@ func (d *Deployer) updatePrometheusRule(desiredObj, runtimeObj *unstructured.Uns } if !apiequality.Semantic.DeepDerivative(desiredPrometheusRule.Spec, runtimePrometheusRule.Spec) { - log.Info("Update", "Kind:", runtimeObj.GroupVersionKind(), "Name:", runtimeObj.GetName()) + logUpdateInfo(runtimeObj) if desiredPrometheusRule.ResourceVersion != runtimePrometheusRule.ResourceVersion { desiredPrometheusRule.ResourceVersion = runtimePrometheusRule.ResourceVersion } @@ -377,9 +375,13 @@ func (d *Deployer) updateIngress(desiredObj, runtimeObj *unstructured.Unstructur } if !apiequality.Semantic.DeepDerivative(desiredIngress.Spec, runtimeIngress.Spec) { - log.Info("Update", "Kind:", runtimeObj.GroupVersionKind(), "Name:", runtimeObj.GetName()) + logUpdateInfo(runtimeObj) return d.client.Update(context.TODO(), desiredIngress) } return nil } + +func logUpdateInfo(obj *unstructured.Unstructured) { + log.Info("Update", "kind", obj.GroupVersionKind().Kind, "kindVersion", obj.GroupVersionKind().Version, "name", obj.GetName()) +}