From 39a79aece7669301c072819293ae052cfcab5868 Mon Sep 17 00:00:00 2001
From: Michal Jura <mjura@suse.com>
Date: Mon, 10 Jun 2024 15:28:32 +0200
Subject: [PATCH] Fix logging types updates

Update getLoggingTypesUpdate function when there are no changes
for logging types. Adding also test scenario to avoid regressions
in the future.

Issue: https://github.com/rancher/eks-operator/issues/546
---
 pkg/eks/update.go      |  8 ++++++--
 pkg/eks/update_test.go | 25 +++++++++++++++++++++----
 2 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/pkg/eks/update.go b/pkg/eks/update.go
index 99fbead4..5bf77e0b 100644
--- a/pkg/eks/update.go
+++ b/pkg/eks/update.go
@@ -191,12 +191,16 @@ func getLoggingTypesUpdate(loggingTypes []string, upstreamLoggingTypes []string)
 
 	if len(loggingTypes) > 0 {
 		loggingTypesToDisable := getLoggingTypesToDisable(loggingTypes, upstreamLoggingTypes)
-		loggingUpdate.ClusterLogging = append(loggingUpdate.ClusterLogging, loggingTypesToDisable)
+		if loggingTypesToDisable.Enabled != nil {
+			loggingUpdate.ClusterLogging = append(loggingUpdate.ClusterLogging, loggingTypesToDisable)
+		}
 	}
 
 	if len(upstreamLoggingTypes) > 0 {
 		loggingTypesToEnable := getLoggingTypesToEnable(loggingTypes, upstreamLoggingTypes)
-		loggingUpdate.ClusterLogging = append(loggingUpdate.ClusterLogging, loggingTypesToEnable)
+		if loggingTypesToEnable.Enabled != nil {
+			loggingUpdate.ClusterLogging = append(loggingUpdate.ClusterLogging, loggingTypesToEnable)
+		}
 	}
 
 	if len(loggingUpdate.ClusterLogging) > 0 {
diff --git a/pkg/eks/update_test.go b/pkg/eks/update_test.go
index 8b13d071..d3cbc7b6 100644
--- a/pkg/eks/update_test.go
+++ b/pkg/eks/update_test.go
@@ -200,11 +200,11 @@ var _ = Describe("UpdateLoggingTypes", func() {
 			EKSService: eksServiceMock,
 			Config: &eksv1.EKSClusterConfig{
 				Spec: eksv1.EKSClusterConfigSpec{
-					LoggingTypes: []string{"test1", "test2", "test3-enabled"},
+					LoggingTypes: []string{"audit", "authenticator", "controllerManager"},
 				},
 			},
 			UpstreamClusterSpec: &eksv1.EKSClusterConfigSpec{
-				LoggingTypes: []string{"test1", "test2", "disabled"},
+				LoggingTypes: []string{"audit", "authenticator", "scheduler"},
 			},
 		}
 	})
@@ -221,11 +221,11 @@ var _ = Describe("UpdateLoggingTypes", func() {
 					ClusterLogging: []ekstypes.LogSetup{
 						{
 							Enabled: aws.Bool(false),
-							Types:   utils.ConvertToLogTypes([]string{"disabled"}),
+							Types:   utils.ConvertToLogTypes([]string{"scheduler"}),
 						},
 						{
 							Enabled: aws.Bool(true),
-							Types:   utils.ConvertToLogTypes([]string{"test3-enabled"}),
+							Types:   utils.ConvertToLogTypes([]string{"controllerManager"}),
 						},
 					},
 				},
@@ -236,6 +236,23 @@ var _ = Describe("UpdateLoggingTypes", func() {
 		Expect(err).NotTo(HaveOccurred())
 	})
 
+	It("shouldn't update cluster logging types when no changes", func() {
+		updateLoggingTypesOpts = &UpdateLoggingTypesOpts{
+			EKSService: eksServiceMock,
+			Config: &eksv1.EKSClusterConfig{
+				Spec: eksv1.EKSClusterConfigSpec{
+					LoggingTypes: []string{"audit", "authenticator", "scheduler", "controllerManager"},
+				},
+			},
+			UpstreamClusterSpec: &eksv1.EKSClusterConfigSpec{
+				LoggingTypes: []string{"audit", "authenticator", "scheduler", "controllerManager"},
+			},
+		}
+		updated, err := UpdateClusterLoggingTypes(ctx, updateLoggingTypesOpts)
+		Expect(updated).To(BeFalse())
+		Expect(err).NotTo(HaveOccurred())
+	})
+
 	It("should return error if update cluster logging types failed", func() {
 		eksServiceMock.EXPECT().UpdateClusterConfig(ctx, gomock.Any()).Return(nil, errors.New("error updating cluster config"))
 		updated, err := UpdateClusterLoggingTypes(ctx, updateLoggingTypesOpts)