Always add ActiveGate ports no matter the capability (#4069)

Co-authored-by: Marcell Sevcsik <31651557+0sewa0@users.noreply.github.com>

pkg/api/v1beta3/dynakube/activegate/props.go

@@ -27,10 +27,6 @@ func (ag *Spec) SetExtensionsDependency(isEnabled bool) {
 	ag.enabledDependencies.extensions = isEnabled
 }
 
-func (ag *Spec) SetKSPMDependency(isEnabled bool) {
-	ag.enabledDependencies.kspm = isEnabled
-}
-
 func (ag *Spec) apiUrlHost() string {
 	parsedUrl, err := url.Parse(ag.apiUrl)
 	if err != nil {
@@ -92,14 +88,6 @@ func (ag *Spec) IsMetricsIngestEnabled() bool {
 	return ag.IsMode(MetricsIngestCapability.DisplayName)
 }
 
-func (ag *Spec) NeedsService() bool {
-	return ag.IsRoutingEnabled() ||
-		ag.IsApiEnabled() ||
-		ag.IsMetricsIngestEnabled() ||
-		ag.enabledDependencies.extensions ||
-		ag.enabledDependencies.kspm
-}
-
 func (ag *Spec) HasCaCert() bool {
 	return ag.IsEnabled() && ag.TlsSecretName != ""
 }

pkg/api/v1beta3/dynakube/activegate/spec.go

@@ -60,11 +60,10 @@ type ActiveGate struct {
 // dependencies is a collection of possible other feature/components that need an ActiveGate, but is not directly configured in the ActiveGate section.
 type dependencies struct {
 	extensions bool
-	kspm       bool
 }
 
 func (d dependencies) Any() bool {
-	return d.extensions
+	return d.extensions // kspm is a dependency too, but blocked by validation webhook to not run standalone
 }
 
 // +kubebuilder:object:generate=true

pkg/api/v1beta3/dynakube/activegate_props.go

@@ -8,7 +8,6 @@ func (dk *DynaKube) ActiveGate() *activegate.ActiveGate {
 	dk.Spec.ActiveGate.SetApiUrl(dk.ApiUrl())
 	dk.Spec.ActiveGate.SetName(dk.Name)
 	dk.Spec.ActiveGate.SetExtensionsDependency(dk.IsExtensionsEnabled())
-	dk.Spec.ActiveGate.SetKSPMDependency(dk.KSPM().IsEnabled())
 
 	return &activegate.ActiveGate{
 		Spec:   &dk.Spec.ActiveGate,

pkg/controllers/dynakube/activegate/internal/capability/reconciler.go

@@ -42,18 +42,14 @@ func (r *Reconciler) Reconcile(ctx context.Context) error {
 		return errors.WithStack(err)
 	}
 
-	if r.dk.ActiveGate().NeedsService() {
-		err = r.createOrUpdateService(ctx)
-		if err != nil {
-			return err
-		}
+	err = r.createOrUpdateService(ctx)
+	if err != nil {
+		return err
+	}
 
-		err = r.setAGServiceIPs(ctx)
-		if err != nil {
-			return err
-		}
-	} else {
-		r.dk.Status.ActiveGate.ServiceIPs = []string{}
+	err = r.setAGServiceIPs(ctx)
+	if err != nil {
+		return err
 	}
 
 	err = r.statefulsetReconciler.Reconcile(ctx)

pkg/controllers/dynakube/activegate/internal/capability/reconciler_test.go

@@ -161,7 +161,7 @@ func TestReconcile(t *testing.T) {
 		assert.NotNil(t, service)
 		require.NoError(t, err)
 	})
-	t.Run(`service does not get created when missing capabilities`, func(t *testing.T) {
+	t.Run(`service is created even though capability does not need it`, func(t *testing.T) {
 		clt := createClient()
 		dk := buildDynakube(capabilitiesWithoutService)
 		mockStatefulSetReconciler := getMockReconciler(t, nil)
@@ -179,8 +179,10 @@ func TestReconcile(t *testing.T) {
 		service := corev1.Service{}
 		err = r.client.Get(context.Background(), client.ObjectKey{Name: r.dk.Name + "-" + r.capability.ShortName(), Namespace: r.dk.Namespace}, &service)
 
-		assert.Empty(t, service)
-		require.Error(t, err)
+		require.NoError(t, err)
+
+		assert.NotEmpty(t, service)
+		assert.Len(t, service.Spec.Ports, 2)
 	})
 }
 

pkg/controllers/dynakube/activegate/internal/capability/service.go

@@ -13,24 +13,18 @@ import (
 func CreateService(dk *dynakube.DynaKube, feature string) *corev1.Service {
 	var ports []corev1.ServicePort
 
-	if dk.ActiveGate().NeedsService() {
-		ports = append(ports,
-			corev1.ServicePort{
-				Name:       consts.HttpsServicePortName,
-				Protocol:   corev1.ProtocolTCP,
-				Port:       consts.HttpsServicePort,
-				TargetPort: intstr.FromString(consts.HttpsServicePortName),
-			},
-		)
-		if dk.ActiveGate().IsMetricsIngestEnabled() {
-			ports = append(ports, corev1.ServicePort{
-				Name:       consts.HttpServicePortName,
-				Protocol:   corev1.ProtocolTCP,
-				Port:       consts.HttpServicePort,
-				TargetPort: intstr.FromString(consts.HttpServicePortName),
-			})
-		}
-	}
+	ports = append(ports,
+		corev1.ServicePort{
+			Name:       consts.HttpsServicePortName,
+			Protocol:   corev1.ProtocolTCP,
+			Port:       consts.HttpsServicePort,
+			TargetPort: intstr.FromString(consts.HttpsServicePortName),
+		}, corev1.ServicePort{
+			Name:       consts.HttpServicePortName,
+			Protocol:   corev1.ProtocolTCP,
+			Port:       consts.HttpServicePort,
+			TargetPort: intstr.FromString(consts.HttpServicePortName),
+		})
 
 	coreLabels := labels.NewCoreLabels(dk.Name, labels.ActiveGateComponentLabel)
 

pkg/controllers/dynakube/activegate/internal/capability/service_test.go

@@ -81,7 +81,7 @@ func TestCreateService(t *testing.T) {
 		ports := service.Spec.Ports
 
 		assert.Contains(t, ports, agHttpsPort)
-		assert.NotContains(t, ports, agHttpPort)
+		assert.Contains(t, ports, agHttpPort)
 	})
 	t.Run("check AG service if metrics-ingest enabled", func(t *testing.T) {
 		dk := createTestDynaKube()

pkg/controllers/dynakube/activegate/internal/statefulset/builder/modifiers/config_test.go

@@ -78,7 +78,6 @@ func enableAllModifiers(dk *dynakube.DynaKube, capability capability.Capability)
 	setCustomPropertyUsage(capability, true)
 	setProxyUsage(dk, true)
 	setKubernetesMonitoringUsage(dk, true)
-	setServicePortUsage(dk, true)
 }
 
 func TestNoConflict(t *testing.T) {

pkg/controllers/dynakube/activegate/internal/statefulset/builder/modifiers/serviceport.go

@@ -30,7 +30,7 @@ type ServicePortModifier struct {
 }
 
 func (mod ServicePortModifier) Enabled() bool {
-	return mod.dk.ActiveGate().NeedsService()
+	return true
 }
 
 func (mod ServicePortModifier) Modify(sts *appsv1.StatefulSet) error {
@@ -48,12 +48,10 @@ func (mod ServicePortModifier) getPorts() []corev1.ContainerPort {
 			Name:          consts.HttpsServicePortName,
 			ContainerPort: consts.HttpsContainerPort,
 		},
-	}
-	if mod.dk.ActiveGate().IsMetricsIngestEnabled() {
-		ports = append(ports, corev1.ContainerPort{
+		{
 			Name:          consts.HttpServicePortName,
 			ContainerPort: consts.HttpContainerPort,
-		})
+		},
 	}
 
 	return ports

pkg/controllers/dynakube/activegate/internal/statefulset/builder/modifiers/serviceport_test.go

@@ -3,47 +3,16 @@ package modifiers
 import (
 	"testing"
 
-	"github.com/Dynatrace/dynatrace-operator/pkg/api/v1beta3/dynakube"
-	"github.com/Dynatrace/dynatrace-operator/pkg/api/v1beta3/dynakube/activegate"
 	"github.com/Dynatrace/dynatrace-operator/pkg/controllers/dynakube/activegate/capability"
 	"github.com/Dynatrace/dynatrace-operator/pkg/controllers/dynakube/activegate/consts"
 	"github.com/Dynatrace/dynatrace-operator/pkg/util/prioritymap"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
-func setServicePortUsage(dk *dynakube.DynaKube, isUsed bool) {
-	if isUsed {
-		dk.Spec.ActiveGate.Capabilities = append(dk.Spec.ActiveGate.Capabilities, activegate.MetricsIngestCapability.DisplayName)
-	}
-}
-
-func TestServicePortEnabled(t *testing.T) {
-	t.Run("true", func(t *testing.T) {
-		dk := getBaseDynakube()
-		setServicePortUsage(&dk, true)
-		multiCapability := capability.NewMultiCapability(&dk)
-
-		mod := NewServicePortModifier(dk, multiCapability, prioritymap.New())
-
-		assert.True(t, mod.Enabled())
-	})
-
-	t.Run("false", func(t *testing.T) {
-		dk := getBaseDynakube()
-		setServicePortUsage(&dk, false)
-		multiCapability := capability.NewMultiCapability(&dk)
-
-		mod := NewServicePortModifier(dk, multiCapability, prioritymap.New())
-
-		assert.False(t, mod.Enabled())
-	})
-}
-
 func TestServicePortModify(t *testing.T) {
 	t.Run("successfully modified", func(t *testing.T) {
 		dk := getBaseDynakube()
-		setServicePortUsage(&dk, true)
 		multiCapability := capability.NewMultiCapability(&dk)
 		mod := NewServicePortModifier(dk, multiCapability, prioritymap.New())
 		builder := createBuilderForTesting()

pkg/controllers/dynakube/activegate/reconciler.go

@@ -199,10 +199,6 @@ func (r *Reconciler) deleteCapability(ctx context.Context, agCapability capabili
 }
 
 func (r *Reconciler) deleteService(ctx context.Context, agCapability capability.Capability) error {
-	if r.dk.ActiveGate().NeedsService() {
-		return nil
-	}
-
 	svc := corev1.Service{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      capability.BuildServiceName(r.dk.Name, agCapability.ShortName()),

pkg/controllers/dynakube/activegate/reconciler_test.go

@@ -435,19 +435,24 @@ func TestServiceCreation(t *testing.T) {
 		},
 	}
 
-	t.Run("service exposes correct ports for single capabilities", func(t *testing.T) {
+	t.Run("service exposes all ports for every capabilities", func(t *testing.T) {
 		expectedCapabilityPorts := map[activegate.CapabilityDisplayName][]string{
 			activegate.RoutingCapability.DisplayName: {
+				consts.HttpServicePortName,
 				consts.HttpsServicePortName,
 			},
 			activegate.MetricsIngestCapability.DisplayName: {
-				consts.HttpsServicePortName,
 				consts.HttpServicePortName,
+				consts.HttpsServicePortName,
 			},
 			activegate.DynatraceApiCapability.DisplayName: {
+				consts.HttpServicePortName,
+				consts.HttpsServicePortName,
+			},
+			activegate.KubeMonCapability.DisplayName: {
+				consts.HttpServicePortName,
 				consts.HttpsServicePortName,
 			},
-			activegate.KubeMonCapability.DisplayName: {},
 		}
 
 		for capName, expectedPorts := range expectedCapabilityPorts {
@@ -477,28 +482,6 @@ func TestServiceCreation(t *testing.T) {
 			assertContainsAllPorts(t, expectedPorts, activegateService.Spec.Ports)
 		}
 	})
-
-	t.Run("service exposes correct ports for multiple capabilities", func(t *testing.T) {
-		fakeClient := fake.NewClient(testKubeSystemNamespace)
-
-		reconciler := NewReconciler(fakeClient, fakeClient, dk, dynatraceClient, nil, nil).(*Reconciler)
-		reconciler.connectionReconciler = createGenericReconcilerMock(t)
-		reconciler.versionReconciler = createVersionReconcilerMock(t)
-		reconciler.pullSecretReconciler = createGenericReconcilerMock(t)
-
-		dk.Spec.ActiveGate.Capabilities = []activegate.CapabilityDisplayName{
-			activegate.RoutingCapability.DisplayName,
-		}
-		expectedPorts := []string{
-			consts.HttpsServicePortName,
-		}
-
-		err := reconciler.Reconcile(context.Background())
-		require.NoError(t, err)
-
-		activegateService := getTestActiveGateService(t, fakeClient)
-		assertContainsAllPorts(t, expectedPorts, activegateService.Spec.Ports)
-	})
 }
 
 func assertContainsAllPorts(t *testing.T, expectedPorts []string, servicePorts []corev1.ServicePort) {