Skip to content

Commit

Permalink
Fix logging types updates
Browse files Browse the repository at this point in the history
Update getLoggingTypesUpdate function when there are no changes
for logging types. Adding also test scenario to avoid regressions
in the future.

Issue: #546
  • Loading branch information
mjura committed Jun 10, 2024
1 parent 5c82d04 commit 39a79ae
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 6 deletions.
8 changes: 6 additions & 2 deletions pkg/eks/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
25 changes: 21 additions & 4 deletions pkg/eks/update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
},
}
})
Expand All @@ -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"}),
},
},
},
Expand All @@ -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)
Expand Down

0 comments on commit 39a79ae

Please sign in to comment.