Improved updating container ports based on their names

Dynatrace · Nov 23, 2021 · 921fa1d · 921fa1d
1 parent d2edaad
commit 921fa1d
Show file tree

Hide file tree

Showing 6 changed files with 114 additions and 19 deletions.
diff --git a/controllers/activegate/internal/consts/consts.go b/controllers/activegate/internal/consts/consts.go
@@ -1,10 +1,14 @@
 package consts
 
 const (
-	ServicePortName   = "https"
-	ServicePort       = 443
-	ServiceTargetPort = "ag-https"
+	ActiveGateContainerName = "dynatrace-operator" // TODO Does it make sense as the container name? Do we need a link to dynatracev1beta1.OperatorName?
+	ServicePortName         = "https"
+	ServicePort             = 443
+	ServiceTargetPort       = "ag-https"
 
+	EecContainerName = ActiveGateContainerName + "-eec"
+
+	StatsDContainerName    = ActiveGateContainerName + "-statsd"
 	StatsDIngestPortName   = "statsd"
 	StatsDIngestPort       = 18125
 	StatsDIngestTargetPort = "statsd-port"

diff --git a/controllers/activegate/reconciler/capability/reconciler.go b/controllers/activegate/reconciler/capability/reconciler.go
@@ -61,22 +61,44 @@ func setReadinessProbePort() events.StatefulSetEvent {
 	}
 }
 
+func getContainerByName(containers []corev1.Container, containerName string) (*corev1.Container, error) {
+	for i, _ := range containers {
+		if containers[i].Name == containerName {
+			return &containers[i], nil
+		}
+	}
+	return nil, errors.Errorf(`Cannot find container "%s" in the provided slice (len %d)`,
+		containerName, len(containers),
+	)
+}
+
 func setCommunicationsPort(dk *dynatracev1beta1.DynaKube) events.StatefulSetEvent {
 	return func(sts *appsv1.StatefulSet) {
-		sts.Spec.Template.Spec.Containers[0].Ports = []corev1.ContainerPort{
-			{
-				Name:          consts.ServiceTargetPort,
-				ContainerPort: containerPort,
-			},
+		{
+			activeGateContainer, err := getContainerByName(sts.Spec.Template.Spec.Containers, consts.ActiveGateContainerName)
+			if err == nil {
+				activeGateContainer.Ports = []corev1.ContainerPort{
+					{
+						Name:          consts.ServiceTargetPort,
+						ContainerPort: containerPort,
+					},
+				}
+			} else {
+				// TODO How to report an error?
+			}
 		}
 		if dk.FeatureEnableStatsDIngest() {
-			// TODO Refactor (access containers by name instead of index)
-			sts.Spec.Template.Spec.Containers[2].Ports = []corev1.ContainerPort{
-				{
-					Name:          consts.StatsDIngestTargetPort,
-					ContainerPort: consts.StatsDIngestPort,
-					Protocol:      corev1.ProtocolUDP,
-				},
+			statsdContainer, err := getContainerByName(sts.Spec.Template.Spec.Containers, consts.StatsDContainerName)
+			if err == nil {
+				statsdContainer.Ports = []corev1.ContainerPort{
+					{
+						Name:          consts.StatsDIngestTargetPort,
+						ContainerPort: consts.StatsDIngestPort,
+						Protocol:      corev1.ProtocolUDP,
+					},
+				}
+			} else {
+				// TODO How to report error?
 			}
 		}
 	}

diff --git a/controllers/activegate/reconciler/capability/reconciler_test.go b/controllers/activegate/reconciler/capability/reconciler_test.go
@@ -308,3 +308,71 @@ func TestReconciler_calculateStatefulSetName(t *testing.T) {
 		})
 	}
 }
