Fixes for making shoot comparison easier

hack/shoot-comparator/pkg/shoot/matcher.go

@@ -64,6 +64,8 @@ func (m *Matcher) Match(actual interface{}) (success bool, err error) {
 		return false, err
 	}
 
+	// Note: we define separate matchers for each field to make input more readable
+	// Annotations are not matched as they are not relevant for the comparison ; both KIM, and Provisioner have different set of annotations
 	for _, matcher := range []matcher{
 		{
 			GomegaMatcher: gomega.Equal(eShoot.TypeMeta),
@@ -85,14 +87,120 @@ func (m *Matcher) Match(actual interface{}) (success bool, err error) {
 			path:          "metadata/labels",
 		},
 		{
-			GomegaMatcher: gomega.Equal(eShoot.Annotations),
-			expected:      aShoot.Annotations,
-			path:          "metadata/annotations",
+			GomegaMatcher: gomega.Equal(eShoot.Spec.Addons),
+			expected:      aShoot.Spec.Addons,
+			path:          "spec/Addons",
 		},
 		{
-			GomegaMatcher: gomega.Equal(eShoot.Spec),
-			expected:      aShoot.Spec,
-			path:          "spec",
+			GomegaMatcher: gomega.Equal(eShoot.Spec.CloudProfileName),
+			expected:      aShoot.Spec.CloudProfileName,
+			path:          "spec/CloudProfileName",
+		},
+		{
+			GomegaMatcher: gomega.Equal(eShoot.Spec.DNS),
+			expected:      aShoot.Spec.DNS,
+			path:          "spec/DNS",
+		},
+		{
+			GomegaMatcher: gomega.Equal(eShoot.Spec.Extensions),
+			expected:      aShoot.Spec.Extensions,
+			path:          "spec/Extensions",
+		},
+		{
+			GomegaMatcher: gomega.Equal(eShoot.Spec.Hibernation),
+			expected:      aShoot.Spec.Hibernation,
+			path:          "spec/Hibernation",
+		},
+		{
+			GomegaMatcher: gomega.Equal(eShoot.Spec.Kubernetes),
+			expected:      aShoot.Spec.Kubernetes,
+			path:          "spec/Kubernetes",
+		},
+		{
+			GomegaMatcher: gomega.Equal(eShoot.Spec.Networking),
+			expected:      aShoot.Spec.Networking,
+			path:          "spec/Networking",
+		},
+		{
+			GomegaMatcher: gomega.Equal(eShoot.Spec.Maintenance),
+			expected:      aShoot.Spec.Maintenance,
+			path:          "spec/Maintenance",
+		},
+		{
+			GomegaMatcher: gomega.Equal(eShoot.Spec.Monitoring),
+			expected:      aShoot.Spec.Monitoring,
+			path:          "spec/Monitoring",
+		},
+		{
+			GomegaMatcher: gomega.Equal(eShoot.Spec.Provider),
+			expected:      aShoot.Spec.Provider,
+			path:          "spec/Provider",
+		},
+
+		{
+			GomegaMatcher: gomega.Equal(eShoot.Spec.Purpose),
+			expected:      aShoot.Spec.Purpose,
+			path:          "spec/Purpose",
+		},
+		{
+			GomegaMatcher: gomega.Equal(eShoot.Spec.Region),
+			expected:      aShoot.Spec.Region,
+			path:          "spec/Region",
+		},
+		{
+			GomegaMatcher: gomega.Equal(eShoot.Spec.SecretBindingName),
+			expected:      aShoot.Spec.SecretBindingName,
+			path:          "spec/SecretBindingName",
+		},
+		{
+			GomegaMatcher: gomega.Equal(eShoot.Spec.SeedName),
+			expected:      aShoot.Spec.SeedName,
+			path:          "spec/SeedName",
+		},
+		{
+			GomegaMatcher: gomega.Equal(eShoot.Spec.SeedSelector),
+			expected:      aShoot.Spec.SeedSelector,
+			path:          "spec/SeedSelector",
+		},
+		{
+			GomegaMatcher: gomega.Equal(eShoot.Spec.Resources),
+			expected:      aShoot.Spec.Resources,
+			path:          "spec/Resources",
+		},
+		{
+			GomegaMatcher: gomega.Equal(eShoot.Spec.Tolerations),
+			expected:      aShoot.Spec.Tolerations,
+			path:          "spec/Tolerations",
+		},
+		{
+			GomegaMatcher: gomega.Equal(eShoot.Spec.ExposureClassName),
+			expected:      aShoot.Spec.ExposureClassName,
+			path:          "spec/ExposureClassName",
+		},
+		{
+			GomegaMatcher: gomega.Equal(eShoot.Spec.SystemComponents),
+			expected:      aShoot.Spec.SystemComponents,
+			path:          "spec/SystemComponents",
+		},
+		{
+			GomegaMatcher: gomega.Equal(eShoot.Spec.ControlPlane),
+			expected:      aShoot.Spec.ControlPlane,
+			path:          "spec/ControlPlane",
+		},
+		{
+			GomegaMatcher: gomega.Equal(eShoot.Spec.SchedulerName),
+			expected:      aShoot.Spec.SchedulerName,
+			path:          "spec/SchedulerName",
+		},
+		{
+			GomegaMatcher: gomega.Equal(eShoot.Spec.CloudProfile),
+			expected:      aShoot.Spec.CloudProfile,
+			path:          "spec/CloudProfile",
+		},
+		{
+			GomegaMatcher: gomega.Equal(eShoot.Spec.CredentialsBindingName),
+			expected:      aShoot.Spec.CredentialsBindingName,
+			path:          "spec/CredentialsBindingName",
 		},
 	} {
 		ok, err := matcher.Match(matcher.expected)

hack/shoot-comparator/pkg/shoot/matcher_test.go

@@ -26,12 +26,6 @@ func withLabels(labels map[string]string) deepCpOpts {
 	}
 }
 
-func withAnnotations(annotations map[string]string) deepCpOpts {
-	return func(s *v1beta1.Shoot) {
-		s.Annotations = annotations
-	}
-}
-
 func withShootSpec(spec v1beta1.ShootSpec) deepCpOpts {
 	return func(s *v1beta1.Shoot) {
 		s.Spec = spec
@@ -103,12 +97,6 @@ var _ = Describe(":: shoot matcher :: ", func() {
 			deepCp(empty, withLabels(map[string]string{})),
 			false,
 		),
-		Entry(
-			"should detect difference in annotations",
-			deepCp(empty, withAnnotations(map[string]string{"test": "me"})),
-			deepCp(empty, withAnnotations(map[string]string{"test": "it"})),
-			false,
-		),
 		Entry(
 			"should detect differences in spec",
 			deepCp(empty, withShootSpec(v1beta1.ShootSpec{

internal/controller/runtime/fsm/runtime_fsm_patch_shoot.go

@@ -49,11 +49,6 @@ func sFnPatchExistingShoot(ctx context.Context, m *fsm, s *systemState) (stateFn
 		"Shoot is pending",
 	)
 
-	shouldDumpShootSpec := m.PVCPath != ""
-	if shouldDumpShootSpec {
-		return switchState(sFnDumpShootSpec)
-	}
-
 	return updateStatusAndRequeueAfter(gardenerRequeueDuration)
 }
 
@@ -63,13 +58,13 @@ func convertShoot(instance *imv1.Runtime, cfg shoot.ConverterConfig) (gardener.S
 	}
 
 	converter := gardener_shoot.NewConverter(cfg)
-	shoot, err := converter.ToShoot(*instance)
+	newShoot, err := converter.ToShoot(*instance)
 
 	if err == nil {
-		setObjectFields(&shoot)
+		setObjectFields(&newShoot)
 	}
 
-	return shoot, err
+	return newShoot, err
 }
 
 // workaround

internal/controller/runtime/fsm/runtime_fsm_persist_shoot.go

@@ -38,11 +38,16 @@ func persist(path string, s interface{}, saveFunc writerGetter) error {
 func sFnDumpShootSpec(_ context.Context, m *fsm, s *systemState) (stateFn, *ctrl.Result, error) {
 	paths := createFilesPath(m.PVCPath, s.shoot.Namespace, s.shoot.Name)
 
-	shootCp := s.shoot.DeepCopy()
+	// To make comparison easier we don't store object obtained from the cluster as it contains additional fields that are not relevant for the comparison.
+	// We use object created by the converter instead (the Provisioner uses the same approach)
+	convertedShoot, err := convertShoot(&s.instance, m.ConverterConfig)
+	if err != nil {
+		return updateStatusAndStopWithError(err)
+	}
+
 	runtimeCp := s.instance.DeepCopy()
-	shootCp.ManagedFields = nil
 
-	if err := persist(paths["shoot"], shootCp, m.writerProvider); err != nil {
+	if err := persist(paths["shoot"], convertedShoot, m.writerProvider); err != nil {
 		return updateStatusAndStopWithError(err)
 	}
 

internal/controller/runtime/fsm/runtime_fsm_persist_shoot_test.go

@@ -3,6 +3,7 @@ package fsm
 import (
 	"bytes"
 	"context"
+	"github.com/kyma-project/infrastructure-manager/internal/gardener/shoot"
 	"io"
 	"time"
 
@@ -24,15 +25,25 @@ var _ = Describe("KIM sFnPersist", func() {
 	testCtx, cancel := context.WithTimeout(context.Background(), time.Second)
 	defer cancel()
 
-	shootWrite, err := yaml.Marshal(&testing.ShootNoDNS)
-	runtimeWrite, err := yaml.Marshal(&testing.RuntimeOnlyName)
+	expectedRuntime := testing.RuntimeOnlyName.DeepCopy()
+	expectedRuntime.Spec.Shoot.Provider.Type = "aws"
+
+	expectedShoot, err := convertShoot(expectedRuntime, shoot.ConverterConfig{})
+	Expect(err).To(BeNil())
+
+	shootWrite, err := yaml.Marshal(&expectedShoot)
+	Expect(err).To(BeNil())
+
+	runtimeWrite, err := yaml.Marshal(expectedRuntime)
+	Expect(err).To(BeNil())
+
 	expectedData := append(shootWrite, runtimeWrite...)
 	Expect(err).ShouldNot(HaveOccurred())
 
 	It("should persist shoot data", func() {
-		next, _, err := sFnDumpShootSpec(testCtx, must(newFakeFSM, withStorageWriter(testWriterGetter)), &systemState{shoot: &testing.ShootNoDNS, instance: testing.RuntimeOnlyName})
+		next, _, err := sFnDumpShootSpec(testCtx, must(newFakeFSM, withStorageWriter(testWriterGetter), withConverterConfig(shoot.ConverterConfig{})), &systemState{shoot: &testing.ShootNoDNS, instance: *expectedRuntime})
 		Expect(err).To(BeNil())
 		Expect(next).To(haveName("sFnUpdateStatus"))
-		Expect(expectedData).To(Equal(b.Bytes()))
+		Expect(b.Bytes()).To(Equal(expectedData))
 	})
 })

internal/controller/runtime/fsm/testing/shoot.go

@@ -18,6 +18,20 @@ var (
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      "test-instance",
 			Namespace: "default",
+			Labels: map[string]string{
+				"kyma-project.io/instance-id":         "instance-id",
+				"kyma-project.io/runtime-id":          "runtime-id",
+				"kyma-project.io/shoot-name":          "shoot-name",
+				"kyma-project.io/region":              "region",
+				"operator.kyma-project.io/kyma-name":  "kyma-name",
+				"kyma-project.io/broker-plan-id":      "broker-plan-id",
+				"kyma-project.io/broker-plan-name":    "broker-plan-name",
+				"kyma-project.io/global-account-id":   "global-account-id",
+				"kyma-project.io/subaccount-id":       "subaccount-id",
+				"operator.kyma-project.io/managed-by": "managed-by",
+				"operator.kyma-project.io/internal":   "false",
+				"kyma-project.io/platform-region":     "platform-region",
+			},
 		},
 		Spec: v1.RuntimeSpec{
 			Shoot: v1.RuntimeShoot{Name: "test-shoot"},

internal/controller/runtime/fsm/utilz_for_test.go

@@ -2,6 +2,7 @@ package fsm
 
 import (
 	"fmt"
+	"github.com/kyma-project/infrastructure-manager/internal/gardener/shoot"
 
 	. "github.com/onsi/gomega" //nolint:revive
 	"k8s.io/apimachinery/pkg/runtime"
@@ -35,6 +36,13 @@ var (
 		}
 	}
 
+	withConverterConfig = func(config shoot.ConverterConfig) fakeFSMOpt {
+		return func(fsm *fsm) error {
+			fsm.ConverterConfig = config
+			return nil
+		}
+	}
+
 	withFakedK8sClient = func(
 		scheme *runtime.Scheme,
 		objs ...client.Object) fakeFSMOpt {