Merge pull request #378 from mvshao/auditlog-update

Handle the Kubernetes conflict error in KIM Audit Log

internal/auditlogging/auditlogging.go

@@ -147,9 +147,14 @@ func ApplyAuditLogConfig(shoot *gardener.Shoot, auditConfigFromFile map[string]m
 	}
 
 	changedExt, err := configureExtension(shoot, tenant)
+
+	if err != nil {
+		return false, err
+	}
+
 	changedSec := configureSecret(shoot, tenant)
 
-	return changedExt || changedSec, err
+	return changedExt || changedSec, nil
 }
 
 func configureExtension(shoot *gardener.Shoot, config AuditLogData) (changed bool, err error) {

internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go

@@ -7,6 +7,7 @@ import (
 	imv1 "github.com/kyma-project/infrastructure-manager/api/v1"
 	"github.com/kyma-project/infrastructure-manager/internal/auditlogging"
 	"github.com/pkg/errors"
+	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	ctrl "sigs.k8s.io/controller-runtime"
 )
 
@@ -15,7 +16,7 @@ func sFnConfigureAuditLog(ctx context.Context, m *fsm, s *systemState) (stateFn,
 
 	wasAuditLogEnabled, err := m.AuditLogging.Enable(ctx, s.shoot)
 
-	if wasAuditLogEnabled {
+	if wasAuditLogEnabled && err == nil {
 		m.log.Info("Audit Log configured for shoot: " + s.shoot.Name)
 		s.instance.UpdateStatePending(
 			imv1.ConditionTypeAuditLogConfigured,
@@ -30,6 +31,16 @@ func sFnConfigureAuditLog(ctx context.Context, m *fsm, s *systemState) (stateFn,
 	auditLogMandatoryString := strconv.FormatBool(m.RCCfg.AuditLogMandatory)
 
 	if err != nil { //nolint:nestif
+		if k8serrors.IsConflict(err) {
+			m.log.Error(err, "Conflict while updating Shoot object after applying Audit Log configuration, retrying")
+			s.instance.UpdateStatePending(
+				imv1.ConditionTypeAuditLogConfigured,
+				imv1.ConditionReasonAuditLogError,
+				"True",
+				err.Error(),
+			)
+			return updateStatusAndRequeue()
+		}
 		errorMessage := err.Error()
 		if errors.Is(err, auditlogging.ErrMissingMapping) {
 			if m.RCCfg.AuditLogMandatory {

internal/controller/runtime/fsm/runtime_fsm_configure_auditlog_test.go

@@ -2,15 +2,16 @@ package fsm
 
 import (
 	"context"
-	"github.com/kyma-project/infrastructure-manager/internal/auditlogging"
 	"testing"
 
 	gardener "github.com/gardener/gardener/pkg/apis/core/v1beta1"
 	v1 "github.com/kyma-project/infrastructure-manager/api/v1"
+	"github.com/kyma-project/infrastructure-manager/internal/auditlogging"
 	"github.com/kyma-project/infrastructure-manager/internal/auditlogging/mocks"
 	"github.com/pkg/errors"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -236,6 +237,43 @@ func TestAuditLogState(t *testing.T) {
 		assert.Equal(t, v1.RuntimeStateReady, string(systemState.instance.Status.State))
 		assert.Equal(t, expectedRuntimeConditions, systemState.instance.Status.Conditions)
 	})
+
+	t.Run("Should requeue in case of Kubernetes object conflict error during update of Shoot object and set status on Runtime CR", func(t *testing.T) {
+		// given
+		ctx := context.Background()
+		auditLog := &mocks.AuditLogging{}
+		shoot := shootForTest()
+		instance := runtimeForTest()
+		systemState := &systemState{
+			instance: instance,
+			shoot:    shoot,
+		}
+		expectedRuntimeConditions := []metav1.Condition{
+			{
+				Type:    string(v1.ConditionTypeAuditLogConfigured),
+				Status:  "True",
+				Reason:  string(v1.ConditionReasonAuditLogError),
+				Message: k8serrors.NewConflict(gardener.Resource("shoots"), shoot.Name, errors.New("k8s conflict on update error")).Error(),
+			},
+		}
+
+		fsm := &fsm{AuditLogging: auditLog}
+		fsm.RCCfg.AuditLogMandatory = true
+
+		auditLog.On("Enable", ctx, shoot).Return(false, k8serrors.NewConflict(gardener.Resource("shoots"), shoot.Name, errors.New("k8s conflict on update error"))).Once()
+
+		// when
+		stateFn, _, _ := sFnConfigureAuditLog(ctx, fsm, systemState)
+
+		// set the time to its zero value for comparison purposes
+		systemState.instance.Status.Conditions[0].LastTransitionTime = metav1.Time{}
+
+		// then
+		auditLog.AssertExpectations(t)
+		require.Contains(t, stateFn.name(), "sFnUpdateStatus")
+		assert.Equal(t, v1.RuntimeStatePending, string(systemState.instance.Status.State))
+		assert.Equal(t, expectedRuntimeConditions, systemState.instance.Status.Conditions)
+	})
 }
 
 func shootForTest() *gardener.Shoot {

internal/controller/runtime/fsm/runtime_fsm_waiting_for_shoot_reconcile.go

@@ -47,9 +47,9 @@ func sFnWaitForShootReconcile(_ context.Context, m *fsm, s *systemState) (stateF
 		return ensureStatusConditionIsSetAndContinue(
 			&s.instance,
 			imv1.ConditionTypeRuntimeProvisioned,
-			imv1.ConditionReasonAuditLogConfigured,
+			imv1.ConditionReasonConfigurationCompleted,
 			"Runtime processing completed successfully",
-			sFnConfigureAuditLog)
+			sFnApplyClusterRoleBindings)
 	}
 
 	m.log.Info("Update did not processed, exiting with no retry")

internal/controller/runtime/runtime_controller_test.go

@@ -157,7 +157,7 @@ var _ = Describe("Runtime Controller", func() {
 					return false
 				}
 
-				if !runtime.IsConditionSet(imv1.ConditionTypeRuntimeProvisioned, imv1.ConditionReasonAuditLogConfigured) {
+				if !runtime.IsConditionSet(imv1.ConditionTypeRuntimeProvisioned, imv1.ConditionReasonConfigurationCompleted) {
 					return false
 				}