From 03b79a8302af2b2b7b0405cb0646b99175fb54a5 Mon Sep 17 00:00:00 2001 From: Robert Fratto Date: Mon, 25 Oct 2021 11:43:48 -0400 Subject: [PATCH 1/2] pkg/operator: only delete managed resources Also ensures that all managed resources have the managed-by label --- CHANGELOG.md | 7 +++++++ pkg/operator/reconciler.go | 3 +++ pkg/operator/reconciler_logs.go | 2 +- pkg/operator/reconciler_metrics.go | 7 ++++--- pkg/operator/resources_logs.go | 1 + pkg/operator/resources_metrics.go | 32 +++++++++++++++++++++--------- 6 files changed, 39 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 35735f5130c7..a2b8700bac25 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,13 @@ - [BUGFIX] Updated envsubst to v2.0.0-20210730161058-179042472c46. This version has a fix needed for escaping values outside of variable substitutions. (@rlankfo) +- [BUGFIX] Grafana Agent Operator should no longer delete resources matching + the names of the resources it manages. (@rfratto) + +- [BUGFIX] Grafana Agent Operator will now appropriately assign an + `app.kubernetes.io/managed-by=grafana-agent-operator` to all created + resources. + - [CHANGE] Configuration API now returns 404 instead of 400 when attempting to get or delete a config which does not exist. (@kgeckhart) diff --git a/pkg/operator/reconciler.go b/pkg/operator/reconciler.go index 60168ae0bd76..894e8c7b01b0 100644 --- a/pkg/operator/reconciler.go +++ b/pkg/operator/reconciler.go @@ -201,6 +201,9 @@ func (r *reconciler) createSecrets( Name: d.Agent.Name, UID: d.Agent.UID, }}, + Labels: map[string]string{ + managedByOperatorLabel: managedByOperatorLabelValue, + }, }, Data: data, } diff --git a/pkg/operator/reconciler_logs.go b/pkg/operator/reconciler_logs.go index 869ac4d24bc6..a687fc632fc9 100644 --- a/pkg/operator/reconciler_logs.go +++ b/pkg/operator/reconciler_logs.go @@ -43,7 +43,7 @@ func (r *reconciler) createLogsDaemonSet( var ds apps_v1.DaemonSet err := r.Client.Get(ctx, key, &ds) - if k8s_errors.IsNotFound(err) { + if k8s_errors.IsNotFound(err) || !isManagedResource(&ds) { return nil } else if err != nil { return fmt.Errorf("failed to find stale DaemonSet %s: %w", key, err) diff --git a/pkg/operator/reconciler_metrics.go b/pkg/operator/reconciler_metrics.go index 01cd04228202..9b2b56618e4e 100644 --- a/pkg/operator/reconciler_metrics.go +++ b/pkg/operator/reconciler_metrics.go @@ -58,7 +58,7 @@ func (r *reconciler) createTelemetryConfigurationSecret( if !shouldCreate { var secret core_v1.Secret err := r.Client.Get(ctx, key, &secret) - if k8s_errors.IsNotFound(err) { + if k8s_errors.IsNotFound(err) || !isManagedResource(&secret) { return nil } else if err != nil { return fmt.Errorf("failed to find stale secret %s: %w", key, err) @@ -125,7 +125,7 @@ func (r *reconciler) createMetricsGoverningService( var service core_v1.Service err := r.Client.Get(ctx, key, &service) - if k8s_errors.IsNotFound(err) { + if k8s_errors.IsNotFound(err) || !isManagedResource(&service) { return nil } else if err != nil { return fmt.Errorf("failed to find stale Service %s: %w", key, err) @@ -191,7 +191,8 @@ func (r *reconciler) createMetricsStatefulSets( var statefulSets apps_v1.StatefulSetList err := r.List(ctx, &statefulSets, &client.ListOptions{ LabelSelector: labels.SelectorFromSet(labels.Set{ - agentNameLabelName: d.Agent.Name, + managedByOperatorLabel: managedByOperatorLabelValue, + agentNameLabelName: d.Agent.Name, }), }) if err != nil { diff --git a/pkg/operator/resources_logs.go b/pkg/operator/resources_logs.go index d25f17a6327b..5a522671ee75 100644 --- a/pkg/operator/resources_logs.go +++ b/pkg/operator/resources_logs.go @@ -44,6 +44,7 @@ func generateLogsDaemonSet( } labels[agentNameLabelName] = d.Agent.Name labels[agentTypeLabel] = "logs" + labels[managedByOperatorLabel] = managedByOperatorLabelValue boolTrue := true ds := &apps_v1.DaemonSet{ diff --git a/pkg/operator/resources_metrics.go b/pkg/operator/resources_metrics.go index 185bb9d5cd87..8c862e185b9a 100644 --- a/pkg/operator/resources_metrics.go +++ b/pkg/operator/resources_metrics.go @@ -12,6 +12,7 @@ import ( v1 "k8s.io/api/core/v1" meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" + "sigs.k8s.io/controller-runtime/pkg/client" ) const ( @@ -33,6 +34,17 @@ var ( probeTimeoutSeconds int32 = 3 ) +// isManagedResource returns true if the given object has a managed-by +// grafana-agent-operator label. +func isManagedResource(obj client.Object) bool { + for key, value := range obj.GetLabels() { + if key == managedByOperatorLabel && value == managedByOperatorLabelValue { + return true + } + } + return false +} + func generateMetricsStatefulSetService(cfg *Config, d config.Deployment) *v1.Service { d = *d.DeepCopy() @@ -55,7 +67,8 @@ func generateMetricsStatefulSetService(cfg *Config, d config.Deployment) *v1.Ser UID: d.Agent.UID, }}, Labels: cfg.Labels.Merge(map[string]string{ - "operated-agent": "true", + managedByOperatorLabel: managedByOperatorLabelValue, + "operated-agent": "true", }), }, Spec: v1.ServiceSpec{ @@ -120,6 +133,7 @@ func generateMetricsStatefulSet( } labels[agentNameLabelName] = d.Agent.Name labels[agentTypeLabel] = "metrics" + labels[managedByOperatorLabel] = managedByOperatorLabelValue boolTrue := true @@ -314,14 +328,14 @@ func generateMetricsStatefulSetSpec( podAnnotations := map[string]string{} podLabels := map[string]string{} podSelectorLabels := map[string]string{ - "app.kubernetes.io/name": "grafana-agent", - "app.kubernetes.io/version": build.Version, - "app.kubernetes.io/managed-by": "grafana-agent-operator", - "app.kubernetes.io/instance": d.Agent.Name, - "grafana-agent": d.Agent.Name, - shardLabelName: fmt.Sprintf("%d", shard), - agentNameLabelName: d.Agent.Name, - agentTypeLabel: "metrics", + "app.kubernetes.io/name": "grafana-agent", + "app.kubernetes.io/version": build.Version, + "app.kubernetes.io/instance": d.Agent.Name, + "grafana-agent": d.Agent.Name, + managedByOperatorLabel: managedByOperatorLabelValue, + shardLabelName: fmt.Sprintf("%d", shard), + agentNameLabelName: d.Agent.Name, + agentTypeLabel: "metrics", } if d.Agent.Spec.PodMetadata != nil { for k, v := range d.Agent.Spec.PodMetadata.Labels { From 5f5d5824c634e75b1d4ee95eaf46d2ef2792ad05 Mon Sep 17 00:00:00 2001 From: Robert Fratto Date: Mon, 25 Oct 2021 12:48:10 -0400 Subject: [PATCH 2/2] optimize isManagedResource --- pkg/operator/resources_metrics.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/pkg/operator/resources_metrics.go b/pkg/operator/resources_metrics.go index 8c862e185b9a..f649fc4be25c 100644 --- a/pkg/operator/resources_metrics.go +++ b/pkg/operator/resources_metrics.go @@ -37,12 +37,8 @@ var ( // isManagedResource returns true if the given object has a managed-by // grafana-agent-operator label. func isManagedResource(obj client.Object) bool { - for key, value := range obj.GetLabels() { - if key == managedByOperatorLabel && value == managedByOperatorLabelValue { - return true - } - } - return false + labelValue := obj.GetLabels()[managedByOperatorLabel] + return labelValue == managedByOperatorLabelValue } func generateMetricsStatefulSetService(cfg *Config, d config.Deployment) *v1.Service {