Merge pull request #417 from akgalwas/fix-update-problem

Fixed problem with skipping update operations

internal/controller/runtime/fsm/runtime_fsm_patch_shoot.go

@@ -7,6 +7,7 @@ import (
 	imv1 "github.com/kyma-project/infrastructure-manager/api/v1"
 	"github.com/kyma-project/infrastructure-manager/internal/config"
 	gardener_shoot "github.com/kyma-project/infrastructure-manager/internal/gardener/shoot"
+	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/utils/ptr"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -29,6 +30,11 @@ func sFnPatchExistingShoot(ctx context.Context, m *fsm, s *systemState) (stateFn
 	})
 
 	if err != nil {
+		if k8serrors.IsConflict(err) {
+			m.log.Info("Gardener shoot for runtime is outdated, retrying", "Name", s.shoot.Name, "Namespace", s.shoot.Namespace)
+			return updateStatusAndRequeueAfter(gardenerRequeueDuration)
+		}
+
 		m.log.Error(err, "Failed to patch shoot object, exiting with no retry")
 		return updateStatePendingWithErrorAndStop(&s.instance, imv1.ConditionTypeRuntimeProvisioned, imv1.ConditionReasonProcessingErr, "Shoot patch error")
 	}
@@ -40,8 +46,6 @@ func sFnPatchExistingShoot(ctx context.Context, m *fsm, s *systemState) (stateFn
 
 	m.log.Info("Gardener shoot for runtime patched successfully", "Name", s.shoot.Name, "Namespace", s.shoot.Namespace)
 
-	s.shoot = updatedShoot.DeepCopy()
-
 	s.instance.UpdateStatePending(
 		imv1.ConditionTypeRuntimeProvisioned,
 		imv1.ConditionReasonProcessing,

internal/controller/runtime/fsm/runtime_fsm_select_shoot_processing.go

@@ -3,9 +3,11 @@ package fsm
 import (
 	"context"
 	"fmt"
+	"strconv"
 
 	gardener "github.com/gardener/gardener/pkg/apis/core/v1beta1"
 	imv1 "github.com/kyma-project/infrastructure-manager/api/v1"
+	"github.com/kyma-project/infrastructure-manager/internal/gardener/shoot/extender"
 	ctrl "sigs.k8s.io/controller-runtime"
 )
 
@@ -27,8 +29,14 @@ func sFnSelectShootProcessing(_ context.Context, m *fsm, s *systemState) (stateF
 		return updateStatusAndRequeueAfter(gardenerRequeueDuration)
 	}
 
-	if s.instance.Status.State == imv1.RuntimeStateReady && lastOperation.State == gardener.LastOperationStateSucceeded {
-		// only allow to patch if full previous cycle was completed
+	patchShoot, err := shouldPatchShoot(&s.instance, s.shoot)
+	if err != nil {
+		msg := fmt.Sprintf("Failed to get applied generation for shoot: %s, scheduling for retry", s.shoot.Name)
+		m.log.Error(err, msg)
+		return updateStatusAndStop()
+	}
+
+	if patchShoot {
 		m.log.Info("Gardener shoot already exists, updating")
 		return switchState(sFnPatchExistingShoot)
 	}
@@ -44,3 +52,19 @@ func sFnSelectShootProcessing(_ context.Context, m *fsm, s *systemState) (stateF
 	m.log.Info("Unknown shoot operation type, exiting with no retry")
 	return stop()
 }
+
+func shouldPatchShoot(runtime *imv1.Runtime, shoot *gardener.Shoot) (bool, error) {
+	runtimeGeneration := runtime.GetGeneration()
+	appliedGenerationString, found := shoot.GetAnnotations()[extender.ShootRuntimeGenerationAnnotation]
+
+	if !found {
+		return true, nil
+	}
+
+	appliedGeneration, err := strconv.ParseInt(appliedGenerationString, 10, 64)
+	if err != nil {
+		return false, err
+	}
+
+	return appliedGeneration < runtimeGeneration, nil
+}

internal/controller/runtime/runtime_controller_test.go

@@ -18,13 +18,13 @@ package runtime
 
 import (
 	"context"
+	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	"time"
 
 	gardener "github.com/gardener/gardener/pkg/apis/core/v1beta1"
 	imv1 "github.com/kyma-project/infrastructure-manager/api/v1"
 	. "github.com/onsi/ginkgo/v2" //nolint:revive
 	. "github.com/onsi/gomega"    //nolint:revive
-	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/utils/ptr"
@@ -178,7 +178,6 @@ var _ = Describe("Runtime Controller", func() {
 			Expect(customTracker.IsSequenceFullyUsed()).To(BeTrue())
 
 			// next test will be for runtime deletion
-
 			By("Process deleting of Runtime CR and delete GardenerCluster CR and Shoot")
 			setupGardenerTestClientForDelete()
 
@@ -253,6 +252,7 @@ func CreateRuntimeStub(resourceName string) *imv1.Runtime {
 				imv1.LabelKymaSubaccountID:        "c5ad84ae-3d1b-4592-bee1-f022661f7b30",
 				imv1.LabelControlledByProvisioner: "false",
 			},
+			Generation: 1,
 		},
 		Spec: imv1.RuntimeSpec{
 			Shoot: imv1.RuntimeShoot{

internal/controller/runtime/suite_test.go

@@ -19,7 +19,10 @@ package runtime
 import (
 	"context"
 	"encoding/json"
+	"github.com/pkg/errors"
+	"k8s.io/apimachinery/pkg/types"
 	"path/filepath"
+	"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
 	"testing"
 	"time"
 
@@ -170,7 +173,20 @@ func setupGardenerClientWithSequence(shoots []*gardener_api.Shoot, seeds []*gard
 
 	tracker := clienttesting.NewObjectTracker(clientScheme, serializer.NewCodecFactory(clientScheme).UniversalDecoder())
 	customTracker = NewCustomTracker(tracker, shoots, seeds)
-	gardenerTestClient = fake.NewClientBuilder().WithScheme(clientScheme).WithObjectTracker(customTracker).Build()
+	gardenerTestClient = fake.NewClientBuilder().WithScheme(clientScheme).WithObjectTracker(customTracker).
+		WithInterceptorFuncs(interceptor.Funcs{Patch: func(ctx context.Context, clnt client.WithWatch, obj client.Object, patch client.Patch, opts ...client.PatchOption) error {
+			// Apply patches are supposed to upsert, but fake client fails if the object doesn't exist,
+			// Update the generation to simulate the object being updated using interceptor function.
+			if patch.Type() != types.ApplyPatchType {
+				return clnt.Patch(ctx, obj, patch, opts...)
+			}
+			shoot, ok := obj.(*gardener_api.Shoot)
+			if !ok {
+				return errors.New("failed to cast object to shoot")
+			}
+			shoot.Generation++
+			return nil
+		}}).Build()
 	runtimeReconciler.UpdateShootClient(gardenerTestClient)
 }
 
@@ -226,22 +242,27 @@ func fixShootsSequenceForProvisioning(shoot *gardener_api.Shoot) []*gardener_api
 }
 
 func fixShootsSequenceForUpdate(shoot *gardener_api.Shoot) []*gardener_api.Shoot {
-	pendingShoot := shoot.DeepCopy()
-
-	addAuditLogConfigToShoot(pendingShoot)
+	existingShoot := shoot.DeepCopy()
+	existingShoot.Spec.SeedName = ptr.To("test-seed")
 
-	pendingShoot.Spec.DNS = &gardener_api.DNS{
-		Domain: ptr.To("test.domain"),
-	}
-
-	pendingShoot.Status = gardener_api.ShootStatus{
+	existingShoot.Status = gardener_api.ShootStatus{
 		LastOperation: &gardener_api.LastOperation{
 			Type:  gardener_api.LastOperationTypeReconcile,
-			State: gardener_api.LastOperationStatePending,
+			State: gardener_api.LastOperationStateSucceeded,
 		},
 	}
 
-	pendingShoot.Spec.SeedName = ptr.To("test-seed")
+	existingShoot.Spec.DNS = &gardener_api.DNS{
+		Domain: ptr.To("test.domain"),
+	}
+
+	addAuditLogConfigToShoot(existingShoot)
+
+	pendingShoot := existingShoot.DeepCopy()
+
+	pendingShoot.ObjectMeta.Annotations["infrastructuremanager.kyma-project.io/runtime-generation"] = "2"
+
+	pendingShoot.Status.LastOperation.State = gardener_api.LastOperationStatePending
 
 	processingShoot := pendingShoot.DeepCopy()
 
@@ -253,7 +274,7 @@ func fixShootsSequenceForUpdate(shoot *gardener_api.Shoot) []*gardener_api.Shoot
 
 	// processedShoot := processingShoot.DeepCopy() // will add specific data later
 
-	return []*gardener_api.Shoot{pendingShoot, processingShoot, readyShoot, readyShoot}
+	return []*gardener_api.Shoot{existingShoot, pendingShoot, processingShoot, readyShoot, readyShoot}
 }
 
 func fixShootsSequenceForDelete(shoot *gardener_api.Shoot) []*gardener_api.Shoot {

internal/gardener/shoot/extender/annotations.go

@@ -1,6 +1,8 @@
 package extender
 
 import (
+	"fmt"
+
 	gardener "github.com/gardener/gardener/pkg/apis/core/v1beta1"
 	imv1 "github.com/kyma-project/infrastructure-manager/api/v1"
 )
@@ -11,6 +13,7 @@ import (
 //- support.gardener.cloud/eu-access-for-cluster-nodes
 
 const (
+	ShootRuntimeGenerationAnnotation  = "infrastructuremanager.kyma-project.io/runtime-generation"
 	ShootRuntimeIDAnnotation          = "infrastructuremanager.kyma-project.io/runtime-id"
 	ShootLicenceTypeAnnotation        = "infrastructuremanager.kyma-project.io/licence-type"
 	RuntimeIDLabel                    = "kyma-project.io/runtime-id"
@@ -25,7 +28,8 @@ func ExtendWithAnnotations(runtime imv1.Runtime, shoot *gardener.Shoot) error {
 
 func getAnnotations(runtime imv1.Runtime) map[string]string {
 	annotations := map[string]string{
-		ShootRuntimeIDAnnotation: runtime.Labels[RuntimeIDLabel],
+		ShootRuntimeIDAnnotation:         runtime.Labels[RuntimeIDLabel],
+		ShootRuntimeGenerationAnnotation: fmt.Sprintf("%v", runtime.Generation),
 	}
 
 	if runtime.Spec.Shoot.LicenceType != nil && *runtime.Spec.Shoot.LicenceType != "" {

internal/gardener/shoot/extender/annotations_test.go

@@ -26,10 +26,12 @@ func TestAnnotationsExtender(t *testing.T) {
 					Labels: map[string]string{
 						"kyma-project.io/runtime-id": "runtime-id",
 					},
+					Generation: 100,
 				},
 			},
 			expectedAnnotations: map[string]string{
-				"infrastructuremanager.kyma-project.io/runtime-id": "runtime-id"},
+				"infrastructuremanager.kyma-project.io/runtime-id":         "runtime-id",
+				"infrastructuremanager.kyma-project.io/runtime-generation": "100"},
 		},
 		{
 			name: "Create licence type annotation",
@@ -48,8 +50,9 @@ func TestAnnotationsExtender(t *testing.T) {
 				},
 			},
 			expectedAnnotations: map[string]string{
-				"infrastructuremanager.kyma-project.io/runtime-id":   "runtime-id",
-				"infrastructuremanager.kyma-project.io/licence-type": "licence"},
+				"infrastructuremanager.kyma-project.io/runtime-id":         "runtime-id",
+				"infrastructuremanager.kyma-project.io/licence-type":       "licence",
+				"infrastructuremanager.kyma-project.io/runtime-generation": "0"},
 		},
 		{
 			name: "Create restricted EU access annotation for cf-eu11 region",
@@ -68,8 +71,9 @@ func TestAnnotationsExtender(t *testing.T) {
 				},
 			},
 			expectedAnnotations: map[string]string{
-				"infrastructuremanager.kyma-project.io/runtime-id":   "runtime-id",
-				"support.gardener.cloud/eu-access-for-cluster-nodes": "true"},
+				"infrastructuremanager.kyma-project.io/runtime-id":         "runtime-id",
+				"support.gardener.cloud/eu-access-for-cluster-nodes":       "true",
+				"infrastructuremanager.kyma-project.io/runtime-generation": "0"},
 		},
 		{
 			name: "Create restricted EU access annotation for cf-ch20 region",
@@ -88,8 +92,9 @@ func TestAnnotationsExtender(t *testing.T) {
 				},
 			},
 			expectedAnnotations: map[string]string{
-				"infrastructuremanager.kyma-project.io/runtime-id":   "runtime-id",
-				"support.gardener.cloud/eu-access-for-cluster-nodes": "true"},
+				"infrastructuremanager.kyma-project.io/runtime-id":         "runtime-id",
+				"support.gardener.cloud/eu-access-for-cluster-nodes":       "true",
+				"infrastructuremanager.kyma-project.io/runtime-generation": "0"},
 		},
 	} {
 		// given