diff --git a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml index 938f63344a..5d02664177 100644 --- a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml +++ b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml @@ -5558,6 +5558,7 @@ spec: configVersions: format: int32 type: integer + default: 3 configmaps: items: properties: diff --git a/config/manager/kustomization.yaml b/config/manager/kustomization.yaml index 9396d7d702..79a1399047 100644 --- a/config/manager/kustomization.yaml +++ b/config/manager/kustomization.yaml @@ -5,4 +5,4 @@ kind: Kustomization images: - name: controller newName: localhost:5001/opentelemetry-operator - newTag: matth-dev-1714578767 + newTag: matth-dev-1714590016 diff --git a/controllers/builder_test.go b/controllers/builder_test.go index 5cf49c9cde..2f612e2108 100644 --- a/controllers/builder_test.go +++ b/controllers/builder_test.go @@ -37,6 +37,7 @@ import ( "github.com/open-telemetry/opentelemetry-operator/internal/config" "github.com/open-telemetry/opentelemetry-operator/internal/manifests" "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/manifestutils" "github.com/open-telemetry/opentelemetry-operator/pkg/featuregate" ) @@ -86,6 +87,10 @@ service: goodConfig := v1beta1.Config{} err := go_yaml.Unmarshal([]byte(goodConfigYaml), &goodConfig) require.NoError(t, err) + + goodConfigHash, _ := manifestutils.GetConfigMapSHA(goodConfig) + goodConfigHash = goodConfigHash[:8] + one := int32(1) type args struct { instance v1beta1.OpenTelemetryCollector @@ -164,7 +169,7 @@ service: VolumeSource: corev1.VolumeSource{ ConfigMap: &corev1.ConfigMapVolumeSource{ LocalObjectReference: corev1.LocalObjectReference{ - Name: "test-collector", + Name: "test-collector-" + goodConfigHash, }, Items: []corev1.KeyToPath{ { @@ -223,13 +228,13 @@ service: }, &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-collector", + Name: "test-collector-" + goodConfigHash, Namespace: "test", Labels: map[string]string{ "app.kubernetes.io/component": "opentelemetry-collector", "app.kubernetes.io/instance": "test.test", "app.kubernetes.io/managed-by": "opentelemetry-operator", - "app.kubernetes.io/name": "test-collector", + "app.kubernetes.io/name": "test-collector-" + goodConfigHash, "app.kubernetes.io/part-of": "opentelemetry", "app.kubernetes.io/version": "latest", }, @@ -411,7 +416,7 @@ service: VolumeSource: corev1.VolumeSource{ ConfigMap: &corev1.ConfigMapVolumeSource{ LocalObjectReference: corev1.LocalObjectReference{ - Name: "test-collector", + Name: "test-collector-" + goodConfigHash, }, Items: []corev1.KeyToPath{ { @@ -470,13 +475,13 @@ service: }, &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-collector", + Name: "test-collector-" + goodConfigHash, Namespace: "test", Labels: map[string]string{ "app.kubernetes.io/component": "opentelemetry-collector", "app.kubernetes.io/instance": "test.test", "app.kubernetes.io/managed-by": "opentelemetry-operator", - "app.kubernetes.io/name": "test-collector", + "app.kubernetes.io/name": "test-collector-" + goodConfigHash, "app.kubernetes.io/part-of": "opentelemetry", "app.kubernetes.io/version": "latest", }, @@ -694,7 +699,7 @@ service: VolumeSource: corev1.VolumeSource{ ConfigMap: &corev1.ConfigMapVolumeSource{ LocalObjectReference: corev1.LocalObjectReference{ - Name: "test-collector", + Name: "test-collector-" + goodConfigHash, }, Items: []corev1.KeyToPath{ { @@ -753,13 +758,13 @@ service: }, &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-collector", + Name: "test-collector-" + goodConfigHash, Namespace: "test", Labels: map[string]string{ "app.kubernetes.io/component": "opentelemetry-collector", "app.kubernetes.io/instance": "test.test", "app.kubernetes.io/managed-by": "opentelemetry-operator", - "app.kubernetes.io/name": "test-collector", + "app.kubernetes.io/name": "test-collector-" + goodConfigHash, "app.kubernetes.io/part-of": "opentelemetry", "app.kubernetes.io/version": "latest", }, @@ -1129,6 +1134,10 @@ service: goodConfig := v1beta1.Config{} err := go_yaml.Unmarshal([]byte(goodConfigYaml), &goodConfig) require.NoError(t, err) + + goodConfigHash, _ := manifestutils.GetConfigMapSHA(goodConfig) + goodConfigHash = goodConfigHash[:8] + one := int32(1) type args struct { instance v1beta1.OpenTelemetryCollector @@ -1216,7 +1225,7 @@ service: VolumeSource: corev1.VolumeSource{ ConfigMap: &corev1.ConfigMapVolumeSource{ LocalObjectReference: corev1.LocalObjectReference{ - Name: "test-collector", + Name: "test-collector-" + goodConfigHash, }, Items: []corev1.KeyToPath{ { @@ -1275,13 +1284,13 @@ service: }, &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-collector", + Name: "test-collector-" + goodConfigHash, Namespace: "test", Labels: map[string]string{ "app.kubernetes.io/component": "opentelemetry-collector", "app.kubernetes.io/instance": "test.test", "app.kubernetes.io/managed-by": "opentelemetry-operator", - "app.kubernetes.io/name": "test-collector", + "app.kubernetes.io/name": "test-collector-" + goodConfigHash, "app.kubernetes.io/part-of": "opentelemetry", "app.kubernetes.io/version": "latest", }, @@ -1610,7 +1619,7 @@ prometheus_cr: VolumeSource: corev1.VolumeSource{ ConfigMap: &corev1.ConfigMapVolumeSource{ LocalObjectReference: corev1.LocalObjectReference{ - Name: "test-collector", + Name: "test-collector-" + goodConfigHash, }, Items: []corev1.KeyToPath{ { @@ -1669,13 +1678,13 @@ prometheus_cr: }, &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-collector", + Name: "test-collector-" + goodConfigHash, Namespace: "test", Labels: map[string]string{ "app.kubernetes.io/component": "opentelemetry-collector", "app.kubernetes.io/instance": "test.test", "app.kubernetes.io/managed-by": "opentelemetry-operator", - "app.kubernetes.io/name": "test-collector", + "app.kubernetes.io/name": "test-collector-" + goodConfigHash, "app.kubernetes.io/part-of": "opentelemetry", "app.kubernetes.io/version": "latest", }, diff --git a/controllers/opentelemetrycollector_controller.go b/controllers/opentelemetrycollector_controller.go index 258c5fadfc..e309a1f943 100644 --- a/controllers/opentelemetrycollector_controller.go +++ b/controllers/opentelemetrycollector_controller.go @@ -18,6 +18,7 @@ package controllers import ( "context" "fmt" + "sort" "github.com/go-logr/logr" routev1 "github.com/openshift/api/route/v1" @@ -127,6 +128,30 @@ func (r *OpenTelemetryCollectorReconciler) findOtelOwnedObjects(ctx context.Cont ownedObjects[pdbList.Items[i].GetUID()] = &pdbList.Items[i] } + configMapList := &corev1.ConfigMapList{} + err = r.List(ctx, configMapList, listOps) + if err != nil { + return nil, fmt.Errorf("error listing ConfigMaps: %w", err) + } + + // only return collector ConfigMaps older than spec.ConfigVersions + sort.Slice(configMapList.Items, func(i, j int) bool { + iTime := configMapList.Items[i].GetCreationTimestamp().Time + jTime := configMapList.Items[j].GetCreationTimestamp().Time + // sort the ConfigMaps newest to oldest + return iTime.After(jTime) + }) + configVersionsToKeep := 3 + if params.OtelCol.Spec.ConfigVersions != nil { + configVersionsToKeep = int(*params.OtelCol.Spec.ConfigVersions) + } + + for i := range configMapList.Items { + if i > configVersionsToKeep { + ownedObjects[configMapList.Items[i].GetUID()] = &configMapList.Items[i] + } + } + return ownedObjects, nil } diff --git a/controllers/reconcile_test.go b/controllers/reconcile_test.go index 8dd272e77f..421a9fd3f1 100644 --- a/controllers/reconcile_test.go +++ b/controllers/reconcile_test.go @@ -428,7 +428,9 @@ func TestOpenTelemetryCollectorReconciler_Reconcile(t *testing.T) { result: controllerruntime.Result{}, checks: []check[v1alpha1.OpenTelemetryCollector]{ func(t *testing.T, params v1alpha1.OpenTelemetryCollector) { - exists, err := populateObjectIfExists(t, &v1.ConfigMap{}, namespacedObjectName(naming.Collector(params.Name), params.Namespace)) + configHash, _ := getConfigMapSHAFromString(params.Spec.Config) + configHash = configHash[:8] + exists, err := populateObjectIfExists(t, &v1.ConfigMap{}, namespacedObjectName(naming.ConfigMap(params.Name, configHash), params.Namespace)) assert.NoError(t, err) assert.True(t, exists) exists, err = populateObjectIfExists(t, &appsv1.StatefulSet{}, namespacedObjectName(naming.Collector(params.Name), params.Namespace)) @@ -450,7 +452,9 @@ func TestOpenTelemetryCollectorReconciler_Reconcile(t *testing.T) { result: controllerruntime.Result{}, checks: []check[v1alpha1.OpenTelemetryCollector]{ func(t *testing.T, params v1alpha1.OpenTelemetryCollector) { - exists, err := populateObjectIfExists(t, &v1.ConfigMap{}, namespacedObjectName(naming.Collector(params.Name), params.Namespace)) + configHash, _ := getConfigMapSHAFromString(params.Spec.Config) + configHash = configHash[:8] + exists, err := populateObjectIfExists(t, &v1.ConfigMap{}, namespacedObjectName(naming.ConfigMap(params.Name, configHash), params.Namespace)) assert.NoError(t, err) assert.True(t, exists) actual := v1.ConfigMap{} @@ -495,7 +499,9 @@ func TestOpenTelemetryCollectorReconciler_Reconcile(t *testing.T) { result: controllerruntime.Result{}, checks: []check[v1alpha1.OpenTelemetryCollector]{ func(t *testing.T, params v1alpha1.OpenTelemetryCollector) { - exists, err := populateObjectIfExists(t, &v1.ConfigMap{}, namespacedObjectName(naming.Collector(params.Name), params.Namespace)) + configHash, _ := getConfigMapSHAFromString(params.Spec.Config) + configHash = configHash[:8] + exists, err := populateObjectIfExists(t, &v1.ConfigMap{}, namespacedObjectName(naming.ConfigMap(params.Name, configHash), params.Namespace)) assert.NoError(t, err) assert.True(t, exists) actual := v1.ConfigMap{} diff --git a/controllers/suite_test.go b/controllers/suite_test.go index d20e7be8b2..7ce7eb0ddd 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -50,6 +50,7 @@ import ( logf "sigs.k8s.io/controller-runtime/pkg/log" metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/yaml" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/apis/v1beta1" @@ -59,6 +60,7 @@ import ( "github.com/open-telemetry/opentelemetry-operator/internal/config" "github.com/open-telemetry/opentelemetry-operator/internal/manifests" "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/testdata" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/manifestutils" "github.com/open-telemetry/opentelemetry-operator/internal/rbac" // +kubebuilder:scaffold:imports ) @@ -471,3 +473,12 @@ func populateObjectIfExists(t testing.TB, object client.Object, namespacedName t } return true, nil } + +func getConfigMapSHAFromString(configStr string) (string, error) { + var config v1beta1.Config + err := yaml.Unmarshal([]byte(configStr), &config) + if err != nil { + return "", err + } + return manifestutils.GetConfigMapSHA(config) +} diff --git a/internal/manifests/collector/configmap_test.go b/internal/manifests/collector/configmap_test.go index b850084c53..62bc989771 100644 --- a/internal/manifests/collector/configmap_test.go +++ b/internal/manifests/collector/configmap_test.go @@ -17,6 +17,7 @@ package collector import ( "testing" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/manifestutils" "github.com/stretchr/testify/assert" ) @@ -29,9 +30,6 @@ func TestDesiredConfigMap(t *testing.T) { } t.Run("should return expected collector config map", func(t *testing.T) { - expectedLables["app.kubernetes.io/component"] = "opentelemetry-collector" - expectedLables["app.kubernetes.io/name"] = "test-collector" - expectedLables["app.kubernetes.io/version"] = "0.47.0" expectedData := map[string]string{ "collector.yaml": `receivers: @@ -58,10 +56,18 @@ service: } param := deploymentParams() + hash, _ := manifestutils.GetConfigMapSHA(param.OtelCol.Spec.Config) + hash = hash[:8] + expectedName := "test-collector-" + hash + + expectedLables["app.kubernetes.io/component"] = "opentelemetry-collector" + expectedLables["app.kubernetes.io/name"] = expectedName + expectedLables["app.kubernetes.io/version"] = "0.47.0" + actual, err := ConfigMap(param) assert.NoError(t, err) - assert.Equal(t, "test-collector", actual.Name) + assert.Equal(t, expectedName, actual.Name) assert.Equal(t, expectedLables, actual.Labels) assert.Equal(t, len(expectedData), len(actual.Data)) for k, expected := range expectedData { @@ -70,10 +76,6 @@ service: }) t.Run("should return expected escaped collector config map with target_allocator config block", func(t *testing.T) { - expectedLables["app.kubernetes.io/component"] = "opentelemetry-collector" - expectedLables["app.kubernetes.io/name"] = "test-collector" - expectedLables["app.kubernetes.io/version"] = "latest" - expectedData := map[string]string{ "collector.yaml": `exporters: debug: @@ -97,11 +99,20 @@ service: param, err := newParams("test/test-img", "testdata/http_sd_config_servicemonitor_test.yaml") assert.NoError(t, err) + + hash, _ := manifestutils.GetConfigMapSHA(param.OtelCol.Spec.Config) + hash = hash[:8] + expectedName := "test-collector-" + hash + + expectedLables["app.kubernetes.io/component"] = "opentelemetry-collector" + expectedLables["app.kubernetes.io/name"] = expectedName + expectedLables["app.kubernetes.io/version"] = "latest" + param.OtelCol.Spec.TargetAllocator.Enabled = true actual, err := ConfigMap(param) assert.NoError(t, err) - assert.Equal(t, "test-collector", actual.Name) + assert.Equal(t, expectedName, actual.Name) assert.Equal(t, expectedLables, actual.Labels) assert.Equal(t, len(expectedData), len(actual.Data)) for k, expected := range expectedData {