From 358b2ac4c92734698cf000db317f444d152541eb Mon Sep 17 00:00:00 2001 From: Tomasz Smelcerz Date: Mon, 4 Nov 2024 13:49:41 +0100 Subject: [PATCH 01/15] initial API preparation --- internal/controller/kyma/controller.go | 8 +- internal/remote/moduletemplate_syncer_test.go | 78 +++++++++++++++++-- internal/remote/remote_catalog.go | 49 ++++++++---- 3 files changed, 111 insertions(+), 24 deletions(-) diff --git a/internal/controller/kyma/controller.go b/internal/controller/kyma/controller.go index 1bb6b83136..daca9f69f0 100644 --- a/internal/controller/kyma/controller.go +++ b/internal/controller/kyma/controller.go @@ -519,8 +519,14 @@ func (r *Reconciler) syncModuleCatalog(ctx context.Context, kyma *v1beta2.Kyma) modulesToSync = append(modulesToSync, mt) } } + + moduleReleaseMetaList := &v1beta2.ModuleReleaseMetaList{} + if err := r.List(ctx, moduleReleaseMetaList, &client.ListOptions{}); err != nil { + return fmt.Errorf("could not aggregate module release metas for module catalog sync: %w", err) + } + remoteCatalog := remote.NewRemoteCatalogFromKyma(r.Client, r.SkrContextFactory, r.RemoteSyncNamespace) - if err := remoteCatalog.Sync(ctx, kyma.GetNamespacedName(), modulesToSync); err != nil { + if err := remoteCatalog.Sync(ctx, kyma.GetNamespacedName(), modulesToSync, moduleReleaseMetaList.Items); err != nil { return fmt.Errorf("could not synchronize remote module catalog: %w", err) } diff --git a/internal/remote/moduletemplate_syncer_test.go b/internal/remote/moduletemplate_syncer_test.go index aff172383e..b45c4d62ad 100644 --- a/internal/remote/moduletemplate_syncer_test.go +++ b/internal/remote/moduletemplate_syncer_test.go @@ -15,9 +15,11 @@ import ( "github.com/kyma-project/lifecycle-manager/api/v1beta2" ) +// TestSyncer_SyncToSKR_happypath tests the happy path of the SyncToSKR method, +// with some modules to be installed in the SKR and some modules to be deleted from the SKR. func TestSyncer_SyncToSKR_happypath(t *testing.T) { // given - mtKCP1 := moduleTemplate("mt1", "kcp-system") + mtKCP1 := moduleTemplate("mt1", "kcp-system") // this one should be installed in the SKR, because it's not there mtKCP2 := moduleTemplate("mt2", "kcp-system") mtKCP3 := moduleTemplate("mt3", "kcp-system") @@ -67,20 +69,74 @@ func TestSyncer_SyncToSKR_happypath(t *testing.T) { } } - force := true - settings := &Settings{ - Namespace: "kyma-system", - SSAPatchOptions: &client.PatchOptions{FieldManager: moduleCatalogSyncFieldManager, Force: &force}, + subject := syncer{ + skrClient: skrClient, + settings: getSettings(), + syncWorkerFactoryFn: syncWokerFactoryFn, + } + + // when + err = subject.SyncToSKR(context.Background(), types.NamespacedName{Name: "kyma", Namespace: "kcp-system"}, []v1beta2.ModuleTemplate{mtKCP1, mtKCP2, mtKCP3}) + + // then + assert.NoError(t, err) +} + +// TestSyncer_SyncToSKR_nilList tests the case when the list of KCP modules is nil. +func TestSyncer_SyncToSKR_nilList(t *testing.T) { + // given + mtSKR2 := moduleTemplate("mt2", "kyma-system") // should be deleted, because it's not in the KCP + mtSKR3 := moduleTemplate("mt3", "kyma-system") // should be deleted, because it's not in the KCP + mtSKR4 := moduleTemplate("mt4", "kyma-system") // should be deleted, because it's not in the KCP + + // Create a fake client with the SKR modules + scheme, err := v1beta2.SchemeBuilder.Build() + require.NoError(t, err) + skrClient := fake.NewClientBuilder(). + WithObjects(&mtSKR2, &mtSKR3, &mtSKR4). + WithScheme(scheme). + Build() + + // onSyncConcurrentlyFn "pretends" to be the moduleTemplateConcurrentWorker.SyncConcurrently + onSyncConcurrentlyFn := func(_ context.Context, kcpModules []v1beta2.ModuleTemplate) { + if kcpModules != nil { + t.Errorf("Expected nil kcp modules, got %v", kcpModules) + } + } + + // onDeleteConcurrentlyFn "pretends" to be the moduleTemplateConcurrentWorker.DeleteConcurrently + onDeleteConcurrentlyFn := func(_ context.Context, runtimeModules []v1beta2.ModuleTemplate) { + if len(runtimeModules) != 3 { + t.Errorf("Expected 1 runtime module, got %d", len(runtimeModules)) + } + if runtimeModules[0].Name != "mt2" { + t.Errorf("Expected module mt2, got %s", runtimeModules[0].Name) + } + if runtimeModules[1].Name != "mt3" { + t.Errorf("Expected module mt2, got %s", runtimeModules[1].Name) + } + if runtimeModules[2].Name != "mt4" { + t.Errorf("Expected module mt2, got %s", runtimeModules[2].Name) + } + } + + syncWokerFactoryFn := func(kcpClient, skrClient client.Client, settings *Settings) syncWorker { + return &fakeSyncWorker{ + namespace: settings.Namespace, + onSyncConcurrently: onSyncConcurrentlyFn, + onDeleteConcurrently: onDeleteConcurrentlyFn, + } } subject := syncer{ skrClient: skrClient, - settings: settings, + settings: getSettings(), syncWorkerFactoryFn: syncWokerFactoryFn, } // when - err = subject.SyncToSKR(context.Background(), types.NamespacedName{Name: "kyma", Namespace: "kcp-system"}, []v1beta2.ModuleTemplate{mtKCP1, mtKCP2, mtKCP3}) + var nilModuleTemplateList []v1beta2.ModuleTemplate = nil + err = subject.SyncToSKR(context.Background(), types.NamespacedName{Name: "kyma", Namespace: "kcp-system"}, nilModuleTemplateList) // then assert.NoError(t, err) @@ -100,6 +156,14 @@ func moduleTemplate(name, namespace string) v1beta2.ModuleTemplate { } } +func getSettings() *Settings { + force := true + return &Settings{ + Namespace: "kyma-system", + SSAPatchOptions: &client.PatchOptions{FieldManager: moduleCatalogSyncFieldManager, Force: &force}, + } +} + // Implements the syncWorker interface. type fakeSyncWorker struct { namespace string diff --git a/internal/remote/remote_catalog.go b/internal/remote/remote_catalog.go index 084105b505..0d17b68e6b 100644 --- a/internal/remote/remote_catalog.go +++ b/internal/remote/remote_catalog.go @@ -2,6 +2,7 @@ package remote import ( "context" + "errors" "fmt" "k8s.io/apimachinery/pkg/types" @@ -19,20 +20,29 @@ type Settings struct { } type RemoteCatalog struct { - kcpClient client.Client - skrContextFactory SkrContextProvider - settings Settings - syncerAPIFactoryFn syncerAPIFactory + kcpClient client.Client + skrContextFactory SkrContextProvider + settings Settings + moduleTemplateSyncerFactoryFn moduleTemplateSyncerFactory + moduleReleaseMetaSyncerFactoryFn moduleReleaseMetaSyncerFactory } -// syncerAPI encapsulates the top-level abstration for syncing module templates to a remote cluster. -type syncerAPI interface { +// moduleTemplateSyncer encapsulates the top-level abstration for syncing module templates to a remote cluster. +type moduleTemplateSyncer interface { SyncToSKR(ctx context.Context, kyma types.NamespacedName, kcpModules []v1beta2.ModuleTemplate) error DeleteAllManaged(ctx context.Context, kyma types.NamespacedName) error } -// syncerAPIFactory is a function that creates a new syncerAPI. -type syncerAPIFactory func(kcpClient, skrClient client.Client, settings *Settings) syncerAPI +type moduleReleaseMetaSyncer interface { + SyncToSKR(ctx context.Context, kyma types.NamespacedName, kcpModuleReleaseMeta []v1beta2.ModuleReleaseMeta) error + DeleteAllManaged(ctx context.Context, kyma types.NamespacedName) error +} + +// moduleTemplateSyncerFactory is a function that creates moduleTemplateSyncer instances. +type moduleTemplateSyncerFactory func(kcpClient, skrClient client.Client, settings *Settings) moduleTemplateSyncer + +// moduleReleaseMetaSyncerFactory is a function that creates moduleReleaseMetaSyncer instances. +type moduleReleaseMetaSyncerFactory func(kcpClient, skrClient client.Client, settings *Settings) moduleReleaseMetaSyncer func NewRemoteCatalogFromKyma(kcpClient client.Client, skrContextFactory SkrContextProvider, remoteSyncNamespace string, @@ -47,15 +57,16 @@ func NewRemoteCatalogFromKyma(kcpClient client.Client, skrContextFactory SkrCont } func newRemoteCatalog(kcpClient client.Client, skrContextFactory SkrContextProvider, settings Settings) *RemoteCatalog { - var syncerAPIFactoryFn syncerAPIFactory = func(kcpClient, skrClient client.Client, settings *Settings) syncerAPI { + var syncerAPIFactoryFn moduleTemplateSyncerFactory = func(kcpClient, skrClient client.Client, settings *Settings) moduleTemplateSyncer { return newSyncer(kcpClient, skrClient, settings) } res := &RemoteCatalog{ - kcpClient: kcpClient, - skrContextFactory: skrContextFactory, - settings: settings, - syncerAPIFactoryFn: syncerAPIFactoryFn, + kcpClient: kcpClient, + skrContextFactory: skrContextFactory, + settings: settings, + moduleTemplateSyncerFactoryFn: syncerAPIFactoryFn, + moduleReleaseMetaSyncerFactoryFn: nil, //TODO: Wire up } return res @@ -65,14 +76,20 @@ func (c *RemoteCatalog) Sync( ctx context.Context, kyma types.NamespacedName, kcpModules []v1beta2.ModuleTemplate, + kcpModuleReleaseMeta []v1beta2.ModuleReleaseMeta, ) error { skrContext, err := c.skrContextFactory.Get(kyma) if err != nil { return fmt.Errorf("failed to get SkrContext to update remote catalog: %w", err) } - moduleTemplates := c.syncerAPIFactoryFn(c.kcpClient, skrContext.Client, &c.settings) - return moduleTemplates.SyncToSKR(ctx, kyma, kcpModules) + moduleTemplates := c.moduleTemplateSyncerFactoryFn(c.kcpClient, skrContext.Client, &c.settings) + moduleReleaseMetas := c.moduleReleaseMetaSyncerFactoryFn(c.kcpClient, skrContext.Client, &c.settings) + + mtErr := moduleTemplates.SyncToSKR(ctx, kyma, kcpModules) + mrmErr := moduleReleaseMetas.SyncToSKR(ctx, kyma, kcpModuleReleaseMeta) + + return errors.Join(mtErr, mrmErr) } func (c *RemoteCatalog) Delete( @@ -84,6 +101,6 @@ func (c *RemoteCatalog) Delete( return fmt.Errorf("failed to get SkrContext for deleting RemoteCatalog: %w", err) } - moduleTemplates := c.syncerAPIFactoryFn(c.kcpClient, skrContext.Client, &c.settings) + moduleTemplates := c.moduleTemplateSyncerFactoryFn(c.kcpClient, skrContext.Client, &c.settings) return moduleTemplates.DeleteAllManaged(ctx, kyma) } From 559bd136264b3eef2e2d0c4ac640342326e0eaaa Mon Sep 17 00:00:00 2001 From: Tomasz Smelcerz Date: Mon, 4 Nov 2024 14:05:59 +0100 Subject: [PATCH 02/15] renaming --- internal/remote/remote_catalog.go | 42 +++++++++++++++---------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/internal/remote/remote_catalog.go b/internal/remote/remote_catalog.go index 0d17b68e6b..88d9ca5e61 100644 --- a/internal/remote/remote_catalog.go +++ b/internal/remote/remote_catalog.go @@ -20,29 +20,29 @@ type Settings struct { } type RemoteCatalog struct { - kcpClient client.Client - skrContextFactory SkrContextProvider - settings Settings - moduleTemplateSyncerFactoryFn moduleTemplateSyncerFactory - moduleReleaseMetaSyncerFactoryFn moduleReleaseMetaSyncerFactory + kcpClient client.Client + skrContextFactory SkrContextProvider + settings Settings + moduleTemplateSyncAPIFactoryFn moduleTemplateSyncAPIFactory + moduleReleaseMetaSyncAPIFactoryFn moduleReleaseMetaSyncAPIFactory } -// moduleTemplateSyncer encapsulates the top-level abstration for syncing module templates to a remote cluster. -type moduleTemplateSyncer interface { +// moduleTemplateSyncAPI encapsulates the top-level abstration for syncing module templates to a remote cluster. +type moduleTemplateSyncAPI interface { SyncToSKR(ctx context.Context, kyma types.NamespacedName, kcpModules []v1beta2.ModuleTemplate) error DeleteAllManaged(ctx context.Context, kyma types.NamespacedName) error } -type moduleReleaseMetaSyncer interface { +type moduleReleaseMetaSyncAPI interface { SyncToSKR(ctx context.Context, kyma types.NamespacedName, kcpModuleReleaseMeta []v1beta2.ModuleReleaseMeta) error DeleteAllManaged(ctx context.Context, kyma types.NamespacedName) error } -// moduleTemplateSyncerFactory is a function that creates moduleTemplateSyncer instances. -type moduleTemplateSyncerFactory func(kcpClient, skrClient client.Client, settings *Settings) moduleTemplateSyncer +// moduleTemplateSyncAPIFactory is a function that creates moduleTemplateSyncAPI instances. +type moduleTemplateSyncAPIFactory func(kcpClient, skrClient client.Client, settings *Settings) moduleTemplateSyncAPI -// moduleReleaseMetaSyncerFactory is a function that creates moduleReleaseMetaSyncer instances. -type moduleReleaseMetaSyncerFactory func(kcpClient, skrClient client.Client, settings *Settings) moduleReleaseMetaSyncer +// moduleReleaseMetaSyncAPIFactory is a function that creates moduleReleaseMetaSyncAPI instances. +type moduleReleaseMetaSyncAPIFactory func(kcpClient, skrClient client.Client, settings *Settings) moduleReleaseMetaSyncAPI func NewRemoteCatalogFromKyma(kcpClient client.Client, skrContextFactory SkrContextProvider, remoteSyncNamespace string, @@ -57,16 +57,16 @@ func NewRemoteCatalogFromKyma(kcpClient client.Client, skrContextFactory SkrCont } func newRemoteCatalog(kcpClient client.Client, skrContextFactory SkrContextProvider, settings Settings) *RemoteCatalog { - var syncerAPIFactoryFn moduleTemplateSyncerFactory = func(kcpClient, skrClient client.Client, settings *Settings) moduleTemplateSyncer { + var syncerAPIFactoryFn moduleTemplateSyncAPIFactory = func(kcpClient, skrClient client.Client, settings *Settings) moduleTemplateSyncAPI { return newSyncer(kcpClient, skrClient, settings) } res := &RemoteCatalog{ - kcpClient: kcpClient, - skrContextFactory: skrContextFactory, - settings: settings, - moduleTemplateSyncerFactoryFn: syncerAPIFactoryFn, - moduleReleaseMetaSyncerFactoryFn: nil, //TODO: Wire up + kcpClient: kcpClient, + skrContextFactory: skrContextFactory, + settings: settings, + moduleTemplateSyncAPIFactoryFn: syncerAPIFactoryFn, + moduleReleaseMetaSyncAPIFactoryFn: nil, //TODO: Wire up } return res @@ -83,8 +83,8 @@ func (c *RemoteCatalog) Sync( return fmt.Errorf("failed to get SkrContext to update remote catalog: %w", err) } - moduleTemplates := c.moduleTemplateSyncerFactoryFn(c.kcpClient, skrContext.Client, &c.settings) - moduleReleaseMetas := c.moduleReleaseMetaSyncerFactoryFn(c.kcpClient, skrContext.Client, &c.settings) + moduleTemplates := c.moduleTemplateSyncAPIFactoryFn(c.kcpClient, skrContext.Client, &c.settings) + moduleReleaseMetas := c.moduleReleaseMetaSyncAPIFactoryFn(c.kcpClient, skrContext.Client, &c.settings) mtErr := moduleTemplates.SyncToSKR(ctx, kyma, kcpModules) mrmErr := moduleReleaseMetas.SyncToSKR(ctx, kyma, kcpModuleReleaseMeta) @@ -101,6 +101,6 @@ func (c *RemoteCatalog) Delete( return fmt.Errorf("failed to get SkrContext for deleting RemoteCatalog: %w", err) } - moduleTemplates := c.moduleTemplateSyncerFactoryFn(c.kcpClient, skrContext.Client, &c.settings) + moduleTemplates := c.moduleTemplateSyncAPIFactoryFn(c.kcpClient, skrContext.Client, &c.settings) return moduleTemplates.DeleteAllManaged(ctx, kyma) } From 89e8728c2fe5a7665151170c5c2b1e7d3fae1496 Mon Sep 17 00:00:00 2001 From: Tomasz Smelcerz Date: Mon, 4 Nov 2024 14:12:49 +0100 Subject: [PATCH 03/15] rename --- internal/remote/moduletemplate_syncer.go | 26 ++++++++++--------- internal/remote/moduletemplate_syncer_test.go | 8 +++--- internal/remote/remote_catalog.go | 2 +- 3 files changed, 19 insertions(+), 17 deletions(-) diff --git a/internal/remote/moduletemplate_syncer.go b/internal/remote/moduletemplate_syncer.go index a6c64d23f1..7a955ea8fd 100644 --- a/internal/remote/moduletemplate_syncer.go +++ b/internal/remote/moduletemplate_syncer.go @@ -13,28 +13,30 @@ import ( "github.com/kyma-project/lifecycle-manager/pkg/util" ) -type syncWorker interface { +// moduleTemplateSyncWorker is an interface for worker synchronizing ModuleTemplates from KCP to SKR. +type moduleTemplateSyncWorker interface { SyncConcurrently(ctx context.Context, kcpModules []v1beta2.ModuleTemplate) error DeleteConcurrently(ctx context.Context, runtimeModules []v1beta2.ModuleTemplate) error } -type syncWorkerFactory func(kcpClient, skrClient client.Client, settings *Settings) syncWorker +// moduleTemplateSyncWorkerFactory is a factory function for creating new moduleTemplateSyncWorker instance. +type moduleTemplateSyncWorkerFactory func(kcpClient, skrClient client.Client, settings *Settings) moduleTemplateSyncWorker -// syncer provides a top-level API for synchronizing ModuleTemplates from KCP to SKR. +// moduleTemplateSyncer provides a top-level API for synchronizing ModuleTemplates from KCP to SKR. // It expects a ready-to-use client to the KCP and SKR cluster. -type syncer struct { +type moduleTemplateSyncer struct { kcpClient client.Client skrClient client.Client settings *Settings - syncWorkerFactoryFn syncWorkerFactory + syncWorkerFactoryFn moduleTemplateSyncWorkerFactory } -func newSyncer(kcpClient, skrClient client.Client, settings *Settings) *syncer { - var syncWokerFactoryFn syncWorkerFactory = func(kcpClient, skrClient client.Client, settings *Settings) syncWorker { +func newModuleTemplateSyncer(kcpClient, skrClient client.Client, settings *Settings) *moduleTemplateSyncer { + var syncWokerFactoryFn moduleTemplateSyncWorkerFactory = func(kcpClient, skrClient client.Client, settings *Settings) moduleTemplateSyncWorker { return newModuleTemplateConcurrentWorker(kcpClient, skrClient, settings) } - return &syncer{ + return &moduleTemplateSyncer{ kcpClient: kcpClient, skrClient: skrClient, settings: settings, @@ -42,13 +44,13 @@ func newSyncer(kcpClient, skrClient client.Client, settings *Settings) *syncer { } } -// Sync first lists all currently available moduleTemplates in the Runtime. +// SyncToSKR first lists all currently available moduleTemplates in the Runtime. // If there is a NoMatchError, it will attempt to install the CRD but only if there are available crs to copy. // It will use a 2 stage process: // 1. All ModuleTemplates that either have to be created based on the given Control Plane Templates // 2. All ModuleTemplates that have to be removed as they were deleted form the Control Plane Templates // It uses Server-Side-Apply Patches to optimize the turnaround required. -func (mts *syncer) SyncToSKR(ctx context.Context, kyma types.NamespacedName, kcpModules []v1beta2.ModuleTemplate) error { +func (mts *moduleTemplateSyncer) SyncToSKR(ctx context.Context, kyma types.NamespacedName, kcpModules []v1beta2.ModuleTemplate) error { worker := mts.syncWorkerFactoryFn(mts.kcpClient, mts.skrClient, mts.settings) if err := worker.SyncConcurrently(ctx, kcpModules); err != nil { @@ -70,8 +72,8 @@ func (mts *syncer) SyncToSKR(ctx context.Context, kyma types.NamespacedName, kcp return worker.DeleteConcurrently(ctx, collections.Dereference(diffsToDelete)) } -// DeleteFromSKR deletes all ModuleTemplates managed by KLM from the SKR cluster. -func (mts *syncer) DeleteAllManaged(ctx context.Context, kyma types.NamespacedName) error { +// DeleteAllManaged deletes all ModuleTemplates managed by KLM from the SKR cluster. +func (mts *moduleTemplateSyncer) DeleteAllManaged(ctx context.Context, kyma types.NamespacedName) error { moduleTemplatesRuntime := &v1beta2.ModuleTemplateList{Items: []v1beta2.ModuleTemplate{}} if err := mts.skrClient.List(ctx, moduleTemplatesRuntime); err != nil { // if there is no CRD or no module template exists, diff --git a/internal/remote/moduletemplate_syncer_test.go b/internal/remote/moduletemplate_syncer_test.go index b45c4d62ad..8555111630 100644 --- a/internal/remote/moduletemplate_syncer_test.go +++ b/internal/remote/moduletemplate_syncer_test.go @@ -61,7 +61,7 @@ func TestSyncer_SyncToSKR_happypath(t *testing.T) { } } - syncWokerFactoryFn := func(kcpClient, skrClient client.Client, settings *Settings) syncWorker { + syncWokerFactoryFn := func(kcpClient, skrClient client.Client, settings *Settings) moduleTemplateSyncWorker { return &fakeSyncWorker{ namespace: settings.Namespace, onSyncConcurrently: onSyncConcurrentlyFn, @@ -69,7 +69,7 @@ func TestSyncer_SyncToSKR_happypath(t *testing.T) { } } - subject := syncer{ + subject := moduleTemplateSyncer{ skrClient: skrClient, settings: getSettings(), syncWorkerFactoryFn: syncWokerFactoryFn, @@ -120,7 +120,7 @@ func TestSyncer_SyncToSKR_nilList(t *testing.T) { } } - syncWokerFactoryFn := func(kcpClient, skrClient client.Client, settings *Settings) syncWorker { + syncWokerFactoryFn := func(kcpClient, skrClient client.Client, settings *Settings) moduleTemplateSyncWorker { return &fakeSyncWorker{ namespace: settings.Namespace, onSyncConcurrently: onSyncConcurrentlyFn, @@ -128,7 +128,7 @@ func TestSyncer_SyncToSKR_nilList(t *testing.T) { } } - subject := syncer{ + subject := moduleTemplateSyncer{ skrClient: skrClient, settings: getSettings(), syncWorkerFactoryFn: syncWokerFactoryFn, diff --git a/internal/remote/remote_catalog.go b/internal/remote/remote_catalog.go index 88d9ca5e61..5fd8e922af 100644 --- a/internal/remote/remote_catalog.go +++ b/internal/remote/remote_catalog.go @@ -58,7 +58,7 @@ func NewRemoteCatalogFromKyma(kcpClient client.Client, skrContextFactory SkrCont func newRemoteCatalog(kcpClient client.Client, skrContextFactory SkrContextProvider, settings Settings) *RemoteCatalog { var syncerAPIFactoryFn moduleTemplateSyncAPIFactory = func(kcpClient, skrClient client.Client, settings *Settings) moduleTemplateSyncAPI { - return newSyncer(kcpClient, skrClient, settings) + return newModuleTemplateSyncer(kcpClient, skrClient, settings) } res := &RemoteCatalog{ From d1aca1fd85e971e3cf521f93a1c674e83b0e9691 Mon Sep 17 00:00:00 2001 From: Tomasz Smelcerz Date: Tue, 5 Nov 2024 10:29:33 +0100 Subject: [PATCH 04/15] rename --- internal/remote/crd.go | 37 ++++ .../remote/modulereleasemeta_syncworker.go | 171 ++++++++++++++++++ internal/remote/moduletemplate_syncworker.go | 29 --- 3 files changed, 208 insertions(+), 29 deletions(-) create mode 100644 internal/remote/crd.go create mode 100644 internal/remote/modulereleasemeta_syncworker.go diff --git a/internal/remote/crd.go b/internal/remote/crd.go new file mode 100644 index 0000000000..dec5275b8e --- /dev/null +++ b/internal/remote/crd.go @@ -0,0 +1,37 @@ +package remote + +import ( + "errors" + + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/apimachinery/pkg/api/meta" +) + +func crdReady(crd *apiextensionsv1.CustomResourceDefinition) bool { + for _, cond := range crd.Status.Conditions { + if cond.Type == apiextensionsv1.Established && + cond.Status == apiextensionsv1.ConditionTrue { + return true + } + + if cond.Type == apiextensionsv1.NamesAccepted && + cond.Status == apiextensionsv1.ConditionFalse { + // This indicates a naming conflict, but it's probably not the + // job of this function to fail because of that. Instead, + // we treat it as a success, since the process should be able to + // continue. + return true + } + } + return false +} + +func containsCRDNotFoundError(errs []error) bool { + for _, err := range errs { + unwrappedError := errors.Unwrap(err) + if meta.IsNoMatchError(unwrappedError) || CRDNotFoundErr(unwrappedError) { + return true + } + } + return false +} diff --git a/internal/remote/modulereleasemeta_syncworker.go b/internal/remote/modulereleasemeta_syncworker.go new file mode 100644 index 0000000000..118ca0e8cb --- /dev/null +++ b/internal/remote/modulereleasemeta_syncworker.go @@ -0,0 +1,171 @@ +package remote + +import ( + "context" + "errors" + "fmt" + + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + apimetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/kyma-project/lifecycle-manager/api/shared" + "github.com/kyma-project/lifecycle-manager/api/v1beta2" + "github.com/kyma-project/lifecycle-manager/internal/util/collections" + "github.com/kyma-project/lifecycle-manager/pkg/util" +) + +var ( + errModuleReleaseMetaCRDNotReady = errors.New("module release meta crd for catalog sync is not ready") + errModuleReleaseMetaCleanup = errors.New("failed to delete obsolete ModuleReleaseMeta from catalog") + errCatModuleReleaseMetaApply = errors.New("could not apply ModuleReleseMeta objects from catalog") +) + +// moduleReleaseMetaConcurrentWorker performs synchronization using multiple goroutines. +type moduleReleaseMetaConcurrentWorker struct { + namespace string + patchDiff func(ctx context.Context, obj *v1beta2.ModuleReleaseMeta) error + deleteDiff func(ctx context.Context, obj *v1beta2.ModuleReleaseMeta) error + createCRD func(ctx context.Context) error +} + +// newModuleReleaseMetaConcurrentWorker returns new moduleReleaseMetaConcurrentWorker instance with default dependencies. +func newModuleReleaseMetaConcurrentWorker(kcpClient, skrClient client.Client, settings *Settings) *moduleReleaseMetaConcurrentWorker { + patchDiffFn := func(ctx context.Context, obj *v1beta2.ModuleReleaseMeta) error { + return patchDiffModuleReleaseMeta(ctx, obj, skrClient, settings.SSAPatchOptions) + } + + deleteDiffFn := func(ctx context.Context, obj *v1beta2.ModuleReleaseMeta) error { + return patchDeleteModuleReleaseMeta(ctx, obj, skrClient) + } + + createCRDFn := func(ctx context.Context) error { + return createModuleReleaseMetaCRDInRuntime(ctx, kcpClient, skrClient) + } + + return &moduleReleaseMetaConcurrentWorker{ + namespace: settings.Namespace, + patchDiff: patchDiffFn, + deleteDiff: deleteDiffFn, + createCRD: createCRDFn, + } +} + +// SyncConcurrently synchronizes ModuleReleaseMetas from KCP to SKR. +// kcpModules are the ModuleReleaseMetas to be synced from the KCP cluster. +func (c *moduleReleaseMetaConcurrentWorker) SyncConcurrently(ctx context.Context, kcpModules []v1beta2.ModuleReleaseMeta) error { + channelLength := len(kcpModules) + results := make(chan error, channelLength) + for kcpIndex := range kcpModules { + go func() { + prepareForSSAModuleReleaseMeta(&kcpModules[kcpIndex], c.namespace) + results <- c.patchDiff(ctx, &kcpModules[kcpIndex]) + }() + } + var errs []error + for range channelLength { + if err := <-results; err != nil { + errs = append(errs, err) + } + } + + // retry if ModuleReleaseMeta CRD is not existing in SKR cluster + if containsCRDNotFoundError(errs) { + if err := c.createCRD(ctx); err != nil { + return err + } + } + + if len(errs) != 0 { + errs = append(errs, errCatModuleReleaseMetaApply) + return errors.Join(errs...) + } + return nil +} + +// DeleteConcurrently deletes ModuleReleaseMetas from SKR. +func (c *moduleReleaseMetaConcurrentWorker) DeleteConcurrently(ctx context.Context, + diffsToDelete []v1beta2.ModuleReleaseMeta, +) error { + channelLength := len(diffsToDelete) + results := make(chan error, channelLength) + for _, diff := range diffsToDelete { + go func() { + results <- c.deleteDiff(ctx, &diff) + }() + } + var errs []error + for range channelLength { + if err := <-results; err != nil { + errs = append(errs, err) + } + } + + if len(errs) != 0 { + errs = append(errs, errModuleReleaseMetaCleanup) + return errors.Join(errs...) + } + return nil +} + +func createModuleReleaseMetaCRDInRuntime(ctx context.Context, kcpClient client.Client, skrClient client.Client) error { + kcpCrd := &apiextensionsv1.CustomResourceDefinition{} + skrCrd := &apiextensionsv1.CustomResourceDefinition{} + objKey := client.ObjectKey{ + Name: fmt.Sprintf("%s.%s", shared.ModuleReleaseMetaKind.Plural(), v1beta2.GroupVersion.Group), + } + err := kcpClient.Get(ctx, objKey, kcpCrd) + if err != nil { + return fmt.Errorf("failed to get ModuleReleaseMeta CRD from KCP: %w", err) + } + + err = skrClient.Get(ctx, objKey, skrCrd) + + if util.IsNotFound(err) || !ContainsLatestVersion(skrCrd, v1beta2.GroupVersion.Version) { + return PatchCRD(ctx, skrClient, kcpCrd) + } + + if !crdReady(skrCrd) { + return errModuleReleaseMetaCRDNotReady + } + + if err != nil { + return fmt.Errorf("failed to get ModuleReleaseMeta CRD from SKR: %w", err) + } + + return nil +} + +func prepareForSSAModuleReleaseMeta(moduleReleaseMeta *v1beta2.ModuleReleaseMeta, namespace string) { + moduleReleaseMeta.SetResourceVersion("") + moduleReleaseMeta.SetUID("") + moduleReleaseMeta.SetManagedFields([]apimetav1.ManagedFieldsEntry{}) + moduleReleaseMeta.SetLabels(collections.MergeMaps(moduleReleaseMeta.GetLabels(), map[string]string{ + shared.ManagedBy: shared.ManagedByLabelValue, + })) + + if namespace != "" { + moduleReleaseMeta.SetNamespace(namespace) + } +} + +func patchDiffModuleReleaseMeta(ctx context.Context, diff *v1beta2.ModuleReleaseMeta, skrClient client.Client, ssaPatchOptions *client.PatchOptions) error { + err := skrClient.Patch( + ctx, diff, client.Apply, ssaPatchOptions, + ) + if err != nil { + return fmt.Errorf("could not apply ModuleReleaseMeta diff: %w", err) + } + return nil +} + +// TODO: rename to deleteModuleReleaseMeta +func patchDeleteModuleReleaseMeta( + ctx context.Context, diff *v1beta2.ModuleReleaseMeta, skrClient client.Client, +) error { + err := skrClient.Delete(ctx, diff) + if err != nil { + return fmt.Errorf("could not delete ModuleReleaseMeta: %w", err) + } + return nil +} diff --git a/internal/remote/moduletemplate_syncworker.go b/internal/remote/moduletemplate_syncworker.go index 0c298cc6e5..a164b8e940 100644 --- a/internal/remote/moduletemplate_syncworker.go +++ b/internal/remote/moduletemplate_syncworker.go @@ -150,35 +150,6 @@ func createModuleTemplateCRDInRuntime(ctx context.Context, kcpClient client.Clie return nil } -func crdReady(crd *apiextensionsv1.CustomResourceDefinition) bool { - for _, cond := range crd.Status.Conditions { - if cond.Type == apiextensionsv1.Established && - cond.Status == apiextensionsv1.ConditionTrue { - return true - } - - if cond.Type == apiextensionsv1.NamesAccepted && - cond.Status == apiextensionsv1.ConditionFalse { - // This indicates a naming conflict, but it's probably not the - // job of this function to fail because of that. Instead, - // we treat it as a success, since the process should be able to - // continue. - return true - } - } - return false -} - -func containsCRDNotFoundError(errs []error) bool { - for _, err := range errs { - unwrappedError := errors.Unwrap(err) - if meta.IsNoMatchError(unwrappedError) || CRDNotFoundErr(unwrappedError) { - return true - } - } - return false -} - func patchDiff(ctx context.Context, diff *v1beta2.ModuleTemplate, skrClient client.Client, ssaPatchOptions *client.PatchOptions) error { err := skrClient.Patch( ctx, diff, client.Apply, ssaPatchOptions, From 57f474d19bd45459efc7ddcab13e643a365d2027 Mon Sep 17 00:00:00 2001 From: Tomasz Smelcerz Date: Tue, 5 Nov 2024 10:49:04 +0100 Subject: [PATCH 05/15] rename --- .../remote/modulereleasemeta_syncworker.go | 19 +++---- internal/remote/moduletemplate_syncer_test.go | 2 +- internal/remote/moduletemplate_syncworker.go | 55 +++++++++---------- internal/remote/prepareforssa_test.go | 4 +- 4 files changed, 39 insertions(+), 41 deletions(-) diff --git a/internal/remote/modulereleasemeta_syncworker.go b/internal/remote/modulereleasemeta_syncworker.go index 118ca0e8cb..3d933f7d85 100644 --- a/internal/remote/modulereleasemeta_syncworker.go +++ b/internal/remote/modulereleasemeta_syncworker.go @@ -16,12 +16,12 @@ import ( ) var ( - errModuleReleaseMetaCRDNotReady = errors.New("module release meta crd for catalog sync is not ready") - errModuleReleaseMetaCleanup = errors.New("failed to delete obsolete ModuleReleaseMeta from catalog") - errCatModuleReleaseMetaApply = errors.New("could not apply ModuleReleseMeta objects from catalog") + errModuleReleaseMetaCRDNotReady = errors.New("catalog sync: ModuleReleaseMeta CRD is not ready") + errModuleReleaseMetaCleanup = errors.New("catalog sync: Failed to delete ModuleReleaseMeta") + errCatModuleReleaseMetaApply = errors.New("catalog sync: Could not apply ModuleReleseMetas") ) -// moduleReleaseMetaConcurrentWorker performs synchronization using multiple goroutines. +// moduleReleaseMetaConcurrentWorker performs ModuleReleaseMeta synchronization using multiple goroutines. type moduleReleaseMetaConcurrentWorker struct { namespace string patchDiff func(ctx context.Context, obj *v1beta2.ModuleReleaseMeta) error @@ -29,14 +29,14 @@ type moduleReleaseMetaConcurrentWorker struct { createCRD func(ctx context.Context) error } -// newModuleReleaseMetaConcurrentWorker returns new moduleReleaseMetaConcurrentWorker instance with default dependencies. +// newModuleReleaseMetaConcurrentWorker returns a new moduleReleaseMetaConcurrentWorker instance with default dependencies. func newModuleReleaseMetaConcurrentWorker(kcpClient, skrClient client.Client, settings *Settings) *moduleReleaseMetaConcurrentWorker { patchDiffFn := func(ctx context.Context, obj *v1beta2.ModuleReleaseMeta) error { return patchDiffModuleReleaseMeta(ctx, obj, skrClient, settings.SSAPatchOptions) } deleteDiffFn := func(ctx context.Context, obj *v1beta2.ModuleReleaseMeta) error { - return patchDeleteModuleReleaseMeta(ctx, obj, skrClient) + return deleteModuleReleaseMeta(ctx, obj, skrClient) } createCRDFn := func(ctx context.Context) error { @@ -58,7 +58,7 @@ func (c *moduleReleaseMetaConcurrentWorker) SyncConcurrently(ctx context.Context results := make(chan error, channelLength) for kcpIndex := range kcpModules { go func() { - prepareForSSAModuleReleaseMeta(&kcpModules[kcpIndex], c.namespace) + prepareModuleReleaseMetaForSSA(&kcpModules[kcpIndex], c.namespace) results <- c.patchDiff(ctx, &kcpModules[kcpIndex]) }() } @@ -136,7 +136,7 @@ func createModuleReleaseMetaCRDInRuntime(ctx context.Context, kcpClient client.C return nil } -func prepareForSSAModuleReleaseMeta(moduleReleaseMeta *v1beta2.ModuleReleaseMeta, namespace string) { +func prepareModuleReleaseMetaForSSA(moduleReleaseMeta *v1beta2.ModuleReleaseMeta, namespace string) { moduleReleaseMeta.SetResourceVersion("") moduleReleaseMeta.SetUID("") moduleReleaseMeta.SetManagedFields([]apimetav1.ManagedFieldsEntry{}) @@ -159,8 +159,7 @@ func patchDiffModuleReleaseMeta(ctx context.Context, diff *v1beta2.ModuleRelease return nil } -// TODO: rename to deleteModuleReleaseMeta -func patchDeleteModuleReleaseMeta( +func deleteModuleReleaseMeta( ctx context.Context, diff *v1beta2.ModuleReleaseMeta, skrClient client.Client, ) error { err := skrClient.Delete(ctx, diff) diff --git a/internal/remote/moduletemplate_syncer_test.go b/internal/remote/moduletemplate_syncer_test.go index 8555111630..e22b5a5727 100644 --- a/internal/remote/moduletemplate_syncer_test.go +++ b/internal/remote/moduletemplate_syncer_test.go @@ -177,7 +177,7 @@ func (f *fakeSyncWorker) SyncConcurrently(ctx context.Context, kcpModules []v1be // Simulate namespace switch on modules in kcpModules list that happens in moduleTemplateConcurrentWorker.SyncConcurrently // This is necessary for proper diff calculation later in the process. for i := range kcpModules { - prepareForSSA(&kcpModules[i], f.namespace) + prepareModuleTemplateForSSA(&kcpModules[i], f.namespace) } return nil diff --git a/internal/remote/moduletemplate_syncworker.go b/internal/remote/moduletemplate_syncworker.go index a164b8e940..52f9ca1d26 100644 --- a/internal/remote/moduletemplate_syncworker.go +++ b/internal/remote/moduletemplate_syncworker.go @@ -6,7 +6,6 @@ import ( "fmt" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" - "k8s.io/apimachinery/pkg/api/meta" apimetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -17,12 +16,12 @@ import ( ) var ( - errTemplateCRDNotReady = errors.New("module template crd for catalog sync is not ready") - errTemplateCleanup = errors.New("failed to delete obsolete catalog templates") - errCatTemplatesApply = errors.New("could not apply catalog templates") + errModuleTemplateCRDNotReady = errors.New("catalog sync: ModuleTemplate CRD is not ready") + errModuleTemplateCleanup = errors.New("catalog sync: Failed to delete obsolete ModuleTemplates") + errCatModuleTemplatesApply = errors.New("catalog sync: Could not apply ModuleTemplates") ) -// moduleTemplateConcurrentWorker performs synchronization using multiple goroutines. +// moduleTemplateConcurrentWorker performs ModuleTemplate synchronization using multiple goroutines. type moduleTemplateConcurrentWorker struct { namespace string patchDiff func(ctx context.Context, obj *v1beta2.ModuleTemplate) error @@ -33,11 +32,11 @@ type moduleTemplateConcurrentWorker struct { // newModuleTemplateConcurrentWorker returns a new moduleTemplateConcurrentWorker instance with default dependencies. func newModuleTemplateConcurrentWorker(kcpClient, skrClient client.Client, settings *Settings) *moduleTemplateConcurrentWorker { patchDiffFn := func(ctx context.Context, obj *v1beta2.ModuleTemplate) error { - return patchDiff(ctx, obj, skrClient, settings.SSAPatchOptions) + return patchDiffModuleTemplate(ctx, obj, skrClient, settings.SSAPatchOptions) } deleteDiffFn := func(ctx context.Context, obj *v1beta2.ModuleTemplate) error { - return patchDelete(ctx, obj, skrClient) + return deleteModuleTemplate(ctx, obj, skrClient) } createCRDFn := func(ctx context.Context) error { @@ -59,7 +58,7 @@ func (c *moduleTemplateConcurrentWorker) SyncConcurrently(ctx context.Context, k results := make(chan error, channelLength) for kcpIndex := range kcpModules { go func() { - prepareForSSA(&kcpModules[kcpIndex], c.namespace) + prepareModuleTemplateForSSA(&kcpModules[kcpIndex], c.namespace) results <- c.patchDiff(ctx, &kcpModules[kcpIndex]) }() } @@ -78,7 +77,7 @@ func (c *moduleTemplateConcurrentWorker) SyncConcurrently(ctx context.Context, k } if len(errs) != 0 { - errs = append(errs, errCatTemplatesApply) + errs = append(errs, errCatModuleTemplatesApply) return errors.Join(errs...) } return nil @@ -103,25 +102,12 @@ func (c *moduleTemplateConcurrentWorker) DeleteConcurrently(ctx context.Context, } if len(errs) != 0 { - errs = append(errs, errTemplateCleanup) + errs = append(errs, errModuleTemplateCleanup) return errors.Join(errs...) } return nil } -func prepareForSSA(moduleTemplate *v1beta2.ModuleTemplate, namespace string) { - moduleTemplate.SetResourceVersion("") - moduleTemplate.SetUID("") - moduleTemplate.SetManagedFields([]apimetav1.ManagedFieldsEntry{}) - moduleTemplate.SetLabels(collections.MergeMaps(moduleTemplate.GetLabels(), map[string]string{ - shared.ManagedBy: shared.ManagedByLabelValue, - })) - - if namespace != "" { - moduleTemplate.SetNamespace(namespace) - } -} - func createModuleTemplateCRDInRuntime(ctx context.Context, kcpClient client.Client, skrClient client.Client) error { kcpCrd := &apiextensionsv1.CustomResourceDefinition{} skrCrd := &apiextensionsv1.CustomResourceDefinition{} @@ -140,7 +126,7 @@ func createModuleTemplateCRDInRuntime(ctx context.Context, kcpClient client.Clie } if !crdReady(skrCrd) { - return errTemplateCRDNotReady + return errModuleTemplateCRDNotReady } if err != nil { @@ -150,22 +136,35 @@ func createModuleTemplateCRDInRuntime(ctx context.Context, kcpClient client.Clie return nil } -func patchDiff(ctx context.Context, diff *v1beta2.ModuleTemplate, skrClient client.Client, ssaPatchOptions *client.PatchOptions) error { +func prepareModuleTemplateForSSA(moduleTemplate *v1beta2.ModuleTemplate, namespace string) { + moduleTemplate.SetResourceVersion("") + moduleTemplate.SetUID("") + moduleTemplate.SetManagedFields([]apimetav1.ManagedFieldsEntry{}) + moduleTemplate.SetLabels(collections.MergeMaps(moduleTemplate.GetLabels(), map[string]string{ + shared.ManagedBy: shared.ManagedByLabelValue, + })) + + if namespace != "" { + moduleTemplate.SetNamespace(namespace) + } +} + +func patchDiffModuleTemplate(ctx context.Context, diff *v1beta2.ModuleTemplate, skrClient client.Client, ssaPatchOptions *client.PatchOptions) error { err := skrClient.Patch( ctx, diff, client.Apply, ssaPatchOptions, ) if err != nil { - return fmt.Errorf("could not apply module template diff: %w", err) + return fmt.Errorf("could not apply ModuleTemplate diff: %w", err) } return nil } -func patchDelete( +func deleteModuleTemplate( ctx context.Context, diff *v1beta2.ModuleTemplate, skrClient client.Client, ) error { err := skrClient.Delete(ctx, diff) if err != nil { - return fmt.Errorf("could not delete module template: %w", err) + return fmt.Errorf("could not delete ModuleTemplate: %w", err) } return nil } diff --git a/internal/remote/prepareforssa_test.go b/internal/remote/prepareforssa_test.go index d463c154cb..ba66d0e451 100644 --- a/internal/remote/prepareforssa_test.go +++ b/internal/remote/prepareforssa_test.go @@ -32,7 +32,7 @@ func TestPrepareForSSA(t *testing.T) { assert.Equal(t, "bar", testModule.GetLabels()["foo"]) assert.Equal(t, "default", testModule.GetNamespace()) - prepareForSSA(&testModule, "someNamespace") + prepareModuleTemplateForSSA(&testModule, "someNamespace") assert.Equal(t, "", testModule.GetResourceVersion()) assert.EqualValues(t, "", testModule.GetUID()) @@ -46,7 +46,7 @@ func TestPrepareForSSA(t *testing.T) { t.Run("ensure no other fields are modified", func(t *testing.T) { // given testModule := v1beta2.ModuleTemplate{} - prepareForSSA(&testModule, "someNamespace") + prepareModuleTemplateForSSA(&testModule, "someNamespace") afterPrepareJSON, err := json.Marshal(testModule) require.NoError(t, err) From 08010c430e8adc9e722ecb4c2702c1ff7459cc3f Mon Sep 17 00:00:00 2001 From: Tomasz Smelcerz Date: Tue, 5 Nov 2024 11:23:58 +0100 Subject: [PATCH 06/15] wire up dependencies --- internal/remote/modulereleasemeta_syncer.go | 114 ++++++++++++++++++++ internal/remote/moduletemplate_syncer.go | 16 +-- internal/remote/remote_catalog.go | 10 +- 3 files changed, 129 insertions(+), 11 deletions(-) create mode 100644 internal/remote/modulereleasemeta_syncer.go diff --git a/internal/remote/modulereleasemeta_syncer.go b/internal/remote/modulereleasemeta_syncer.go new file mode 100644 index 0000000000..70b9cebc21 --- /dev/null +++ b/internal/remote/modulereleasemeta_syncer.go @@ -0,0 +1,114 @@ +package remote + +import ( + "context" + "fmt" + + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/kyma-project/lifecycle-manager/api/v1beta2" + "github.com/kyma-project/lifecycle-manager/internal/util/collections" + "github.com/kyma-project/lifecycle-manager/pkg/util" +) + +// moduleReleaseMetaSyncWorker is an interface for worker synchronizing ModuleReleaseMetas from KCP to SKR. +type moduleReleaseMetaSyncWorker interface { + SyncConcurrently(ctx context.Context, kcpModules []v1beta2.ModuleReleaseMeta) error + DeleteConcurrently(ctx context.Context, runtimeModules []v1beta2.ModuleReleaseMeta) error +} + +// moduleReleaseMetaSyncWorkerFactory is a factory function for creating new moduleReleaseMetaSyncWorker instance. +type moduleReleaseMetaSyncWorkerFactory func(kcpClient, skrClient client.Client, settings *Settings) moduleReleaseMetaSyncWorker + +// moduleReleaseMetaSyncer provides a top-level API for synchronizing ModuleReleaseMetas from KCP to SKR. +// It expects a ready-to-use client to the KCP and SKR cluster. +type moduleReleaseMetaSyncer struct { + kcpClient client.Client + skrClient client.Client + settings *Settings + syncWorkerFactoryFn moduleReleaseMetaSyncWorkerFactory +} + +func newModuleReleaseMetaSyncer(kcpClient, skrClient client.Client, settings *Settings) *moduleReleaseMetaSyncer { + var syncWokerFactoryFn moduleReleaseMetaSyncWorkerFactory = func(kcpClient, skrClient client.Client, settings *Settings) moduleReleaseMetaSyncWorker { + return newModuleReleaseMetaConcurrentWorker(kcpClient, skrClient, settings) + } + + return &moduleReleaseMetaSyncer{ + kcpClient: kcpClient, + skrClient: skrClient, + settings: settings, + syncWorkerFactoryFn: syncWokerFactoryFn, + } +} + +// SyncToSKR first lists all currently available ModuleReleaseMetas in the Runtime. +// If there is a NoMatchError, it will attempt to install the CRD but only if there are available crs to copy. +// It will use a 2 stage process: +// 1. All ModuleReleaseMeta that have to be created based on the ModuleReleaseMetas existing in the Control Plane. +// 2. All ModuleReleaseMeta that have to be removed as they are not existing in the Control Plane. +// It uses Server-Side-Apply Patches to optimize the turnaround required. +func (mts *moduleReleaseMetaSyncer) SyncToSKR(ctx context.Context, kyma types.NamespacedName, kcpModuleReleases []v1beta2.ModuleReleaseMeta) error { + worker := mts.syncWorkerFactoryFn(mts.kcpClient, mts.skrClient, mts.settings) + + if err := worker.SyncConcurrently(ctx, kcpModuleReleases); err != nil { + return err + } + + runtimeModuleReleases := &v1beta2.ModuleReleaseMetaList{} + if err := mts.skrClient.List(ctx, runtimeModuleReleases); err != nil { + // it can happen that the ModuleReleaseMeta CRD is not caught during to apply if there are no objects to apply + // if this is the case and there is no CRD there can never be any ModuleReleaseMetas to delete + if meta.IsNoMatchError(err) { + return nil + } + return fmt.Errorf("failed to list ModuleReleaseMetas from runtime: %w", err) + } + + diffsToDelete := moduleReleaseMetasDiffFor(runtimeModuleReleases.Items).NotExistingIn(kcpModuleReleases) + diffsToDelete = collections.FilterInPlace(diffsToDelete, isModuleReleaseMetaManagedByKcp) + return worker.DeleteConcurrently(ctx, collections.Dereference(diffsToDelete)) +} + +// DeleteAllManaged deletes all ModuleReleaseMetas managed by KLM from the SKR cluster. +func (mts *moduleReleaseMetaSyncer) DeleteAllManaged(ctx context.Context, kyma types.NamespacedName) error { + moduleReleaseMetasRuntime := &v1beta2.ModuleReleaseMetaList{Items: []v1beta2.ModuleReleaseMeta{}} + if err := mts.skrClient.List(ctx, moduleReleaseMetasRuntime); err != nil { + // if there is no CRD or no ModuleReleaseMeta exists, + // there can never be any ModuleReleaseMeta to delete + if util.IsNotFound(err) { + return nil + } + return fmt.Errorf("failed to list ModuleReleaseMeta from skr: %w", err) + } + for i := range moduleReleaseMetasRuntime.Items { + if isModuleReleaseMetaManagedByKcp(&moduleReleaseMetasRuntime.Items[i]) { + if err := mts.skrClient.Delete(ctx, &moduleReleaseMetasRuntime.Items[i]); err != nil && + !util.IsNotFound(err) { + return fmt.Errorf("failed to delete ModuleReleaseMeta from skr: %w", err) + } + } + } + return nil +} + +// moduleReleaseMetasDiffFor returns a diffCalc for ModuleReleaseMeta objects. +func moduleReleaseMetasDiffFor(first []v1beta2.ModuleReleaseMeta) *collections.DiffCalc[v1beta2.ModuleReleaseMeta] { + return &collections.DiffCalc[v1beta2.ModuleReleaseMeta]{ + First: first, + Identity: func(obj v1beta2.ModuleReleaseMeta) string { + return obj.Namespace + obj.Name + }, + } +} + +func isModuleReleaseMetaManagedByKcp(skrObject *v1beta2.ModuleReleaseMeta) bool { + for _, managedFieldEntry := range skrObject.ObjectMeta.ManagedFields { + if managedFieldEntry.Manager == moduleCatalogSyncFieldManager { + return true + } + } + return false +} diff --git a/internal/remote/moduletemplate_syncer.go b/internal/remote/moduletemplate_syncer.go index 7a955ea8fd..03c48f0bde 100644 --- a/internal/remote/moduletemplate_syncer.go +++ b/internal/remote/moduletemplate_syncer.go @@ -47,8 +47,8 @@ func newModuleTemplateSyncer(kcpClient, skrClient client.Client, settings *Setti // SyncToSKR first lists all currently available moduleTemplates in the Runtime. // If there is a NoMatchError, it will attempt to install the CRD but only if there are available crs to copy. // It will use a 2 stage process: -// 1. All ModuleTemplates that either have to be created based on the given Control Plane Templates -// 2. All ModuleTemplates that have to be removed as they were deleted form the Control Plane Templates +// 1. All ModuleTemplates that have to be created based on the ModuleTemplates existing in the Control Plane. +// 2. All ModuleTemplates that have to be removed as they are not existing in the Control Plane. // It uses Server-Side-Apply Patches to optimize the turnaround required. func (mts *moduleTemplateSyncer) SyncToSKR(ctx context.Context, kyma types.NamespacedName, kcpModules []v1beta2.ModuleTemplate) error { worker := mts.syncWorkerFactoryFn(mts.kcpClient, mts.skrClient, mts.settings) @@ -64,11 +64,11 @@ func (mts *moduleTemplateSyncer) SyncToSKR(ctx context.Context, kyma types.Names if meta.IsNoMatchError(err) { return nil } - return fmt.Errorf("failed to list module templates from runtime: %w", err) + return fmt.Errorf("failed to list ModuleTemplates from runtime: %w", err) } diffsToDelete := moduleTemplatesDiffFor(runtimeModules.Items).NotExistingIn(kcpModules) - diffsToDelete = collections.FilterInPlace(diffsToDelete, isManagedByKcp) + diffsToDelete = collections.FilterInPlace(diffsToDelete, isModuleTemplateManagedByKcp) return worker.DeleteConcurrently(ctx, collections.Dereference(diffsToDelete)) } @@ -81,13 +81,13 @@ func (mts *moduleTemplateSyncer) DeleteAllManaged(ctx context.Context, kyma type if util.IsNotFound(err) { return nil } - return fmt.Errorf("failed to list module templates from skr: %w", err) + return fmt.Errorf("failed to list ModuleTemplates from skr: %w", err) } for i := range moduleTemplatesRuntime.Items { - if isManagedByKcp(&moduleTemplatesRuntime.Items[i]) { + if isModuleTemplateManagedByKcp(&moduleTemplatesRuntime.Items[i]) { if err := mts.skrClient.Delete(ctx, &moduleTemplatesRuntime.Items[i]); err != nil && !util.IsNotFound(err) { - return fmt.Errorf("failed to delete module template from skr: %w", err) + return fmt.Errorf("failed to delete ModuleTemplate from skr: %w", err) } } } @@ -104,7 +104,7 @@ func moduleTemplatesDiffFor(first []v1beta2.ModuleTemplate) *collections.DiffCal } } -func isManagedByKcp(skrTemplate *v1beta2.ModuleTemplate) bool { +func isModuleTemplateManagedByKcp(skrTemplate *v1beta2.ModuleTemplate) bool { for _, managedFieldEntry := range skrTemplate.ObjectMeta.ManagedFields { if managedFieldEntry.Manager == moduleCatalogSyncFieldManager { return true diff --git a/internal/remote/remote_catalog.go b/internal/remote/remote_catalog.go index 5fd8e922af..4cc0e2e59f 100644 --- a/internal/remote/remote_catalog.go +++ b/internal/remote/remote_catalog.go @@ -57,16 +57,20 @@ func NewRemoteCatalogFromKyma(kcpClient client.Client, skrContextFactory SkrCont } func newRemoteCatalog(kcpClient client.Client, skrContextFactory SkrContextProvider, settings Settings) *RemoteCatalog { - var syncerAPIFactoryFn moduleTemplateSyncAPIFactory = func(kcpClient, skrClient client.Client, settings *Settings) moduleTemplateSyncAPI { + var moduleTemplateSyncerAPIFactoryFn moduleTemplateSyncAPIFactory = func(kcpClient, skrClient client.Client, settings *Settings) moduleTemplateSyncAPI { return newModuleTemplateSyncer(kcpClient, skrClient, settings) } + var moduleReleaseMetaSyncerAPIFactoryFn moduleReleaseMetaSyncAPIFactory = func(kcpClient, skrClient client.Client, settings *Settings) moduleReleaseMetaSyncAPI { + return newModuleReleaseMetaSyncer(kcpClient, skrClient, settings) + } + res := &RemoteCatalog{ kcpClient: kcpClient, skrContextFactory: skrContextFactory, settings: settings, - moduleTemplateSyncAPIFactoryFn: syncerAPIFactoryFn, - moduleReleaseMetaSyncAPIFactoryFn: nil, //TODO: Wire up + moduleTemplateSyncAPIFactoryFn: moduleTemplateSyncerAPIFactoryFn, + moduleReleaseMetaSyncAPIFactoryFn: moduleReleaseMetaSyncerAPIFactoryFn, } return res From e4d66d41210cbf2d9169312fc859e99c6b3c9f17 Mon Sep 17 00:00:00 2001 From: Tomasz Smelcerz Date: Tue, 5 Nov 2024 12:28:28 +0100 Subject: [PATCH 07/15] Extend tests --- .../remote/modulereleasemeta_syncer_test.go | 181 ++++++++++++++++++ internal/remote/moduletemplate_syncer_test.go | 6 +- unit-test-coverage.yaml | 2 +- 3 files changed, 185 insertions(+), 4 deletions(-) create mode 100644 internal/remote/modulereleasemeta_syncer_test.go diff --git a/internal/remote/modulereleasemeta_syncer_test.go b/internal/remote/modulereleasemeta_syncer_test.go new file mode 100644 index 0000000000..43f35c9f07 --- /dev/null +++ b/internal/remote/modulereleasemeta_syncer_test.go @@ -0,0 +1,181 @@ +//nolint:testpackage // this file tests unexported types of the package +package remote + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + apimetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + "github.com/kyma-project/lifecycle-manager/api/v1beta2" +) + +// TestModuleReleaseMetaSyncer_SyncToSKR_happypath tests the happy path of the SyncToSKR method, +// with some ModuleReleseMetas to be installed in the SKR and some objects to be deleted from the SKR. +func TestModuleReleaseMetaSyncer_SyncToSKR_happypath(t *testing.T) { //nolint:dupl // duplication will be removed: https://github.com/kyma-project/lifecycle-manager/issues/2015 + // given + mrmKCP1 := moduleReleaseMeta("mrm1", "kcp-system") // this one should be installed in the SKR, because it's not there + mrmKCP2 := moduleReleaseMeta("mrm2", "kcp-system") + mrmKCP3 := moduleReleaseMeta("mrm3", "kcp-system") + + mrmSKR2 := moduleReleaseMeta("mrm2", "kyma-system") + mrmSKR3 := moduleReleaseMeta("mrm3", "kyma-system") + mrmSKR4 := moduleReleaseMeta("mrm4", "kyma-system") // this one should be deleted, because it's not in the KCP + + // Create a fake client with the SKR objects + scheme, err := v1beta2.SchemeBuilder.Build() + require.NoError(t, err) + skrClient := fake.NewClientBuilder(). + WithObjects(&mrmSKR2, &mrmSKR3, &mrmSKR4). + WithScheme(scheme). + Build() + + // onSyncConcurrentlyFn "pretends" to be the moduleReleaseMetaConcurrentWorker.SyncConcurrently + onSyncConcurrentlyFn := func(_ context.Context, kcpModules []v1beta2.ModuleReleaseMeta) { + if len(kcpModules) != 3 { + t.Errorf("Expected 3 kcp modules, got %d", len(kcpModules)) + } + if kcpModules[0].Name != "mrm1" { + t.Errorf("Expected module mrm1, got %s", kcpModules[0].Name) + } + if kcpModules[1].Name != "mrm2" { + t.Errorf("Expected module mrm2, got %s", kcpModules[1].Name) + } + if kcpModules[2].Name != "mrm3" { + t.Errorf("Expected module mrm3, got %s", kcpModules[2].Name) + } + } + + // onDeleteConcurrentlyFn "pretends" to be the moduleReleaseMetaConcurrentWorker.DeleteConcurrently + onDeleteConcurrentlyFn := func(_ context.Context, runtimeModules []v1beta2.ModuleReleaseMeta) { + if len(runtimeModules) != 1 { + t.Errorf("Expected 1 runtime module, got %d", len(runtimeModules)) + } + if runtimeModules[0].Name != "mrm4" { + t.Errorf("Expected module mrm4, got %s", runtimeModules[0].Name) + } + } + + syncWokerFactoryFn := func(kcpClient, skrClient client.Client, settings *Settings) moduleReleaseMetaSyncWorker { + return &fakeModuleReleaseMetaSyncWorker{ + namespace: settings.Namespace, + onSyncConcurrently: onSyncConcurrentlyFn, + onDeleteConcurrently: onDeleteConcurrentlyFn, + } + } + + subject := moduleReleaseMetaSyncer{ + skrClient: skrClient, + settings: getSettings(), + syncWorkerFactoryFn: syncWokerFactoryFn, + } + + // when + err = subject.SyncToSKR(context.Background(), types.NamespacedName{Name: "kyma", Namespace: "kcp-system"}, []v1beta2.ModuleReleaseMeta{mrmKCP1, mrmKCP2, mrmKCP3}) + + // then + assert.NoError(t, err) +} + +// TestSyncer_SyncToSKR_nilList tests the case when the list of KCP modules is nil. +func TestModuleReleaseMetaSyncer_SyncToSKR_nilList(t *testing.T) { + // given + mtSKR2 := moduleReleaseMeta("mrm2", "kyma-system") // should be deleted, because it's not in the KCP + mtSKR3 := moduleReleaseMeta("mrm3", "kyma-system") // should be deleted, because it's not in the KCP + mtSKR4 := moduleReleaseMeta("mrm4", "kyma-system") // should be deleted, because it's not in the KCP + + // Create a fake client with the SKR modules + scheme, err := v1beta2.SchemeBuilder.Build() + require.NoError(t, err) + skrClient := fake.NewClientBuilder(). + WithObjects(&mtSKR2, &mtSKR3, &mtSKR4). + WithScheme(scheme). + Build() + + // onSyncConcurrentlyFn "pretends" to be the moduleReleaseMetaConcurrentWorker.SyncConcurrently + onSyncConcurrentlyFn := func(_ context.Context, kcpModules []v1beta2.ModuleReleaseMeta) { + if kcpModules != nil { + t.Errorf("Expected nil kcp modules, got %v", kcpModules) + } + } + + // onDeleteConcurrentlyFn "pretends" to be the moduleReleaseMetaConcurrentWorker.DeleteConcurrently + onDeleteConcurrentlyFn := func(_ context.Context, runtimeModules []v1beta2.ModuleReleaseMeta) { + if len(runtimeModules) != 3 { + t.Errorf("Expected 3 runtime module, got %d", len(runtimeModules)) + } + if runtimeModules[0].Name != "mrm2" { + t.Errorf("Expected module mt2, got %s", runtimeModules[0].Name) + } + if runtimeModules[1].Name != "mrm3" { + t.Errorf("Expected module mt2, got %s", runtimeModules[1].Name) + } + if runtimeModules[2].Name != "mrm4" { + t.Errorf("Expected module mt2, got %s", runtimeModules[2].Name) + } + } + + syncWokerFactoryFn := func(kcpClient, skrClient client.Client, settings *Settings) moduleReleaseMetaSyncWorker { + return &fakeModuleReleaseMetaSyncWorker{ + namespace: settings.Namespace, + onSyncConcurrently: onSyncConcurrentlyFn, + onDeleteConcurrently: onDeleteConcurrentlyFn, + } + } + + subject := moduleReleaseMetaSyncer{ + skrClient: skrClient, + settings: getSettings(), + syncWorkerFactoryFn: syncWokerFactoryFn, + } + + // when + var nilModuleReleaseMetaList []v1beta2.ModuleReleaseMeta = nil + err = subject.SyncToSKR(context.Background(), types.NamespacedName{Name: "kyma", Namespace: "kcp-system"}, nilModuleReleaseMetaList) + + // then + assert.NoError(t, err) +} + +func moduleReleaseMeta(name, namespace string) v1beta2.ModuleReleaseMeta { + return v1beta2.ModuleReleaseMeta{ + ObjectMeta: apimetav1.ObjectMeta{ + Name: name, + Namespace: namespace, + ManagedFields: []apimetav1.ManagedFieldsEntry{ + { + Manager: moduleCatalogSyncFieldManager, + }, + }, + }, + } +} + +// Implements the syncWorker interface. +type fakeModuleReleaseMetaSyncWorker struct { + namespace string + onSyncConcurrently func(ctx context.Context, kcpModules []v1beta2.ModuleReleaseMeta) + onDeleteConcurrently func(ctx context.Context, runtimeModules []v1beta2.ModuleReleaseMeta) +} + +func (f *fakeModuleReleaseMetaSyncWorker) SyncConcurrently(ctx context.Context, kcpModules []v1beta2.ModuleReleaseMeta) error { + f.onSyncConcurrently(ctx, kcpModules) + + // Simulate namespace switch on modules in kcpModules list that happens in moduleReleaseMetaConcurrentWorker.SyncConcurrently + // This is necessary for proper diff calculation later in the process. + for i := range kcpModules { + prepareModuleReleaseMetaForSSA(&kcpModules[i], f.namespace) + } + + return nil +} + +func (f *fakeModuleReleaseMetaSyncWorker) DeleteConcurrently(ctx context.Context, runtimeModules []v1beta2.ModuleReleaseMeta) error { + f.onDeleteConcurrently(ctx, runtimeModules) + return nil +} diff --git a/internal/remote/moduletemplate_syncer_test.go b/internal/remote/moduletemplate_syncer_test.go index e22b5a5727..24b035122a 100644 --- a/internal/remote/moduletemplate_syncer_test.go +++ b/internal/remote/moduletemplate_syncer_test.go @@ -16,8 +16,8 @@ import ( ) // TestSyncer_SyncToSKR_happypath tests the happy path of the SyncToSKR method, -// with some modules to be installed in the SKR and some modules to be deleted from the SKR. -func TestSyncer_SyncToSKR_happypath(t *testing.T) { +// with some ModuleTemplates to be installed in the SKR and some modules to be deleted from the SKR. +func TestSyncer_SyncToSKR_happypath(t *testing.T) { //nolint:dupl // duplication will be removed: https://github.com/kyma-project/lifecycle-manager/issues/2015 // given mtKCP1 := moduleTemplate("mt1", "kcp-system") // this one should be installed in the SKR, because it's not there mtKCP2 := moduleTemplate("mt2", "kcp-system") @@ -107,7 +107,7 @@ func TestSyncer_SyncToSKR_nilList(t *testing.T) { // onDeleteConcurrentlyFn "pretends" to be the moduleTemplateConcurrentWorker.DeleteConcurrently onDeleteConcurrentlyFn := func(_ context.Context, runtimeModules []v1beta2.ModuleTemplate) { if len(runtimeModules) != 3 { - t.Errorf("Expected 1 runtime module, got %d", len(runtimeModules)) + t.Errorf("Expected 3 runtime module, got %d", len(runtimeModules)) } if runtimeModules[0].Name != "mt2" { t.Errorf("Expected module mt2, got %s", runtimeModules[0].Name) diff --git a/unit-test-coverage.yaml b/unit-test-coverage.yaml index bf9830e852..750022fc6b 100644 --- a/unit-test-coverage.yaml +++ b/unit-test-coverage.yaml @@ -14,6 +14,6 @@ packages: internal/manifest/modulecr: 51.1 internal/istio: 63.3 internal/pkg/resources: 91.7 - internal/remote: 11.8 + internal/remote: 13.2 internal/util/collections: 83.3 pkg/templatelookup: 77.7 From eb79b110b7e29fb56f870abc45475ec29f0f54f0 Mon Sep 17 00:00:00 2001 From: Tomasz Smelcerz Date: Wed, 6 Nov 2024 06:42:27 +0100 Subject: [PATCH 08/15] enable e2e tests --- internal/controller/kyma/controller.go | 2 +- tests/e2e/modulereleasemeta_sync_test.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/controller/kyma/controller.go b/internal/controller/kyma/controller.go index daca9f69f0..6ca6f39cc0 100644 --- a/internal/controller/kyma/controller.go +++ b/internal/controller/kyma/controller.go @@ -527,7 +527,7 @@ func (r *Reconciler) syncModuleCatalog(ctx context.Context, kyma *v1beta2.Kyma) remoteCatalog := remote.NewRemoteCatalogFromKyma(r.Client, r.SkrContextFactory, r.RemoteSyncNamespace) if err := remoteCatalog.Sync(ctx, kyma.GetNamespacedName(), modulesToSync, moduleReleaseMetaList.Items); err != nil { - return fmt.Errorf("could not synchronize remote module catalog: %w", err) + return err } return nil diff --git a/tests/e2e/modulereleasemeta_sync_test.go b/tests/e2e/modulereleasemeta_sync_test.go index 354a10e420..524bf27ade 100644 --- a/tests/e2e/modulereleasemeta_sync_test.go +++ b/tests/e2e/modulereleasemeta_sync_test.go @@ -40,7 +40,7 @@ var _ = Describe("ModuleReleaseMeta Sync", Ordered, func() { WithArguments(module.Name, ControlPlaneNamespace, v1beta2.DefaultChannel, v1Version, kcpClient). Should(Succeed()) - Skip("And the ModuleReleaseMeta exists on the SKR Cluster with the correct channel-version") + By("And the ModuleReleaseMeta exists on the SKR Cluster with the correct channel-version") Eventually(ModuleReleaseMetaExists). WithContext(ctx). WithArguments(module.Name, RemoteNamespace, skrClient). @@ -85,7 +85,7 @@ var _ = Describe("ModuleReleaseMeta Sync", Ordered, func() { WithArguments(kcpClient, module, v1beta2.DefaultChannel). Should(Succeed()) - Skip("And the Template Operator v2 ModuleTemplate exists in the SKR Cluster") + By("And the Template Operator v2 ModuleTemplate exists in the SKR Cluster") Eventually(ModuleTemplateExists). WithContext(ctx). WithArguments(skrClient, module, v1beta2.DefaultChannel). @@ -102,7 +102,7 @@ var _ = Describe("ModuleReleaseMeta Sync", Ordered, func() { WithArguments(module.Name, ControlPlaneNamespace, v1beta2.DefaultChannel, v2Version, kcpClient). Should(Succeed()) - Skip("And the ModuleReleaseMeta exists on the SKR Cluster with the correct channel-version") + By("And the ModuleReleaseMeta exists on the SKR Cluster with the correct channel-version") Eventually(ModuleReleaseMetaExists). WithContext(ctx). WithArguments(module.Name, RemoteNamespace, skrClient). @@ -126,7 +126,7 @@ var _ = Describe("ModuleReleaseMeta Sync", Ordered, func() { WithArguments(module.Name, ControlPlaneNamespace, kcpClient). Should(Equal(ErrNotFound)) - Skip("And the ModuleReleaseMeta no longer exists on the SKR Cluster") + By("And the ModuleReleaseMeta no longer exists on the SKR Cluster") Eventually(ModuleReleaseMetaExists). WithContext(ctx). WithArguments(module.Name, RemoteNamespace, skrClient). From 21fff25f1407131e9eaba672f6137224019e6f65 Mon Sep 17 00:00:00 2001 From: "Badr, Nesma" Date: Wed, 6 Nov 2024 10:51:51 +0100 Subject: [PATCH 09/15] Fix tests --- pkg/testutils/moduletemplate.go | 19 +++++++++++-------- tests/e2e/module_deletion_test.go | 4 ++-- tests/e2e/module_upgrade_new_version_test.go | 2 +- tests/e2e/modulereleasemeta_sync_test.go | 14 +++++++------- .../integration/controller/kcp/helper_test.go | 3 ++- .../controller/kcp/remote_sync_test.go | 10 +++++----- .../integration/controller/kyma/kyma_test.go | 5 +++-- .../controller/kyma/manifest_test.go | 17 ++++++++++------- 8 files changed, 41 insertions(+), 33 deletions(-) diff --git a/pkg/testutils/moduletemplate.go b/pkg/testutils/moduletemplate.go index 28e7d9a587..7f6e6ee91c 100644 --- a/pkg/testutils/moduletemplate.go +++ b/pkg/testutils/moduletemplate.go @@ -17,13 +17,14 @@ func GetModuleTemplate(ctx context.Context, clnt client.Client, module v1beta2.Module, defaultChannel string, + namespace string, ) (*v1beta2.ModuleTemplate, error) { descriptorProvider := provider.NewCachedDescriptorProvider() templateLookup := templatelookup.NewTemplateLookup(clnt, descriptorProvider) availableModule := templatelookup.AvailableModule{ Module: module, } - templateInfo := templateLookup.PopulateModuleTemplateInfo(ctx, availableModule, ControlPlaneNamespace, + templateInfo := templateLookup.PopulateModuleTemplateInfo(ctx, availableModule, namespace, defaultChannel) if templateInfo.Err != nil { @@ -36,8 +37,9 @@ func ModuleTemplateExists(ctx context.Context, clnt client.Client, module v1beta2.Module, defaultChannel string, + namespace string, ) error { - moduleTemplate, err := GetModuleTemplate(ctx, clnt, module, defaultChannel) + moduleTemplate, err := GetModuleTemplate(ctx, clnt, module, defaultChannel, namespace) if moduleTemplate == nil || errors.Is(err, templatelookup.ErrNoTemplatesInListResult) { return ErrNotFound } @@ -47,7 +49,7 @@ func ModuleTemplateExists(ctx context.Context, func AllModuleTemplatesExists(ctx context.Context, clnt client.Client, kyma *v1beta2.Kyma) error { for _, module := range kyma.Spec.Modules { - if err := ModuleTemplateExists(ctx, clnt, module, kyma.Spec.Channel); err != nil { + if err := ModuleTemplateExists(ctx, clnt, module, kyma.Spec.Channel, kyma.Namespace); err != nil { return err } } @@ -61,8 +63,9 @@ func UpdateModuleTemplateSpec(ctx context.Context, key, newValue, kymaChannel string, + namespace string, ) error { - moduleTemplate, err := GetModuleTemplate(ctx, clnt, module, kymaChannel) + moduleTemplate, err := GetModuleTemplate(ctx, clnt, module, kymaChannel, namespace) if err != nil { return err } @@ -77,9 +80,9 @@ func UpdateModuleTemplateSpec(ctx context.Context, } func DeleteModuleTemplate(ctx context.Context, - clnt client.Client, module v1beta2.Module, kymaChannel string, + clnt client.Client, module v1beta2.Module, kymaChannel string, namespace string, ) error { - moduleTemplate, err := GetModuleTemplate(ctx, clnt, module, kymaChannel) + moduleTemplate, err := GetModuleTemplate(ctx, clnt, module, kymaChannel, namespace) if util.IsNotFound(err) { return nil } @@ -92,9 +95,9 @@ func DeleteModuleTemplate(ctx context.Context, } func ReadModuleVersionFromModuleTemplate(ctx context.Context, clnt client.Client, module v1beta2.Module, - channel string, + channel string, namespace string, ) (string, error) { - moduleTemplate, err := GetModuleTemplate(ctx, clnt, module, channel) + moduleTemplate, err := GetModuleTemplate(ctx, clnt, module, channel, namespace) if err != nil { return "", fmt.Errorf("failed to fetch ModuleTemplate: %w", err) } diff --git a/tests/e2e/module_deletion_test.go b/tests/e2e/module_deletion_test.go index 1f5c69c729..c5536071aa 100644 --- a/tests/e2e/module_deletion_test.go +++ b/tests/e2e/module_deletion_test.go @@ -271,14 +271,14 @@ var _ = Describe("Non Blocking Kyma Module Deletion", Ordered, func() { It("When ModuleTemplate is removed from KCP Cluster", func() { Eventually(DeleteModuleTemplate). WithContext(ctx). - WithArguments(kcpClient, module, kyma.Spec.Channel). + WithArguments(kcpClient, module, kyma.Spec.Channel, ControlPlaneNamespace). Should(Succeed()) }) It("Then ModuleTemplate is no longer in SKR Cluster", func() { Eventually(ModuleTemplateExists). WithContext(ctx). - WithArguments(skrClient, module, kyma.Spec.Channel). + WithArguments(skrClient, module, kyma.Spec.Channel, RemoteNamespace). Should(Equal(ErrNotFound)) }) }) diff --git a/tests/e2e/module_upgrade_new_version_test.go b/tests/e2e/module_upgrade_new_version_test.go index f8100c2a4f..a3715b087d 100644 --- a/tests/e2e/module_upgrade_new_version_test.go +++ b/tests/e2e/module_upgrade_new_version_test.go @@ -84,7 +84,7 @@ var _ = Describe("Module Upgrade By New Version", Ordered, func() { By("And Kyma Module Version in Kyma Status is updated") newModuleTemplateVersion, err := ReadModuleVersionFromModuleTemplate(ctx, kcpClient, module, - kyma.Spec.Channel) + kyma.Spec.Channel, ControlPlaneNamespace) Expect(err).ToNot(HaveOccurred()) Eventually(ModuleVersionInKymaStatusIsCorrect). diff --git a/tests/e2e/modulereleasemeta_sync_test.go b/tests/e2e/modulereleasemeta_sync_test.go index 524bf27ade..888caaab62 100644 --- a/tests/e2e/modulereleasemeta_sync_test.go +++ b/tests/e2e/modulereleasemeta_sync_test.go @@ -20,13 +20,13 @@ var _ = Describe("ModuleReleaseMeta Sync", Ordered, func() { By("Then the Template Operator v1 ModuleTemplate exists in the KCP Cluster") Eventually(ModuleTemplateExists). WithContext(ctx). - WithArguments(kcpClient, module, v1beta2.DefaultChannel). + WithArguments(kcpClient, module, v1beta2.DefaultChannel, ControlPlaneNamespace). Should(Succeed()) By("And the Template Operator v1 ModuleTemplate exists in the SKR Cluster") Eventually(ModuleTemplateExists). WithContext(ctx). - WithArguments(skrClient, module, v1beta2.DefaultChannel). + WithArguments(skrClient, module, v1beta2.DefaultChannel, RemoteNamespace). Should(Succeed()) By("And the ModuleReleaseMeta exists on the KCP Cluster with the correct channel-version") @@ -55,19 +55,19 @@ var _ = Describe("ModuleReleaseMeta Sync", Ordered, func() { It("When Template Operator v1 ModuleTemplate is removed from the KCP Cluster", func() { Eventually(DeleteModuleTemplate). WithContext(ctx). - WithArguments(kcpClient, module, v1beta2.DefaultChannel). + WithArguments(kcpClient, module, v1beta2.DefaultChannel, ControlPlaneNamespace). Should(Succeed()) By("Then Template Operator v1 ModuleTemplate no longer exists on the KCP Cluster") Eventually(ModuleTemplateExists). WithContext(ctx). - WithArguments(kcpClient, module, v1beta2.DefaultChannel). + WithArguments(kcpClient, module, v1beta2.DefaultChannel, ControlPlaneNamespace). Should(Equal(ErrNotFound)) By("Then Template Operator v1 ModuleTemplate no longer exists on the SKR Cluster") Eventually(ModuleTemplateExists). WithContext(ctx). - WithArguments(skrClient, module, v1beta2.DefaultChannel). + WithArguments(skrClient, module, v1beta2.DefaultChannel, RemoteNamespace). Should(Equal(ErrNotFound)) }) @@ -82,13 +82,13 @@ var _ = Describe("ModuleReleaseMeta Sync", Ordered, func() { By("Then the Template Operator v2 ModuleTemplate exists in the KCP Cluster") Eventually(ModuleTemplateExists). WithContext(ctx). - WithArguments(kcpClient, module, v1beta2.DefaultChannel). + WithArguments(kcpClient, module, v1beta2.DefaultChannel, ControlPlaneNamespace). Should(Succeed()) By("And the Template Operator v2 ModuleTemplate exists in the SKR Cluster") Eventually(ModuleTemplateExists). WithContext(ctx). - WithArguments(skrClient, module, v1beta2.DefaultChannel). + WithArguments(skrClient, module, v1beta2.DefaultChannel, RemoteNamespace). Should(Succeed()) By("And the ModuleReleaseMeta exists on the KCP Cluster with the correct channel-version") diff --git a/tests/integration/controller/kcp/helper_test.go b/tests/integration/controller/kcp/helper_test.go index 058a369070..d085d7ebee 100644 --- a/tests/integration/controller/kcp/helper_test.go +++ b/tests/integration/controller/kcp/helper_test.go @@ -110,8 +110,9 @@ func expectModuleTemplateSpecGetReset( clnt client.Client, module v1beta2.Module, kymaChannel string, + namespace string, ) error { - moduleTemplate, err := GetModuleTemplate(ctx, clnt, module, kymaChannel) + moduleTemplate, err := GetModuleTemplate(ctx, clnt, module, kymaChannel, namespace) if err != nil { return err } diff --git a/tests/integration/controller/kcp/remote_sync_test.go b/tests/integration/controller/kcp/remote_sync_test.go index 9bdc033b7f..2aaf474718 100644 --- a/tests/integration/controller/kcp/remote_sync_test.go +++ b/tests/integration/controller/kcp/remote_sync_test.go @@ -92,11 +92,11 @@ var _ = Describe("Kyma sync into Remote Cluster", Ordered, func() { Should(Succeed()) By("ModuleTemplate exists in KCP cluster") Eventually(ModuleTemplateExists, Timeout, Interval). - WithArguments(ctx, kcpClient, moduleInKCP, kyma.Spec.Channel). + WithArguments(ctx, kcpClient, moduleInKCP, kyma.Spec.Channel, ControlPlaneNamespace). Should(Succeed()) By("ModuleTemplate exists in SKR cluster") Eventually(ModuleTemplateExists, Timeout, Interval).WithArguments(ctx, skrClient, moduleInKCP, - kyma.Spec.Channel).Should(Succeed()) + kyma.Spec.Channel, RemoteNamespace).Should(Succeed()) By("No module synced to remote Kyma") Eventually(NotContainsModuleInSpec, Timeout, Interval). @@ -106,7 +106,7 @@ var _ = Describe("Kyma sync into Remote Cluster", Ordered, func() { By("Remote Module Catalog created") Eventually(ModuleTemplateExists, Timeout, Interval). - WithArguments(ctx, skrClient, moduleInSKR, kyma.Spec.Channel). + WithArguments(ctx, skrClient, moduleInSKR, kyma.Spec.Channel, RemoteNamespace). Should(Succeed()) Eventually(containsModuleTemplateCondition, Timeout, Interval). WithArguments(skrClient, skrKyma.GetName(), flags.DefaultRemoteSyncNamespace). @@ -170,13 +170,13 @@ var _ = Describe("Kyma sync into Remote Cluster", Ordered, func() { By("Update SKR Module Template spec.data.spec field") Eventually(UpdateModuleTemplateSpec, Timeout, Interval). WithContext(ctx). - WithArguments(skrClient, moduleInSKR, InitSpecKey, "valueUpdated", kyma.Spec.Channel). + WithArguments(skrClient, moduleInSKR, InitSpecKey, "valueUpdated", kyma.Spec.Channel, RemoteNamespace). Should(Succeed()) By("Expect SKR Module Template spec.data.spec field get reset") Eventually(expectModuleTemplateSpecGetReset, 2*Timeout, Interval). WithArguments(skrClient, - moduleInSKR, kyma.Spec.Channel). + moduleInSKR, kyma.Spec.Channel, RemoteNamespace). Should(Succeed()) }) diff --git a/tests/integration/controller/kyma/kyma_test.go b/tests/integration/controller/kyma/kyma_test.go index dc757e4847..cc2862eae3 100644 --- a/tests/integration/controller/kyma/kyma_test.go +++ b/tests/integration/controller/kyma/kyma_test.go @@ -162,7 +162,8 @@ var _ = Describe("Kyma enable one Module", Ordered, func() { if len(modulesStatus) != 1 { return ErrWrongModulesStatus } - template, err := GetModuleTemplate(ctx, kcpClient, module, v1beta2.DefaultChannel) + template, err := GetModuleTemplate(ctx, kcpClient, module, v1beta2.DefaultChannel, + ControlPlaneNamespace) if err != nil { return err } @@ -515,7 +516,7 @@ func updateKCPModuleTemplateSpecData(kymaName, valueUpdated string) func() error } for _, activeModule := range createdKyma.Spec.Modules { return UpdateModuleTemplateSpec(ctx, kcpClient, - activeModule, InitSpecKey, valueUpdated, createdKyma.Spec.Channel) + activeModule, InitSpecKey, valueUpdated, createdKyma.Spec.Channel, ControlPlaneNamespace) } return nil } diff --git a/tests/integration/controller/kyma/manifest_test.go b/tests/integration/controller/kyma/manifest_test.go index 1f65e33914..9be2cd79e2 100644 --- a/tests/integration/controller/kyma/manifest_test.go +++ b/tests/integration/controller/kyma/manifest_test.go @@ -139,7 +139,7 @@ var _ = Describe("Manifest.Spec is rendered correctly", Ordered, func() { RegisterDefaultLifecycleForKyma(kyma) It("validate Manifest", func() { - moduleTemplate, err := GetModuleTemplate(ctx, kcpClient, module, kyma.Spec.Channel) + moduleTemplate, err := GetModuleTemplate(ctx, kcpClient, module, kyma.Spec.Channel, ControlPlaneNamespace) Expect(err).NotTo(HaveOccurred()) expectManifest := expectManifestFor(kyma) @@ -209,7 +209,7 @@ var _ = Describe("Manifest.Spec is reset after manual update", Ordered, func() { }) It("validate Manifest", func() { - moduleTemplate, err := GetModuleTemplate(ctx, kcpClient, module, kyma.Spec.Channel) + moduleTemplate, err := GetModuleTemplate(ctx, kcpClient, module, kyma.Spec.Channel, ControlPlaneNamespace) Expect(err).NotTo(HaveOccurred()) expectManifest := expectManifestFor(kyma) @@ -343,7 +343,8 @@ var _ = Describe("Modules can only be referenced via module name", Ordered, func Context("When operator is referenced just by the label name", func() { It("returns the expected operator", func() { - moduleTemplate, err := GetModuleTemplate(ctx, kcpClient, moduleReferencedWithLabel, kyma.Spec.Channel) + moduleTemplate, err := GetModuleTemplate(ctx, kcpClient, moduleReferencedWithLabel, kyma.Spec.Channel, + ControlPlaneNamespace) Expect(err).ToNot(HaveOccurred()) foundModuleName := moduleTemplate.Labels[shared.ModuleName] @@ -353,14 +354,16 @@ var _ = Describe("Modules can only be referenced via module name", Ordered, func Context("When operator is referenced by Namespace/Name", func() { It("cannot find the operator", func() { - _, err := GetModuleTemplate(ctx, kcpClient, moduleReferencedWithNamespacedName, kyma.Spec.Channel) + _, err := GetModuleTemplate(ctx, kcpClient, moduleReferencedWithNamespacedName, kyma.Spec.Channel, + ControlPlaneNamespace) Expect(err.Error()).Should(ContainSubstring(templatelookup.ErrNoTemplatesInListResult.Error())) }) }) Context("When operator is referenced by FQDN", func() { It("cannot find the operator", func() { - _, err := GetModuleTemplate(ctx, kcpClient, moduleReferencedWithFQDN, kyma.Spec.Channel) + _, err := GetModuleTemplate(ctx, kcpClient, moduleReferencedWithFQDN, kyma.Spec.Channel, + ControlPlaneNamespace) Expect(err.Error()).Should(ContainSubstring(templatelookup.ErrNoTemplatesInListResult.Error())) }) }) @@ -510,7 +513,7 @@ func validateManifestSpecResource(manifestResource, moduleTemplateData *unstruct // getKCPModuleTemplate is a generic ModuleTemplate validation function. func validateKCPModuleTemplate(module v1beta2.Module, kymaChannel string) func(moduleTemplateFn) error { return func(validateFunc moduleTemplateFn) error { - moduleTemplate, err := GetModuleTemplate(ctx, kcpClient, module, kymaChannel) + moduleTemplate, err := GetModuleTemplate(ctx, kcpClient, module, kymaChannel, ControlPlaneNamespace) if err != nil { return err } @@ -527,7 +530,7 @@ func validateKCPModuleTemplate(module v1beta2.Module, kymaChannel string) func(m // updateKCPModuleTemplate is a generic ModuleTemplate update function. func updateKCPModuleTemplate(module v1beta2.Module, kymaChannel string) func(moduleTemplateFn) error { return func(updateFunc moduleTemplateFn) error { - moduleTemplate, err := GetModuleTemplate(ctx, kcpClient, module, kymaChannel) + moduleTemplate, err := GetModuleTemplate(ctx, kcpClient, module, kymaChannel, ControlPlaneNamespace) if err != nil { return err } From d196dddfd7a70d5bc41fc5ac732cde8ab9f1620f Mon Sep 17 00:00:00 2001 From: Tomasz Smelcerz Date: Wed, 6 Nov 2024 14:00:58 +0100 Subject: [PATCH 10/15] review fix --- internal/remote/modulereleasemeta_syncer.go | 5 ++--- internal/remote/modulereleasemeta_syncer_test.go | 5 ++--- internal/remote/moduletemplate_syncer.go | 5 ++--- internal/remote/moduletemplate_syncer_test.go | 5 ++--- internal/remote/remote_catalog.go | 14 +++++++------- 5 files changed, 15 insertions(+), 19 deletions(-) diff --git a/internal/remote/modulereleasemeta_syncer.go b/internal/remote/modulereleasemeta_syncer.go index 70b9cebc21..fb2963fd45 100644 --- a/internal/remote/modulereleasemeta_syncer.go +++ b/internal/remote/modulereleasemeta_syncer.go @@ -5,7 +5,6 @@ import ( "fmt" "k8s.io/apimachinery/pkg/api/meta" - "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/kyma-project/lifecycle-manager/api/v1beta2" @@ -50,7 +49,7 @@ func newModuleReleaseMetaSyncer(kcpClient, skrClient client.Client, settings *Se // 1. All ModuleReleaseMeta that have to be created based on the ModuleReleaseMetas existing in the Control Plane. // 2. All ModuleReleaseMeta that have to be removed as they are not existing in the Control Plane. // It uses Server-Side-Apply Patches to optimize the turnaround required. -func (mts *moduleReleaseMetaSyncer) SyncToSKR(ctx context.Context, kyma types.NamespacedName, kcpModuleReleases []v1beta2.ModuleReleaseMeta) error { +func (mts *moduleReleaseMetaSyncer) SyncToSKR(ctx context.Context, kcpModuleReleases []v1beta2.ModuleReleaseMeta) error { worker := mts.syncWorkerFactoryFn(mts.kcpClient, mts.skrClient, mts.settings) if err := worker.SyncConcurrently(ctx, kcpModuleReleases); err != nil { @@ -73,7 +72,7 @@ func (mts *moduleReleaseMetaSyncer) SyncToSKR(ctx context.Context, kyma types.Na } // DeleteAllManaged deletes all ModuleReleaseMetas managed by KLM from the SKR cluster. -func (mts *moduleReleaseMetaSyncer) DeleteAllManaged(ctx context.Context, kyma types.NamespacedName) error { +func (mts *moduleReleaseMetaSyncer) DeleteAllManaged(ctx context.Context) error { moduleReleaseMetasRuntime := &v1beta2.ModuleReleaseMetaList{Items: []v1beta2.ModuleReleaseMeta{}} if err := mts.skrClient.List(ctx, moduleReleaseMetasRuntime); err != nil { // if there is no CRD or no ModuleReleaseMeta exists, diff --git a/internal/remote/modulereleasemeta_syncer_test.go b/internal/remote/modulereleasemeta_syncer_test.go index 43f35c9f07..a1b8e02255 100644 --- a/internal/remote/modulereleasemeta_syncer_test.go +++ b/internal/remote/modulereleasemeta_syncer_test.go @@ -8,7 +8,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" apimetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -76,7 +75,7 @@ func TestModuleReleaseMetaSyncer_SyncToSKR_happypath(t *testing.T) { //nolint:du } // when - err = subject.SyncToSKR(context.Background(), types.NamespacedName{Name: "kyma", Namespace: "kcp-system"}, []v1beta2.ModuleReleaseMeta{mrmKCP1, mrmKCP2, mrmKCP3}) + err = subject.SyncToSKR(context.Background(), []v1beta2.ModuleReleaseMeta{mrmKCP1, mrmKCP2, mrmKCP3}) // then assert.NoError(t, err) @@ -136,7 +135,7 @@ func TestModuleReleaseMetaSyncer_SyncToSKR_nilList(t *testing.T) { // when var nilModuleReleaseMetaList []v1beta2.ModuleReleaseMeta = nil - err = subject.SyncToSKR(context.Background(), types.NamespacedName{Name: "kyma", Namespace: "kcp-system"}, nilModuleReleaseMetaList) + err = subject.SyncToSKR(context.Background(), nilModuleReleaseMetaList) // then assert.NoError(t, err) diff --git a/internal/remote/moduletemplate_syncer.go b/internal/remote/moduletemplate_syncer.go index 03c48f0bde..2d30e75f59 100644 --- a/internal/remote/moduletemplate_syncer.go +++ b/internal/remote/moduletemplate_syncer.go @@ -5,7 +5,6 @@ import ( "fmt" "k8s.io/apimachinery/pkg/api/meta" - "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/kyma-project/lifecycle-manager/api/v1beta2" @@ -50,7 +49,7 @@ func newModuleTemplateSyncer(kcpClient, skrClient client.Client, settings *Setti // 1. All ModuleTemplates that have to be created based on the ModuleTemplates existing in the Control Plane. // 2. All ModuleTemplates that have to be removed as they are not existing in the Control Plane. // It uses Server-Side-Apply Patches to optimize the turnaround required. -func (mts *moduleTemplateSyncer) SyncToSKR(ctx context.Context, kyma types.NamespacedName, kcpModules []v1beta2.ModuleTemplate) error { +func (mts *moduleTemplateSyncer) SyncToSKR(ctx context.Context, kcpModules []v1beta2.ModuleTemplate) error { worker := mts.syncWorkerFactoryFn(mts.kcpClient, mts.skrClient, mts.settings) if err := worker.SyncConcurrently(ctx, kcpModules); err != nil { @@ -73,7 +72,7 @@ func (mts *moduleTemplateSyncer) SyncToSKR(ctx context.Context, kyma types.Names } // DeleteAllManaged deletes all ModuleTemplates managed by KLM from the SKR cluster. -func (mts *moduleTemplateSyncer) DeleteAllManaged(ctx context.Context, kyma types.NamespacedName) error { +func (mts *moduleTemplateSyncer) DeleteAllManaged(ctx context.Context) error { moduleTemplatesRuntime := &v1beta2.ModuleTemplateList{Items: []v1beta2.ModuleTemplate{}} if err := mts.skrClient.List(ctx, moduleTemplatesRuntime); err != nil { // if there is no CRD or no module template exists, diff --git a/internal/remote/moduletemplate_syncer_test.go b/internal/remote/moduletemplate_syncer_test.go index 24b035122a..54197499a6 100644 --- a/internal/remote/moduletemplate_syncer_test.go +++ b/internal/remote/moduletemplate_syncer_test.go @@ -8,7 +8,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" apimetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -76,7 +75,7 @@ func TestSyncer_SyncToSKR_happypath(t *testing.T) { //nolint:dupl // duplication } // when - err = subject.SyncToSKR(context.Background(), types.NamespacedName{Name: "kyma", Namespace: "kcp-system"}, []v1beta2.ModuleTemplate{mtKCP1, mtKCP2, mtKCP3}) + err = subject.SyncToSKR(context.Background(), []v1beta2.ModuleTemplate{mtKCP1, mtKCP2, mtKCP3}) // then assert.NoError(t, err) @@ -136,7 +135,7 @@ func TestSyncer_SyncToSKR_nilList(t *testing.T) { // when var nilModuleTemplateList []v1beta2.ModuleTemplate = nil - err = subject.SyncToSKR(context.Background(), types.NamespacedName{Name: "kyma", Namespace: "kcp-system"}, nilModuleTemplateList) + err = subject.SyncToSKR(context.Background(), nilModuleTemplateList) // then assert.NoError(t, err) diff --git a/internal/remote/remote_catalog.go b/internal/remote/remote_catalog.go index 4cc0e2e59f..0a76ed7bbf 100644 --- a/internal/remote/remote_catalog.go +++ b/internal/remote/remote_catalog.go @@ -29,13 +29,13 @@ type RemoteCatalog struct { // moduleTemplateSyncAPI encapsulates the top-level abstration for syncing module templates to a remote cluster. type moduleTemplateSyncAPI interface { - SyncToSKR(ctx context.Context, kyma types.NamespacedName, kcpModules []v1beta2.ModuleTemplate) error - DeleteAllManaged(ctx context.Context, kyma types.NamespacedName) error + SyncToSKR(ctx context.Context, kcpModules []v1beta2.ModuleTemplate) error + DeleteAllManaged(ctx context.Context) error } type moduleReleaseMetaSyncAPI interface { - SyncToSKR(ctx context.Context, kyma types.NamespacedName, kcpModuleReleaseMeta []v1beta2.ModuleReleaseMeta) error - DeleteAllManaged(ctx context.Context, kyma types.NamespacedName) error + SyncToSKR(ctx context.Context, kcpModuleReleaseMeta []v1beta2.ModuleReleaseMeta) error + DeleteAllManaged(ctx context.Context) error } // moduleTemplateSyncAPIFactory is a function that creates moduleTemplateSyncAPI instances. @@ -90,8 +90,8 @@ func (c *RemoteCatalog) Sync( moduleTemplates := c.moduleTemplateSyncAPIFactoryFn(c.kcpClient, skrContext.Client, &c.settings) moduleReleaseMetas := c.moduleReleaseMetaSyncAPIFactoryFn(c.kcpClient, skrContext.Client, &c.settings) - mtErr := moduleTemplates.SyncToSKR(ctx, kyma, kcpModules) - mrmErr := moduleReleaseMetas.SyncToSKR(ctx, kyma, kcpModuleReleaseMeta) + mtErr := moduleTemplates.SyncToSKR(ctx, kcpModules) + mrmErr := moduleReleaseMetas.SyncToSKR(ctx, kcpModuleReleaseMeta) return errors.Join(mtErr, mrmErr) } @@ -106,5 +106,5 @@ func (c *RemoteCatalog) Delete( } moduleTemplates := c.moduleTemplateSyncAPIFactoryFn(c.kcpClient, skrContext.Client, &c.settings) - return moduleTemplates.DeleteAllManaged(ctx, kyma) + return moduleTemplates.DeleteAllManaged(ctx) } From 210b13c48907b7e80eb93c8c84e5edf5a4360f73 Mon Sep 17 00:00:00 2001 From: Tomasz Smelcerz Date: Wed, 6 Nov 2024 14:07:13 +0100 Subject: [PATCH 11/15] review fix --- internal/remote/crd.go | 37 ---------------------------------- internal/remote/crd_upgrade.go | 30 +++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 37 deletions(-) delete mode 100644 internal/remote/crd.go diff --git a/internal/remote/crd.go b/internal/remote/crd.go deleted file mode 100644 index dec5275b8e..0000000000 --- a/internal/remote/crd.go +++ /dev/null @@ -1,37 +0,0 @@ -package remote - -import ( - "errors" - - apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" - "k8s.io/apimachinery/pkg/api/meta" -) - -func crdReady(crd *apiextensionsv1.CustomResourceDefinition) bool { - for _, cond := range crd.Status.Conditions { - if cond.Type == apiextensionsv1.Established && - cond.Status == apiextensionsv1.ConditionTrue { - return true - } - - if cond.Type == apiextensionsv1.NamesAccepted && - cond.Status == apiextensionsv1.ConditionFalse { - // This indicates a naming conflict, but it's probably not the - // job of this function to fail because of that. Instead, - // we treat it as a success, since the process should be able to - // continue. - return true - } - } - return false -} - -func containsCRDNotFoundError(errs []error) bool { - for _, err := range errs { - unwrappedError := errors.Unwrap(err) - if meta.IsNoMatchError(unwrappedError) || CRDNotFoundErr(unwrappedError) { - return true - } - } - return false -} diff --git a/internal/remote/crd_upgrade.go b/internal/remote/crd_upgrade.go index ee7ba4043f..0fff2642b2 100644 --- a/internal/remote/crd_upgrade.go +++ b/internal/remote/crd_upgrade.go @@ -9,6 +9,7 @@ import ( apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" apimetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/discovery" "sigs.k8s.io/controller-runtime/pkg/client" @@ -226,3 +227,32 @@ func cannotFoundResource(err error) bool { } return false } + +func crdReady(crd *apiextensionsv1.CustomResourceDefinition) bool { + for _, cond := range crd.Status.Conditions { + if cond.Type == apiextensionsv1.Established && + cond.Status == apiextensionsv1.ConditionTrue { + return true + } + + if cond.Type == apiextensionsv1.NamesAccepted && + cond.Status == apiextensionsv1.ConditionFalse { + // This indicates a naming conflict, but it's probably not the + // job of this function to fail because of that. Instead, + // we treat it as a success, since the process should be able to + // continue. + return true + } + } + return false +} + +func containsCRDNotFoundError(errs []error) bool { + for _, err := range errs { + unwrappedError := errors.Unwrap(err) + if meta.IsNoMatchError(unwrappedError) || CRDNotFoundErr(unwrappedError) { + return true + } + } + return false +} From ebff8db7d6907b2fa37cb7a9c873f96baf935029 Mon Sep 17 00:00:00 2001 From: Tomasz Smelcerz Date: Wed, 6 Nov 2024 15:12:12 +0100 Subject: [PATCH 12/15] review fix --- internal/remote/crd_upgrade.go | 45 +++++++++++++++++-- .../remote/modulereleasemeta_syncer_test.go | 2 - .../remote/modulereleasemeta_syncworker.go | 28 +----------- internal/remote/moduletemplate_syncer_test.go | 2 - internal/remote/moduletemplate_syncworker.go | 28 +----------- 5 files changed, 44 insertions(+), 61 deletions(-) diff --git a/internal/remote/crd_upgrade.go b/internal/remote/crd_upgrade.go index 0fff2642b2..6bf8b57aa4 100644 --- a/internal/remote/crd_upgrade.go +++ b/internal/remote/crd_upgrade.go @@ -18,6 +18,7 @@ import ( "github.com/kyma-project/lifecycle-manager/api/v1beta2" "github.com/kyma-project/lifecycle-manager/internal/crd" "github.com/kyma-project/lifecycle-manager/internal/util/collections" + "github.com/kyma-project/lifecycle-manager/pkg/util" ) type SyncCrdsUseCase struct { @@ -48,11 +49,12 @@ func (s *SyncCrdsUseCase) Execute(ctx context.Context, kyma *v1beta2.Kyma) (bool if err != nil { return false, fmt.Errorf("failed to get SKR context: %w", err) } + kymaCrdUpdated, err := s.fetchCrdsAndUpdateKymaAnnotations(ctx, skrContext.Client, kyma, shared.KymaKind.Plural()) if err != nil { err = client.IgnoreNotFound(err) if err != nil { - return false, fmt.Errorf("failed to fetch module template CRDs and update Kyma annotations: %w", err) + return false, fmt.Errorf("failed to fetch Kyma CRDs and update Kyma annotations: %w", err) } } @@ -61,11 +63,20 @@ func (s *SyncCrdsUseCase) Execute(ctx context.Context, kyma *v1beta2.Kyma) (bool if err != nil { err = client.IgnoreNotFound(err) if err != nil { - return false, fmt.Errorf("failed to fetch kyma CRDs and update Kyma annotations: %w", err) + return false, fmt.Errorf("failed to fetch ModuleTemplate CRDs and update Kyma annotations: %w", err) } } - return kymaCrdUpdated || moduleTemplateCrdUpdated, nil + moduleReleaseMetaCrdUpdated, err := s.fetchCrdsAndUpdateKymaAnnotations(ctx, skrContext.Client, kyma, + shared.ModuleReleaseMetaKind.Plural()) + if err != nil { + err = client.IgnoreNotFound(err) + if err != nil { + return false, fmt.Errorf("failed to fetch ModuleReleaseMeta CRDs and update Kyma annotations: %w", err) + } + } + + return kymaCrdUpdated || moduleTemplateCrdUpdated || moduleReleaseMetaCrdUpdated, nil } func PatchCRD(ctx context.Context, clnt client.Client, crd *apiextensionsv1.CustomResourceDefinition) error { @@ -256,3 +267,31 @@ func containsCRDNotFoundError(errs []error) bool { } return false } + +func createCRDInRuntime(ctx context.Context, crdKind shared.Kind, crdNotReadyErr error, kcpClient client.Client, skrClient client.Client) error { + kcpCrd := &apiextensionsv1.CustomResourceDefinition{} + skrCrd := &apiextensionsv1.CustomResourceDefinition{} + objKey := client.ObjectKey{ + Name: fmt.Sprintf("%s.%s", crdKind.Plural(), v1beta2.GroupVersion.Group), + } + err := kcpClient.Get(ctx, objKey, kcpCrd) + if err != nil { + return fmt.Errorf("failed to get %s CRD from KCP: %w", string(crdKind), err) + } + + err = skrClient.Get(ctx, objKey, skrCrd) + + if util.IsNotFound(err) || !ContainsLatestVersion(skrCrd, v1beta2.GroupVersion.Version) { + return PatchCRD(ctx, skrClient, kcpCrd) + } + + if !crdReady(skrCrd) { + return crdNotReadyErr + } + + if err != nil { + return fmt.Errorf("failed to get %s CRD from SKR: %w", string(crdKind), err) + } + + return nil +} diff --git a/internal/remote/modulereleasemeta_syncer_test.go b/internal/remote/modulereleasemeta_syncer_test.go index a1b8e02255..94c391ed4d 100644 --- a/internal/remote/modulereleasemeta_syncer_test.go +++ b/internal/remote/modulereleasemeta_syncer_test.go @@ -34,7 +34,6 @@ func TestModuleReleaseMetaSyncer_SyncToSKR_happypath(t *testing.T) { //nolint:du WithScheme(scheme). Build() - // onSyncConcurrentlyFn "pretends" to be the moduleReleaseMetaConcurrentWorker.SyncConcurrently onSyncConcurrentlyFn := func(_ context.Context, kcpModules []v1beta2.ModuleReleaseMeta) { if len(kcpModules) != 3 { t.Errorf("Expected 3 kcp modules, got %d", len(kcpModules)) @@ -50,7 +49,6 @@ func TestModuleReleaseMetaSyncer_SyncToSKR_happypath(t *testing.T) { //nolint:du } } - // onDeleteConcurrentlyFn "pretends" to be the moduleReleaseMetaConcurrentWorker.DeleteConcurrently onDeleteConcurrentlyFn := func(_ context.Context, runtimeModules []v1beta2.ModuleReleaseMeta) { if len(runtimeModules) != 1 { t.Errorf("Expected 1 runtime module, got %d", len(runtimeModules)) diff --git a/internal/remote/modulereleasemeta_syncworker.go b/internal/remote/modulereleasemeta_syncworker.go index 3d933f7d85..60d84b9c1f 100644 --- a/internal/remote/modulereleasemeta_syncworker.go +++ b/internal/remote/modulereleasemeta_syncworker.go @@ -5,14 +5,12 @@ import ( "errors" "fmt" - apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apimetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/kyma-project/lifecycle-manager/api/shared" "github.com/kyma-project/lifecycle-manager/api/v1beta2" "github.com/kyma-project/lifecycle-manager/internal/util/collections" - "github.com/kyma-project/lifecycle-manager/pkg/util" ) var ( @@ -109,31 +107,7 @@ func (c *moduleReleaseMetaConcurrentWorker) DeleteConcurrently(ctx context.Conte } func createModuleReleaseMetaCRDInRuntime(ctx context.Context, kcpClient client.Client, skrClient client.Client) error { - kcpCrd := &apiextensionsv1.CustomResourceDefinition{} - skrCrd := &apiextensionsv1.CustomResourceDefinition{} - objKey := client.ObjectKey{ - Name: fmt.Sprintf("%s.%s", shared.ModuleReleaseMetaKind.Plural(), v1beta2.GroupVersion.Group), - } - err := kcpClient.Get(ctx, objKey, kcpCrd) - if err != nil { - return fmt.Errorf("failed to get ModuleReleaseMeta CRD from KCP: %w", err) - } - - err = skrClient.Get(ctx, objKey, skrCrd) - - if util.IsNotFound(err) || !ContainsLatestVersion(skrCrd, v1beta2.GroupVersion.Version) { - return PatchCRD(ctx, skrClient, kcpCrd) - } - - if !crdReady(skrCrd) { - return errModuleReleaseMetaCRDNotReady - } - - if err != nil { - return fmt.Errorf("failed to get ModuleReleaseMeta CRD from SKR: %w", err) - } - - return nil + return createCRDInRuntime(ctx, shared.ModuleReleaseMetaKind, errModuleReleaseMetaCRDNotReady, kcpClient, skrClient) } func prepareModuleReleaseMetaForSSA(moduleReleaseMeta *v1beta2.ModuleReleaseMeta, namespace string) { diff --git a/internal/remote/moduletemplate_syncer_test.go b/internal/remote/moduletemplate_syncer_test.go index 54197499a6..edf02f4e04 100644 --- a/internal/remote/moduletemplate_syncer_test.go +++ b/internal/remote/moduletemplate_syncer_test.go @@ -34,7 +34,6 @@ func TestSyncer_SyncToSKR_happypath(t *testing.T) { //nolint:dupl // duplication WithScheme(scheme). Build() - // onSyncConcurrentlyFn "pretends" to be the moduleTemplateConcurrentWorker.SyncConcurrently onSyncConcurrentlyFn := func(_ context.Context, kcpModules []v1beta2.ModuleTemplate) { if len(kcpModules) != 3 { t.Errorf("Expected 3 kcp modules, got %d", len(kcpModules)) @@ -50,7 +49,6 @@ func TestSyncer_SyncToSKR_happypath(t *testing.T) { //nolint:dupl // duplication } } - // onDeleteConcurrentlyFn "pretends" to be the moduleTemplateConcurrentWorker.DeleteConcurrently onDeleteConcurrentlyFn := func(_ context.Context, runtimeModules []v1beta2.ModuleTemplate) { if len(runtimeModules) != 1 { t.Errorf("Expected 1 runtime module, got %d", len(runtimeModules)) diff --git a/internal/remote/moduletemplate_syncworker.go b/internal/remote/moduletemplate_syncworker.go index 52f9ca1d26..ac41dbc191 100644 --- a/internal/remote/moduletemplate_syncworker.go +++ b/internal/remote/moduletemplate_syncworker.go @@ -5,14 +5,12 @@ import ( "errors" "fmt" - apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apimetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/kyma-project/lifecycle-manager/api/shared" "github.com/kyma-project/lifecycle-manager/api/v1beta2" "github.com/kyma-project/lifecycle-manager/internal/util/collections" - "github.com/kyma-project/lifecycle-manager/pkg/util" ) var ( @@ -109,31 +107,7 @@ func (c *moduleTemplateConcurrentWorker) DeleteConcurrently(ctx context.Context, } func createModuleTemplateCRDInRuntime(ctx context.Context, kcpClient client.Client, skrClient client.Client) error { - kcpCrd := &apiextensionsv1.CustomResourceDefinition{} - skrCrd := &apiextensionsv1.CustomResourceDefinition{} - objKey := client.ObjectKey{ - Name: fmt.Sprintf("%s.%s", shared.ModuleTemplateKind.Plural(), v1beta2.GroupVersion.Group), - } - err := kcpClient.Get(ctx, objKey, kcpCrd) - if err != nil { - return fmt.Errorf("failed to get ModuleTemplate CRD from KCP: %w", err) - } - - err = skrClient.Get(ctx, objKey, skrCrd) - - if util.IsNotFound(err) || !ContainsLatestVersion(skrCrd, v1beta2.GroupVersion.Version) { - return PatchCRD(ctx, skrClient, kcpCrd) - } - - if !crdReady(skrCrd) { - return errModuleTemplateCRDNotReady - } - - if err != nil { - return fmt.Errorf("failed to get ModuleTemplate CRD from SKR: %w", err) - } - - return nil + return createCRDInRuntime(ctx, shared.ModuleTemplateKind, errModuleTemplateCRDNotReady, kcpClient, skrClient) } func prepareModuleTemplateForSSA(moduleTemplate *v1beta2.ModuleTemplate, namespace string) { From 9b435728450f4257d8ce0feb42cf69efc2ebd4e4 Mon Sep 17 00:00:00 2001 From: Tomasz Smelcerz Date: Thu, 7 Nov 2024 13:45:52 +0100 Subject: [PATCH 13/15] extend CRD annotations test --- internal/util/collections/filter.go | 13 ++- pkg/testutils/builder/modulereleasemeta.go | 6 + .../integration/controller/kcp/helper_test.go | 24 ++-- .../controller/kcp/remote_sync_test.go | 108 ++++++++++++++++-- 4 files changed, 130 insertions(+), 21 deletions(-) diff --git a/internal/util/collections/filter.go b/internal/util/collections/filter.go index 058bdb9401..68f959f621 100644 --- a/internal/util/collections/filter.go +++ b/internal/util/collections/filter.go @@ -1,6 +1,6 @@ package collections -// filterInPlace is a function that filters a slice by a predicate function. +// FilterInPlace modifies a slice using provided predicate function. // It returns a sub-slice of the input list that only contains the elements for which the predicate function returns true. // Warning: This function modifies the input list! func FilterInPlace[E any](list []*E, predicate func(*E) bool) []*E { @@ -14,6 +14,17 @@ func FilterInPlace[E any](list []*E, predicate func(*E) bool) []*E { return list[:last] } +// Filter returns a new slice which results from applying the provided predicate to the input slice. +func Filter[E any](input []E, predicate func(E) bool) []E { + output := []E{} + for _, val := range input { + if predicate(val) { + output = append(output, val) + } + } + return output +} + // Dereference is a function that dereferences elements of a provided slice of pointers. func Dereference[E any](list []*E) []E { res := make([]E, len(list)) diff --git a/pkg/testutils/builder/modulereleasemeta.go b/pkg/testutils/builder/modulereleasemeta.go index 6eb9e42e2f..8fd980792f 100644 --- a/pkg/testutils/builder/modulereleasemeta.go +++ b/pkg/testutils/builder/modulereleasemeta.go @@ -33,6 +33,12 @@ func (m ModuleReleaseMetaBuilder) WithName(name string) ModuleReleaseMetaBuilder return m } +func (m ModuleReleaseMetaBuilder) WithSingleModuleChannelAndVersions(channel, version string) ModuleReleaseMetaBuilder { + chanVer := v1beta2.ChannelVersionAssignment{Channel: channel, Version: version} + m.moduleReleaseMeta.Spec.Channels = append(m.moduleReleaseMeta.Spec.Channels, chanVer) + return m +} + func (m ModuleReleaseMetaBuilder) WithModuleChannelAndVersions(channelVersions []v1beta2.ChannelVersionAssignment) ModuleReleaseMetaBuilder { m.moduleReleaseMeta.Spec.Channels = append(m.moduleReleaseMeta.Spec.Channels, channelVersions...) return m diff --git a/tests/integration/controller/kcp/helper_test.go b/tests/integration/controller/kcp/helper_test.go index d085d7ebee..1431cee408 100644 --- a/tests/integration/controller/kcp/helper_test.go +++ b/tests/integration/controller/kcp/helper_test.go @@ -175,16 +175,24 @@ func containsModuleTemplateCondition(clnt client.Client, kymaName, kymaNamespace } func updateKymaCRD(clnt client.Client) (*apiextensionsv1.CustomResourceDefinition, error) { - crd, err := fetchCrd(clnt, shared.KymaKind) + return updateCRDPropertyDescription(clnt, shared.KymaKind, "channel", "test change") +} + +func updateModuleReleaseMetaCRD(clnt client.Client) (*apiextensionsv1.CustomResourceDefinition, error) { + return updateCRDPropertyDescription(clnt, shared.ModuleReleaseMetaKind, "channels", "test change") +} + +func updateCRDPropertyDescription(clnt client.Client, crdKind shared.Kind, propertyName, newValue string) (*apiextensionsv1.CustomResourceDefinition, error) { + crd, err := fetchCrd(clnt, crdKind) if err != nil { return nil, err } crd.SetManagedFields(nil) crdSpecVersions := crd.Spec.Versions - channelProperty := getCrdSpec(crd).Properties["channel"] - channelProperty.Description = "test change" - getCrdSpec(crd).Properties["channel"] = channelProperty + channelProperty := getCrdSpec(crd).Properties[propertyName] + channelProperty.Description = newValue + getCrdSpec(crd).Properties[propertyName] = channelProperty crd.Spec = apiextensionsv1.CustomResourceDefinitionSpec{ Versions: crdSpecVersions, Names: crd.Spec.Names, @@ -199,13 +207,13 @@ func updateKymaCRD(clnt client.Client) (*apiextensionsv1.CustomResourceDefinitio client.FieldOwner(shared.OperatorName)); err != nil { return nil, err } - crd, err = fetchCrd(clnt, shared.KymaKind) - kymaCrdName := fmt.Sprintf("%s.%s", shared.KymaKind.Plural(), v1beta2.GroupVersion.Group) + crd, err = fetchCrd(clnt, crdKind) + crdName := fmt.Sprintf("%s.%s", crdKind.Plural(), v1beta2.GroupVersion.Group) // Replace the cached CRD after updating the KCP CRD to validate that // the Generation values are updated correctly - if _, ok := crdCache.Get(kymaCrdName); ok { - crdCache.Add(kymaCrdName, *crd) + if _, ok := crdCache.Get(crdName); ok { + crdCache.Add(crdName, *crd) } if err != nil { return nil, err diff --git a/tests/integration/controller/kcp/remote_sync_test.go b/tests/integration/controller/kcp/remote_sync_test.go index 2aaf474718..c1eea4102e 100644 --- a/tests/integration/controller/kcp/remote_sync_test.go +++ b/tests/integration/controller/kcp/remote_sync_test.go @@ -2,7 +2,11 @@ package kcp_test import ( "errors" + "fmt" + "maps" + "slices" "strconv" + "strings" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apimetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -13,6 +17,7 @@ import ( "github.com/kyma-project/lifecycle-manager/api/v1beta2" "github.com/kyma-project/lifecycle-manager/internal/descriptor/cache" "github.com/kyma-project/lifecycle-manager/internal/pkg/flags" + "github.com/kyma-project/lifecycle-manager/internal/util/collections" "github.com/kyma-project/lifecycle-manager/pkg/testutils/builder" . "github.com/onsi/ginkgo/v2" @@ -301,7 +306,12 @@ var _ = Describe("Kyma sync default module list into Remote Cluster", Ordered, f var _ = Describe("CRDs sync to SKR and annotations updated in KCP kyma", Ordered, func() { kyma := NewTestKyma("kyma-test-crd-update") - moduleInKCP := NewTestModule("in-kcp", v1beta2.DefaultChannel) + moduleInKCP := NewTestModule("module-inkcp", v1beta2.DefaultChannel) + moduleReleaseMetaInKCP := builder.NewModuleReleaseMetaBuilder(). + WithName("modulereleasemeta-inkcp"). + WithModuleName("module-inkcp"). + WithSingleModuleChannelAndVersions(v1beta2.DefaultChannel, "0.1.0"). + Build() kyma.Spec.Modules = []v1beta2.Module{{Name: moduleInKCP.Name, Channel: moduleInKCP.Channel}} skrKyma := buildSkrKyma() var skrClient client.Client @@ -312,9 +322,15 @@ var _ = Describe("CRDs sync to SKR and annotations updated in KCP kyma", Ordered skrClient, err = testSkrContextFactory.Get(kyma.GetNamespacedName()) return err }, Timeout, Interval).Should(Succeed()) + + Eventually(CreateCR, Timeout, Interval). + WithContext(ctx). + WithArguments(kcpClient, moduleReleaseMetaInKCP).Should(Succeed()) }) - annotations := []string{ + injectedAnnotations := []string{ + "modulereleasemeta-skr-crd-generation", + "modulereleasemeta-kcp-crd-generation", "moduletemplate-skr-crd-generation", "moduletemplate-kcp-crd-generation", "kyma-skr-crd-generation", @@ -328,9 +344,15 @@ var _ = Describe("CRDs sync to SKR and annotations updated in KCP kyma", Ordered return err } - for _, annotation := range annotations { - if _, ok := kcpKyma.Annotations[annotation]; !ok { - return ErrNotContainsExpectedAnnotation + relevantKymaAnnotations := collections.Filter(slices.Collect(maps.Keys(kcpKyma.Annotations)), func(val string) bool { + return strings.HasSuffix(val, "crd-generation") + }) + if len(relevantKymaAnnotations) < len(injectedAnnotations) { + return fmt.Errorf("%w: expected: %d, actual: %d", ErrNotContainsExpectedAnnotation, len(injectedAnnotations), len(relevantKymaAnnotations)) + } + for _, expectedAnnotation := range injectedAnnotations { + if _, ok := kcpKyma.Annotations[expectedAnnotation]; !ok { + return fmt.Errorf("%w: %s is missing", ErrNotContainsExpectedAnnotation, expectedAnnotation) } } @@ -345,9 +367,9 @@ var _ = Describe("CRDs sync to SKR and annotations updated in KCP kyma", Ordered return err } - for _, annotation := range annotations { - if _, ok := skrKyma.Annotations[annotation]; ok { - return ErrContainsUnexpectedAnnotation + for _, unwantedAnnotation := range injectedAnnotations { + if _, ok := skrKyma.Annotations[unwantedAnnotation]; ok { + return fmt.Errorf("%w: %s is present but it should not", ErrContainsUnexpectedAnnotation, unwantedAnnotation) } } @@ -358,6 +380,11 @@ var _ = Describe("CRDs sync to SKR and annotations updated in KCP kyma", Ordered It("Kyma CRD should sync to SKR and annotations get updated", func() { var kcpKymaCrd *apiextensionsv1.CustomResourceDefinition var skrKymaCrd *apiextensionsv1.CustomResourceDefinition + var skrModuleTemplateCrd *apiextensionsv1.CustomResourceDefinition + var kcpModuleTemplateCrd *apiextensionsv1.CustomResourceDefinition + var skrModuleReleaseMetaCrd *apiextensionsv1.CustomResourceDefinition + var kcpModuleReleaseMetaCrd *apiextensionsv1.CustomResourceDefinition + By("Update KCP Kyma CRD") Eventually(func() string { var err error @@ -380,6 +407,42 @@ var _ = Describe("CRDs sync to SKR and annotations updated in KCP kyma", Ordered return skrKymaCrd.Spec.Versions[0].Schema }, Timeout, Interval).Should(Equal(kcpKymaCrd.Spec.Versions[0].Schema)) + By("Update ModuleReleaseMeta CRD") + Eventually(func() string { + var err error + kcpModuleReleaseMetaCrd, err = updateModuleReleaseMetaCRD(kcpClient) + if err != nil { + return "" + } + + return getCrdSpec(kcpModuleReleaseMetaCrd).Properties["channels"].Description + }, Timeout, Interval).Should(Equal("test change")) + + By("SKR ModuleReleaseMeta CRD should be updated") + Eventually(func() *apiextensionsv1.CustomResourceValidation { + var err error + skrModuleReleaseMetaCrd, err = fetchCrd(skrClient, shared.ModuleReleaseMetaKind) + if err != nil { + return nil + } + + return skrModuleReleaseMetaCrd.Spec.Versions[0].Schema + }, Timeout, Interval).Should(Equal(kcpModuleReleaseMetaCrd.Spec.Versions[0].Schema)) + + By("Read ModuleTemplate CRDs") + Eventually(func() error { + var err error + skrModuleTemplateCrd, err = fetchCrd(skrClient, shared.ModuleTemplateKind) + if err != nil { + return err + } + kcpModuleTemplateCrd, err = fetchCrd(kcpClient, shared.ModuleTemplateKind) + if err != nil { + return err + } + return nil + }, Timeout, Interval).Should(Succeed()) + By("Kyma CR generation annotations should be updated") Eventually(func() error { kcpKyma, err := GetKyma(ctx, kcpClient, kyma.GetName(), kyma.GetNamespace()) @@ -387,11 +450,23 @@ var _ = Describe("CRDs sync to SKR and annotations updated in KCP kyma", Ordered return err } - if kcpKyma.Annotations["kyma-skr-crd-generation"] != strconv.FormatInt(skrKymaCrd.Generation, 10) { - return ErrAnnotationNotUpdated + if err = assertCrdGenerationAnnotations(kcpKyma, "kyma-skr-crd-generation", skrKymaCrd); err != nil { + return err + } + if err = assertCrdGenerationAnnotations(kcpKyma, "kyma-kcp-crd-generation", kcpKymaCrd); err != nil { + return err + } + if err = assertCrdGenerationAnnotations(kcpKyma, "moduletemplate-skr-crd-generation", skrModuleTemplateCrd); err != nil { + return err } - if kcpKyma.Annotations["kyma-kcp-crd-generation"] != strconv.FormatInt(kcpKymaCrd.Generation, 10) { - return ErrAnnotationNotUpdated + if err = assertCrdGenerationAnnotations(kcpKyma, "moduletemplate-kcp-crd-generation", kcpModuleTemplateCrd); err != nil { + return err + } + if err = assertCrdGenerationAnnotations(kcpKyma, "modulereleasemeta-skr-crd-generation", skrModuleReleaseMetaCrd); err != nil { + return err + } + if err = assertCrdGenerationAnnotations(kcpKyma, "modulereleasemeta-kcp-crd-generation", kcpModuleReleaseMetaCrd); err != nil { + return err } return nil @@ -426,3 +501,12 @@ var _ = Describe("CRDs sync to SKR and annotations updated in KCP kyma", Ordered }, Timeout, Interval).WithContext(ctx).Should(Not(HaveOccurred())) }) }) + +func assertCrdGenerationAnnotations(kcpKyma *v1beta2.Kyma, annotationName string, targetCrd *apiextensionsv1.CustomResourceDefinition) error { + annotationValue := kcpKyma.Annotations[annotationName] + targetCrdGeneration := strconv.FormatInt(targetCrd.Generation, 10) + if annotationValue != targetCrdGeneration { + return fmt.Errorf("%w: expected: %s, actual: %s", ErrAnnotationNotUpdated, targetCrdGeneration, annotationValue) + } + return nil +} From b5daee40dc48f1759924a3144c81b6bad3c4b873 Mon Sep 17 00:00:00 2001 From: Tomasz Smelcerz Date: Thu, 7 Nov 2024 14:25:55 +0100 Subject: [PATCH 14/15] test coverage --- internal/util/collections/filter_test.go | 55 ++++++++++++++++++++++-- 1 file changed, 51 insertions(+), 4 deletions(-) diff --git a/internal/util/collections/filter_test.go b/internal/util/collections/filter_test.go index 4171cf0d5b..81f6b648f1 100644 --- a/internal/util/collections/filter_test.go +++ b/internal/util/collections/filter_test.go @@ -8,7 +8,7 @@ import ( "github.com/kyma-project/lifecycle-manager/internal/util/collections" ) -func TestFilter_empty(t *testing.T) { +func TestInPlaceFilter_empty(t *testing.T) { // given list := []*string{} predicate := func(*string) bool { return true } @@ -20,7 +20,7 @@ func TestFilter_empty(t *testing.T) { assert.Empty(t, actual) } -func TestFilter_predicate_always_matches(t *testing.T) { +func TestInPlaceFilter_predicate_always_matches(t *testing.T) { // given initial := listOfStringPtr("a", "b", "c") aCopy := append([]*string(nil), initial...) // we use the copy because the filterInPlace may modify the input list. @@ -34,7 +34,7 @@ func TestFilter_predicate_always_matches(t *testing.T) { assert.Equal(t, initial, actual) } -func TestFilter_predicate_never_matches(t *testing.T) { +func TestInPlaceFilter_predicate_never_matches(t *testing.T) { // given initial := listOfStringPtr("a", "b", "c") aCopy := append([]*string(nil), initial...) // we use the copy because the filterInPlace may modify the input list. @@ -47,7 +47,7 @@ func TestFilter_predicate_never_matches(t *testing.T) { assert.Empty(t, actual) } -func TestFilter_retains_order(t *testing.T) { +func TestInPlaceFilter_retains_order(t *testing.T) { // given initial := listOfStringPtr("a", "b", "c", "d", "e", "f", "g") aCopy := append([]*string(nil), initial...) // we use the copy because the filterInPlace may modify the input list. @@ -95,3 +95,50 @@ func listOfStringPtr(strings ...string) []*string { func inRange(val, lowerBound, upperBound string) bool { return lowerBound < val && val < upperBound } + +func Test_Filter(t *testing.T) { + t.Run("empty list", func(t *testing.T) { + // given + list := []string{} + predicate := func(string) bool { return true } + + // when + actual := collections.Filter(list, predicate) + + // then + assert.Empty(t, actual) + }) + t.Run("predicate always true", func(t *testing.T) { + // given + list := []string{"a", "b", "c"} + predicate := func(string) bool { return true } + + // when + actual := collections.Filter(list, predicate) + + // then + assert.Equal(t, list, actual) + }) + t.Run("predicate always false", func(t *testing.T) { + // given + list := []string{"a", "b", "c"} + predicate := func(string) bool { return false } + + // when + actual := collections.Filter(list, predicate) + + // then + assert.Empty(t, actual) + }) + t.Run("predicate retains order", func(t *testing.T) { + // given + list := []string{"a", "b", "c", "d", "e", "f", "g", "h"} + predicate := func(val string) bool { return inRange(val, "b", "g") && (val != "d") } + + // when + actual := collections.Filter(list, predicate) + + // then + assert.Equal(t, []string{"c", "e", "f"}, actual) + }) +} From 4f8d6353c3d4eb25b9eb6cb056ff15603d6e2710 Mon Sep 17 00:00:00 2001 From: Tomasz Smelcerz Date: Thu, 7 Nov 2024 14:26:58 +0100 Subject: [PATCH 15/15] test coverage --- unit-test-coverage.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unit-test-coverage.yaml b/unit-test-coverage.yaml index 750022fc6b..010ae49160 100644 --- a/unit-test-coverage.yaml +++ b/unit-test-coverage.yaml @@ -15,5 +15,5 @@ packages: internal/istio: 63.3 internal/pkg/resources: 91.7 internal/remote: 13.2 - internal/util/collections: 83.3 + internal/util/collections: 86 pkg/templatelookup: 77.7