From 993eff3c4f4f51f98579069b5855bc002ca44727 Mon Sep 17 00:00:00 2001 From: Jacob Aronoff Date: Tue, 9 Aug 2022 16:11:09 -0400 Subject: [PATCH 1/4] Update the target allocator on any manifest change --- pkg/collector/parser/receiver.go | 4 ++++ pkg/collector/reconcile/deployment.go | 6 +----- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/collector/parser/receiver.go b/pkg/collector/parser/receiver.go index 156a6928b5..ef950c89b7 100644 --- a/pkg/collector/parser/receiver.go +++ b/pkg/collector/parser/receiver.go @@ -107,6 +107,10 @@ func singlePortFromConfigEndpoint(logger logr.Logger, name string, config map[in case name == "kubeletstats": return nil + // ignore prometheus receiver as it has no listening endpoint + case name == "prometheus": + return nil + default: endpoint = getAddressFromConfig(logger, name, endpointKey, config) } diff --git a/pkg/collector/reconcile/deployment.go b/pkg/collector/reconcile/deployment.go index 99aa3a5a31..3cf567ce27 100644 --- a/pkg/collector/reconcile/deployment.go +++ b/pkg/collector/reconcile/deployment.go @@ -85,11 +85,7 @@ func expectedDeployments(ctx context.Context, params Params, expected []appsv1.D updated.Labels = map[string]string{} } - if desired.Labels["app.kubernetes.io/component"] == "opentelemetry-targetallocator" { - updated.Spec.Template.Spec.Containers[0].Image = desired.Spec.Template.Spec.Containers[0].Image - } else { - updated.Spec = desired.Spec - } + updated.Spec = desired.Spec updated.ObjectMeta.OwnerReferences = desired.ObjectMeta.OwnerReferences for k, v := range desired.ObjectMeta.Annotations { From 24358497d2e49b920edaa3f91baad6ae7083dd65 Mon Sep 17 00:00:00 2001 From: Jacob Aronoff Date: Tue, 9 Aug 2022 16:45:28 -0400 Subject: [PATCH 2/4] Change test to check for prometheusCR change validation --- pkg/collector/reconcile/deployment_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/collector/reconcile/deployment_test.go b/pkg/collector/reconcile/deployment_test.go index f1e2389e2d..c0714288e1 100644 --- a/pkg/collector/reconcile/deployment_test.go +++ b/pkg/collector/reconcile/deployment_test.go @@ -168,15 +168,14 @@ func TestExpectedDeployments(t *testing.T) { assert.Equal(t, int32(2), *actual.Spec.Replicas) }) - t.Run("should not update target allocator deployment when the container image is not updated", func(t *testing.T) { + t.Run("should update target allocator deployment when the container image is updated", func(t *testing.T) { ctx := context.Background() createObjectIfNotExists(t, "test-targetallocator", &expectedTADeploy) orgUID := expectedTADeploy.OwnerReferences[0].UID updatedParam, err := newParams("test/test-img", "") assert.NoError(t, err) - updatedDeploy := targetallocator.Deployment(updatedParam.Config, logger, param.Instance) - *updatedDeploy.Spec.Replicas = int32(3) + updatedDeploy := targetallocator.Deployment(updatedParam.Config, logger, updatedParam.Instance) err = expectedDeployments(ctx, param, []v1.Deployment{updatedDeploy}) assert.NoError(t, err) @@ -187,17 +186,18 @@ func TestExpectedDeployments(t *testing.T) { assert.NoError(t, err) assert.True(t, exists) assert.Equal(t, orgUID, actual.OwnerReferences[0].UID) - assert.Equal(t, expectedTADeploy.Spec.Template.Spec.Containers[0].Image, actual.Spec.Template.Spec.Containers[0].Image) + assert.NotEqual(t, expectedTADeploy.Spec.Template.Spec.Containers[0].Image, actual.Spec.Template.Spec.Containers[0].Image) assert.Equal(t, int32(1), *actual.Spec.Replicas) }) - t.Run("should update target allocator deployment when the container image is updated", func(t *testing.T) { + t.Run("should update target allocator deployment when the prometheusCR is updated", func(t *testing.T) { ctx := context.Background() createObjectIfNotExists(t, "test-targetallocator", &expectedTADeploy) orgUID := expectedTADeploy.OwnerReferences[0].UID - updatedParam, err := newParams("test/test-img", "") + updatedParam, err := newParams(expectedTADeploy.Spec.Template.Spec.Containers[0].Image, "") assert.NoError(t, err) + updatedParam.Instance.Spec.TargetAllocator.PrometheusCR.Enabled = true updatedDeploy := targetallocator.Deployment(updatedParam.Config, logger, updatedParam.Instance) err = expectedDeployments(ctx, param, []v1.Deployment{updatedDeploy}) From a7e52dfe27ae015a1a2c14c8ff7d99de89faf431 Mon Sep 17 00:00:00 2001 From: Jacob Aronoff Date: Tue, 9 Aug 2022 16:56:54 -0400 Subject: [PATCH 3/4] Fix test --- pkg/collector/reconcile/deployment_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/collector/reconcile/deployment_test.go b/pkg/collector/reconcile/deployment_test.go index c0714288e1..5759f196e1 100644 --- a/pkg/collector/reconcile/deployment_test.go +++ b/pkg/collector/reconcile/deployment_test.go @@ -209,7 +209,8 @@ func TestExpectedDeployments(t *testing.T) { assert.NoError(t, err) assert.True(t, exists) assert.Equal(t, orgUID, actual.OwnerReferences[0].UID) - assert.NotEqual(t, expectedTADeploy.Spec.Template.Spec.Containers[0].Image, actual.Spec.Template.Spec.Containers[0].Image) + assert.True(t, len(actual.Spec.Template.Spec.Containers[0].Args) == 1) + assert.True(t, actual.Spec.Template.Spec.Containers[0].Args[0] == "--enable-prometheus-cr-watcher") assert.Equal(t, int32(1), *actual.Spec.Replicas) }) From cd96bd3e934bf302af8a80a5c745ed88f1672905 Mon Sep 17 00:00:00 2001 From: Jacob Aronoff Date: Wed, 10 Aug 2022 10:46:58 -0400 Subject: [PATCH 4/4] Updated test to be more flexible for the future --- pkg/collector/reconcile/deployment_test.go | 47 +++++++++++----------- 1 file changed, 23 insertions(+), 24 deletions(-) diff --git a/pkg/collector/reconcile/deployment_test.go b/pkg/collector/reconcile/deployment_test.go index 5759f196e1..3867900754 100644 --- a/pkg/collector/reconcile/deployment_test.go +++ b/pkg/collector/reconcile/deployment_test.go @@ -102,6 +102,29 @@ func TestExpectedDeployments(t *testing.T) { assert.Len(t, expected, 0) }) + t.Run("should update target allocator deployment when the prometheusCR is updated", func(t *testing.T) { + ctx := context.Background() + createObjectIfNotExists(t, "test-targetallocator", &expectedTADeploy) + orgUID := expectedTADeploy.OwnerReferences[0].UID + + updatedParam, err := newParams(expectedTADeploy.Spec.Template.Spec.Containers[0].Image, "") + assert.NoError(t, err) + updatedParam.Instance.Spec.TargetAllocator.PrometheusCR.Enabled = true + updatedDeploy := targetallocator.Deployment(updatedParam.Config, logger, updatedParam.Instance) + + err = expectedDeployments(ctx, param, []v1.Deployment{updatedDeploy}) + assert.NoError(t, err) + + actual := v1.Deployment{} + exists, err := populateObjectIfExists(t, &actual, types.NamespacedName{Namespace: "default", Name: "test-targetallocator"}) + + assert.NoError(t, err) + assert.True(t, exists) + assert.Equal(t, orgUID, actual.OwnerReferences[0].UID) + assert.ElementsMatch(t, actual.Spec.Template.Spec.Containers[0].Args, []string{"--enable-prometheus-cr-watcher"}) + assert.Equal(t, int32(1), *actual.Spec.Replicas) + }) + t.Run("should not update target allocator deployment replicas when collector max replicas is set", func(t *testing.T) { replicas, maxReplicas := int32(2), int32(10) param := Params{ @@ -190,30 +213,6 @@ func TestExpectedDeployments(t *testing.T) { assert.Equal(t, int32(1), *actual.Spec.Replicas) }) - t.Run("should update target allocator deployment when the prometheusCR is updated", func(t *testing.T) { - ctx := context.Background() - createObjectIfNotExists(t, "test-targetallocator", &expectedTADeploy) - orgUID := expectedTADeploy.OwnerReferences[0].UID - - updatedParam, err := newParams(expectedTADeploy.Spec.Template.Spec.Containers[0].Image, "") - assert.NoError(t, err) - updatedParam.Instance.Spec.TargetAllocator.PrometheusCR.Enabled = true - updatedDeploy := targetallocator.Deployment(updatedParam.Config, logger, updatedParam.Instance) - - err = expectedDeployments(ctx, param, []v1.Deployment{updatedDeploy}) - assert.NoError(t, err) - - actual := v1.Deployment{} - exists, err := populateObjectIfExists(t, &actual, types.NamespacedName{Namespace: "default", Name: "test-targetallocator"}) - - assert.NoError(t, err) - assert.True(t, exists) - assert.Equal(t, orgUID, actual.OwnerReferences[0].UID) - assert.True(t, len(actual.Spec.Template.Spec.Containers[0].Args) == 1) - assert.True(t, actual.Spec.Template.Spec.Containers[0].Args[0] == "--enable-prometheus-cr-watcher") - assert.Equal(t, int32(1), *actual.Spec.Replicas) - }) - t.Run("should delete deployment", func(t *testing.T) { labels := map[string]string{ "app.kubernetes.io/instance": "default.test",