+
+func TestGetContainerByName(t *testing.T) {
+	type testData struct {
+		containers          []corev1.Container
+		lookingForContainer string
+		errorMessage        string
+	}
+
+	verifyAll := func(t *testing.T, testCases []testData) {
+		for _, testCase := range testCases {
+			container, err := getContainerByName(testCase.containers, testCase.lookingForContainer)
+			if testCase.errorMessage == "" {
+				assert.NoError(t, err)
+				assert.NotNil(t, container)
+				assert.Equal(t, testCase.lookingForContainer, container.Name)
+			} else {
+				assert.Error(t, err)
+				assert.Contains(t, err.Error(), testCase.errorMessage)
+				assert.Nil(t, container)
+			}
+		}
+	}
+
+	t.Run("empty slice test cases", func(t *testing.T) {
+		verifyAll(t, []testData{
+			{
+				containers:          nil,
+				lookingForContainer: "",
+				errorMessage:        `Cannot find container "" in the provided slice (len 0)`,
+			},
+			{
+				containers:          []corev1.Container{},
+				lookingForContainer: "",
+				errorMessage:        `Cannot find container "" in the provided slice (len 0)`,
+			},
+			{
+				containers:          []corev1.Container{},
+				lookingForContainer: "something",
+				errorMessage:        `Cannot find container "something" in the provided slice (len 0)`,
+			},
+		})
+	})
+
+	t.Run("non-empty collection but cannot match name", func(t *testing.T) {
+		verifyAll(t, []testData{
+			{
+				containers: []corev1.Container{
+					{Name: consts.ActiveGateContainerName},
+					{Name: consts.StatsDContainerName},
+				},
+				lookingForContainer: consts.EecContainerName,
+				errorMessage:        fmt.Sprintf(`Cannot find container "%s" in the provided slice (len 2)`, consts.EecContainerName),
+			},
+		})
+	})
+
+	t.Run("happy path", func(t *testing.T) {
+		verifyAll(t, []testData{
+			{
+				containers: []corev1.Container{
+					{Name: consts.StatsDContainerName},
+				},
+				lookingForContainer: consts.StatsDContainerName,
+				errorMessage:        "",
+			},
+		})
+	})
+}
diff --git a/controllers/activegate/reconciler/statefulset/container_eec.go b/controllers/activegate/reconciler/statefulset/container_eec.go
@@ -4,6 +4,7 @@ import (
 	"fmt"
 
 	dynatracev1beta1 "github.com/Dynatrace/dynatrace-operator/api/v1beta1"
+	"github.com/Dynatrace/dynatrace-operator/controllers/activegate/internal/consts"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
 )
@@ -27,7 +28,7 @@ func NewExtensionController(stsProperties *statefulSetProperties) *ExtensionCont
 
 func (eec *ExtensionController) BuildContainer() corev1.Container {
 	return corev1.Container{
-		Name:            fmt.Sprintf("%s-eec", dynatracev1beta1.OperatorName),
+		Name:            consts.EecContainerName,
 		Image:           eec.StsProperties.DynaKube.ActiveGateImage(),
 		ImagePullPolicy: corev1.PullAlways,
 		Env:             eec.buildEnvs(),

diff --git a/controllers/activegate/reconciler/statefulset/container_statsd.go b/controllers/activegate/reconciler/statefulset/container_statsd.go
@@ -3,7 +3,6 @@ package statefulset
 import (
 	"fmt"
 
-	dynatracev1beta1 "github.com/Dynatrace/dynatrace-operator/api/v1beta1"
 	"github.com/Dynatrace/dynatrace-operator/controllers/activegate/internal/consts"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
@@ -26,7 +25,7 @@ func NewStatsD(stsProperties *statefulSetProperties) *StatsD {
 
 func (statsd *StatsD) BuildContainer() corev1.Container {
 	return corev1.Container{
-		Name:            fmt.Sprintf("%s-statsd", dynatracev1beta1.OperatorName),
+		Name:            consts.StatsDContainerName,
 		Image:           statsd.StsProperties.DynaKube.ActiveGateImage(),
 		ImagePullPolicy: corev1.PullAlways,
 		Env:             statsd.buildEnvs(),

diff --git a/controllers/activegate/reconciler/statefulset/statefulset.go b/controllers/activegate/reconciler/statefulset/statefulset.go
@@ -4,6 +4,7 @@ import (
 	"fmt"
 
 	dynatracev1beta1 "github.com/Dynatrace/dynatrace-operator/api/v1beta1"
+	"github.com/Dynatrace/dynatrace-operator/controllers/activegate/internal/consts"
 	"github.com/Dynatrace/dynatrace-operator/controllers/activegate/internal/events"
 	"github.com/Dynatrace/dynatrace-operator/controllers/customproperties"
 	"github.com/Dynatrace/dynatrace-operator/controllers/kubeobjects"
@@ -161,7 +162,7 @@ func buildContainers(stsProperties *statefulSetProperties, extraContainerBuilder
 
 func buildActiveGateContainer(stsProperties *statefulSetProperties) corev1.Container {
 	return corev1.Container{
-		Name:            dynatracev1beta1.OperatorName,
+		Name:            consts.ActiveGateContainerName,
 		Image:           stsProperties.DynaKube.ActiveGateImage(),
 		Resources:       stsProperties.CapabilityProperties.Resources,
 		ImagePullPolicy: corev1.PullAlways,