fix(k8s): don't temporarily remove all AvailableServices on ZoneIngre…

…ss Pod reconciliations (backport of #8301) (#8305)

fix(k8s): don't temporarily remove all AvailableServices on ZoneIngress Pod reconciliations (#8301)

* test(k8s): add existing ZoneIngress test



* fix(k8s): don't temporarily remove all AvailableServices on ZoneIngress Pod reconciliations



* test: add ResourceConverter



* test: update with fixed golden file



---------

Signed-off-by: Mike Beaumont <mjboamail@gmail.com>
Co-authored-by: Krzysztof Słonka <slonka@users.noreply.github.com>
Co-authored-by: Mike Beaumont <mjboamail@gmail.com>

pkg/plugins/runtime/k8s/controllers/pod_controller_test.go

@@ -278,10 +278,13 @@ var _ = Describe("PodReconciler", func() {
 			}).Build()
 
 		reconciler = &PodReconciler{
-			Client:            kubeClient,
-			EventRecorder:     kube_record.NewFakeRecorder(10),
-			Scheme:            k8sClientScheme,
-			Log:               core.Log.WithName("test"),
+			Client:        kubeClient,
+			EventRecorder: kube_record.NewFakeRecorder(10),
+			Scheme:        k8sClientScheme,
+			Log:           core.Log.WithName("test"),
+			PodConverter: PodConverter{
+				ResourceConverter: k8s.NewSimpleConverter(),
+			},
 			SystemNamespace:   "kuma-system",
 			Persistence:       vips.NewPersistence(core_manager.NewResourceManager(memory.NewStore()), manager.NewConfigManager(memory.NewStore())),
 			ResourceConverter: k8s.NewSimpleConverter(),

pkg/plugins/runtime/k8s/controllers/pod_converter.go

@@ -11,6 +11,7 @@ import (
 
 	mesh_proto "github.com/kumahq/kuma/api/mesh/v1alpha1"
 	"github.com/kumahq/kuma/pkg/core"
+	core_mesh "github.com/kumahq/kuma/pkg/core/resources/apis/mesh"
 	k8s_common "github.com/kumahq/kuma/pkg/plugins/common/k8s"
 	mesh_k8s "github.com/kumahq/kuma/pkg/plugins/resources/k8s/native/api/v1alpha1"
 	"github.com/kumahq/kuma/pkg/plugins/runtime/k8s/metadata"
@@ -50,24 +51,36 @@ func (p *PodConverter) PodToDataplane(
 }
 
 func (p *PodConverter) PodToIngress(ctx context.Context, zoneIngress *mesh_k8s.ZoneIngress, pod *kube_core.Pod, services []*kube_core.Service) error {
-	zoneIngressProto := &mesh_proto.ZoneIngress{}
-	// Pass the current dataplane so we won't override available services in Ingress section
-	if err := p.IngressFor(ctx, zoneIngressProto, pod, services); err != nil {
+	logger := converterLog.WithValues("ZoneIngress.name", zoneIngress.Name, "Pod.name", pod.Name)
+	// Start with the existing ZoneIngress spec so we won't override available services in Ingress section
+	zoneIngressRes := core_mesh.NewZoneIngressResource()
+	if err := p.ResourceConverter.ToCoreResource(zoneIngress, zoneIngressRes); err != nil {
+		logger.Error(err, "unable to convert ZoneIngress k8s object into core resource")
 		return err
 	}
-	zoneIngress.SetSpec(zoneIngressProto)
+
+	if err := p.IngressFor(ctx, zoneIngressRes.Spec, pod, services); err != nil {
+		return err
+	}
+
+	zoneIngress.SetSpec(zoneIngressRes.Spec)
 	return nil
 }
 
 func (p *PodConverter) PodToEgress(ctx context.Context, zoneEgress *mesh_k8s.ZoneEgress, pod *kube_core.Pod, services []*kube_core.Service) error {
-	zoneEgressProto := &mesh_proto.ZoneEgress{}
-	// Pass the current dataplane, so we won't override available services in Egress section
-	if err := p.EgressFor(ctx, zoneEgressProto, pod, services); err != nil {
+	logger := converterLog.WithValues("ZoneEgress.name", zoneEgress.Name, "Pod.name", pod.Name)
+	// Start with the existing ZoneEgress spec
+	zoneEgressRes := core_mesh.NewZoneEgressResource()
+	if err := p.ResourceConverter.ToCoreResource(zoneEgress, zoneEgressRes); err != nil {
+		logger.Error(err, "unable to convert ZoneEgress k8s object into core resource")
 		return err
 	}
 
-	zoneEgress.SetSpec(zoneEgressProto)
+	if err := p.EgressFor(ctx, zoneEgressRes.Spec, pod, services); err != nil {
+		return err
+	}
 
+	zoneEgress.SetSpec(zoneEgressRes.Spec)
 	return nil
 }
 

pkg/plugins/runtime/k8s/controllers/pod_converter_test.go

@@ -64,14 +64,15 @@ var _ = Describe("PodToDataplane(..)", func() {
 `
 
 	type testCase struct {
-		pod              string
-		servicesForPod   string
-		otherDataplanes  string
-		otherServices    string
-		otherReplicaSets string
-		otherJobs        string
-		node             string
-		dataplane        string
+		pod               string
+		servicesForPod    string
+		otherDataplanes   string
+		otherServices     string
+		otherReplicaSets  string
+		otherJobs         string
+		node              string
+		dataplane         string
+		existingDataplane string
 	}
 	DescribeTable("should convert Pod into a Dataplane YAML version",
 		func(given testCase) {
@@ -304,16 +305,23 @@ var _ = Describe("PodToDataplane(..)", func() {
 			}
 
 			converter := PodConverter{
-				ServiceGetter: nil,
-				NodeGetter:    nodeGetter,
-				Zone:          "zone-1",
+				ServiceGetter:     nil,
+				NodeGetter:        nodeGetter,
+				ResourceConverter: k8s.NewSimpleConverter(),
+				Zone:              "zone-1",
 			}
 
 			// when
 			ingress := &mesh_k8s.ZoneIngress{}
-			err = converter.PodToIngress(context.Background(), ingress, pod, services)
+			if given.existingDataplane != "" {
+				bytes, err = os.ReadFile(filepath.Join("testdata", "ingress", given.existingDataplane))
+				Expect(err).ToNot(HaveOccurred())
+				err = yaml.Unmarshal(bytes, ingress)
+				Expect(err).ToNot(HaveOccurred())
+			}
 
 			// then
+			err = converter.PodToIngress(context.Background(), ingress, pod, services)
 			Expect(err).ToNot(HaveOccurred())
 
 			actual, err := yaml.Marshal(ingress)
@@ -352,6 +360,12 @@ var _ = Describe("PodToDataplane(..)", func() {
 			servicesForPod: "06.services-for-pod.yaml",
 			dataplane:      "06.dataplane.yaml",
 		}),
+		Entry("Existing ZoneIngress with load balancer and ip", testCase{
+			pod:               "ingress-exists.pod.yaml",
+			servicesForPod:    "ingress-exists.services-for-pod.yaml",
+			existingDataplane: "ingress-exists.existing-dataplane.yaml",
+			dataplane:         "ingress-exists.golden.yaml",
+		}),
 	)
 
 	DescribeTable("should convert Egress Pod into an Egress Dataplane YAML version",
@@ -381,9 +395,10 @@ var _ = Describe("PodToDataplane(..)", func() {
 			}
 
 			converter := PodConverter{
-				ServiceGetter: nil,
-				NodeGetter:    nodeGetter,
-				Zone:          "zone-1",
+				ServiceGetter:     nil,
+				NodeGetter:        nodeGetter,
+				ResourceConverter: k8s.NewSimpleConverter(),
+				Zone:              "zone-1",
 			}
 
 			// when
@@ -436,7 +451,9 @@ var _ = Describe("PodToDataplane(..)", func() {
 		DescribeTable("should return a descriptive error",
 			func(given testCase) {
 				// given
-				converter := PodConverter{}
+				converter := PodConverter{
+					ResourceConverter: k8s.NewSimpleConverter(),
+				}
 
 				pod := &kube_core.Pod{}
 				err := yaml.Unmarshal([]byte(given.pod), pod)

pkg/plugins/runtime/k8s/controllers/testdata/ingress/ingress-exists.existing-dataplane.yaml

@@ -0,0 +1,14 @@
+metadata:
+  creationTimestamp: null
+spec:
+  networking:
+    address: 192.168.0.1
+    advertisedAddress: 192.168.100.1
+    advertisedPort: 10001
+    port: 10001
+  availableServices:
+  - tags:
+      kuma.io/service: service-1-zone-2
+      kuma.io/protocol: http
+    instances: 3
+    mesh: mesh-1

pkg/plugins/runtime/k8s/controllers/testdata/ingress/ingress-exists.golden.yaml

@@ -0,0 +1,14 @@
+metadata:
+  creationTimestamp: null
+spec:
+  availableServices:
+  - instances: 3
+    mesh: mesh-1
+    tags:
+      kuma.io/protocol: http
+      kuma.io/service: service-1-zone-2
+  networking:
+    address: 192.168.0.1
+    advertisedAddress: 192.168.100.1
+    advertisedPort: 10001
+    port: 10001

pkg/plugins/runtime/k8s/controllers/testdata/ingress/ingress-exists.pod.yaml

@@ -0,0 +1,13 @@
+metadata:
+  namespace: kuma-system
+  name: kuma-ingress
+  labels:
+    app: kuma-ingress
+  annotations:
+    kuma.io/ingress: enabled
+spec:
+  containers:
+    - ports:
+        - containerPort: 10001
+status:
+  podIP: 192.168.0.1

pkg/plugins/runtime/k8s/controllers/testdata/ingress/ingress-exists.services-for-pod.yaml

@@ -0,0 +1,15 @@
+---
+metadata:
+  namespace: kuma-system
+  name: kuma-ingress
+spec:
+  type: LoadBalancer
+  ports:
+    - port: 10001
+      targetPort: 10001
+  selector:
+    app: kuma-ingress
+status:
+  loadBalancer:
+    ingress:
+      - ip: 192.168.100.1