From 05c7ed82b0f86c453ca556fc87b9743b3277669e Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 4 Jul 2024 15:11:55 +0200 Subject: [PATCH 01/14] chore(dependabot): bump golang from 1.22.4-alpine to 1.22.5-alpine (#1664) Bumps golang from 1.22.4-alpine to 1.22.5-alpine. --- updated-dependencies: - dependency-name: golang dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index 539c8545c7..5618b0aaa5 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,5 +1,5 @@ # Build the manager binary -FROM golang:1.22.4-alpine as builder +FROM golang:1.22.5-alpine as builder WORKDIR /lifecycle-manager # Copy the Go Modules manifests From dfd484b2153c3e330420dc08e4da504480921fb5 Mon Sep 17 00:00:00 2001 From: Benjamin Lindner <50365642+lindnerby@users.noreply.github.com> Date: Fri, 5 Jul 2024 14:31:56 +0200 Subject: [PATCH 02/14] chore: Bump k8s version for e2e to 1.29.6 (#1665) * chore: Bump k8s version for e2e to 1.29.6 * add wait for main build and bump smoke test version --- .github/actions/get-configuration/action.yaml | 2 +- .github/workflows/test-e2e.yaml | 2 +- .github/workflows/test-smoke.yml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/actions/get-configuration/action.yaml b/.github/actions/get-configuration/action.yaml index ebdd642278..2d6002744f 100644 --- a/.github/actions/get-configuration/action.yaml +++ b/.github/actions/get-configuration/action.yaml @@ -26,7 +26,7 @@ runs: id: define-variables shell: bash run: | - echo "k8s_version=${{ github.event.inputs.k8s_version || '1.28.7' }}" >> $GITHUB_OUTPUT + echo "k8s_version=${{ github.event.inputs.k8s_version || '1.29.6' }}" >> $GITHUB_OUTPUT echo "istio_version=1.20.3" >> $GITHUB_OUTPUT echo "k3d_version=5.6.0" >> $GITHUB_OUTPUT echo "cert_manager_version=1.15.0" >> $GITHUB_OUTPUT diff --git a/.github/workflows/test-e2e.yaml b/.github/workflows/test-e2e.yaml index eec21a94e4..d2618526b2 100644 --- a/.github/workflows/test-e2e.yaml +++ b/.github/workflows/test-e2e.yaml @@ -20,7 +20,7 @@ jobs: - uses: ./.github/actions/wait-for-image-build with: token: ${{ secrets.GITHUB_TOKEN }} - statusName: pull-lifecycle-mgr-build + statusName: ${{ (github.event_name == 'pull_request') && 'pull-lifecycle-mgr-build' || 'main-lifecycle-mgr-build' }} e2e-integration: name: E2E needs: wait-for-image-build diff --git a/.github/workflows/test-smoke.yml b/.github/workflows/test-smoke.yml index a734a8031d..cb08239ccb 100644 --- a/.github/workflows/test-smoke.yml +++ b/.github/workflows/test-smoke.yml @@ -50,7 +50,7 @@ jobs: env: LIFECYCLE_MANAGER: ${{ github.repository }} K3D_VERSION: v5.6.0 - K8S_VERSION: v1.28.7 + K8S_VERSION: v1.29.6 KUSTOMIZE_VERSION: 5.3.0 ISTIO_VERSION: 1.20.3 GOSUMDB: off From 136476d2ad0b74df2d3fbaece9a46e9548762405 Mon Sep 17 00:00:00 2001 From: Raj <54686422+LeelaChacha@users.noreply.github.com> Date: Mon, 8 Jul 2024 10:22:42 +0200 Subject: [PATCH 03/14] feat: Avoid Redundant SSA for Manifest Patching (#1620) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: avoid redundant ssa for manifest patching * refactor: linting issue * test: add unit test * fix: integration tests * refactor: unwrapped error * fix: state flickering * chore: add linter exception * chore: remove linter exception * fix: null pointer ref in case of mandatory module * chore: Add helpful comment Co-authored-by: Christoph Schwägerl * feat: add additional diff check in NeedToUpdate() * test: diff check in unit test * refactor: lint * refactor: remove manifest diff check * fix: module template integration test * test: add unit test * Revert "test: add unit test" This reverts commit a5a910283deda0ce8273b0129802ddece38f2094. * Revert "fix: module template integration test" This reverts commit 9ed7e26180c1e51461312b45a413ad77fd66846f. * fix integration test * chore: retrigger * refactor: gofunmpt * docs: Apply suggestions from code review Co-authored-by: Małgorzata Świeca --------- Co-authored-by: Christoph Schwägerl Co-authored-by: Christoph Schwägerl Co-authored-by: Małgorzata Świeca --- ...ta2_moduletemplate_kcp-module_updated.yaml | 56 +++++++++++++ docs/technical-reference/api/manifest-cr.md | 11 +++ pkg/module/sync/runner.go | 45 +++++++--- pkg/module/sync/runner_test.go | 82 +++++++++++++++++++ pkg/testutils/builder/moduletemplate.go | 5 ++ .../controller/kyma/manifest_test.go | 50 +++++------ 6 files changed, 206 insertions(+), 43 deletions(-) create mode 100644 config/samples/component-integration-installed/operator_v1beta2_moduletemplate_kcp-module_updated.yaml diff --git a/config/samples/component-integration-installed/operator_v1beta2_moduletemplate_kcp-module_updated.yaml b/config/samples/component-integration-installed/operator_v1beta2_moduletemplate_kcp-module_updated.yaml new file mode 100644 index 0000000000..653e7001b5 --- /dev/null +++ b/config/samples/component-integration-installed/operator_v1beta2_moduletemplate_kcp-module_updated.yaml @@ -0,0 +1,56 @@ +apiVersion: operator.kyma-project.io/v1beta2 +kind: ModuleTemplate +metadata: + name: moduletemplate-template-operator + namespace: default + labels: + "operator.kyma-project.io/module-name": "template-operator" +spec: + channel: regular + data: + apiVersion: operator.kyma-project.io/v1alpha1 + kind: Sample + metadata: + name: sample-yaml + spec: + initKey: valueUpdated + resourceFilePath: "./module-data/yaml" + descriptor: + component: + componentReferences: [] + name: kyma-project.io/template-operator + provider: internal + repositoryContexts: + - baseUrl: registry.docker.io/kyma-project/sap-kyma-jellyfish-dev/template-operator + componentNameMapping: urlPath + type: ociRegistry + resources: + - access: + digest: sha256:db86408caca4c94250d8291aa79655b84146f9cc45e0da49f05a52b3722d74a0 + type: localOciBlob + name: config + relation: local + type: yaml + version: v1.7.2 + - access: + digest: sha256:1735cfa45bf07b63427c8e11717278f8847e352a66af7633611db902386d18ed + type: localOciBlob + name: raw-manifest + relation: local + type: yaml + version: v1.7.2 + sources: + - access: + commit: 4e4b9d47cb655ca23e5c706462485ff7605e8d71 + repoUrl: github.com/kyma-project/template-operator + type: gitHub + labels: + - name: git.kyma-project.io/ref + value: refs/heads/main + version: v1 + name: module-sources + type: git + version: v1.7.2 + version: v1.7.2 + meta: + schemaVersion: v2 diff --git a/docs/technical-reference/api/manifest-cr.md b/docs/technical-reference/api/manifest-cr.md index e6ee00ef8c..9ba16a0428 100644 --- a/docs/technical-reference/api/manifest-cr.md +++ b/docs/technical-reference/api/manifest-cr.md @@ -2,6 +2,17 @@ The [Manifest custom resource (CR)](../../../api/v1beta2/manifest_types.go) is our internal representation of what results from the resolution of a ModuleTemplate CR in the context of a single cluster represented by a Kyma CR. Thus, a lot of configuration elements are similar or entirely equivalent to the data layer found in a ModuleTemplate CR. +## Patching + +The [Runner](../../../pkg/module/sync/runner.go) is responsible for creating and updating Manifest CRs using Server Side Apply (SSA). An update is only performed when one of the following conditions is met: + +1. The Manifest CR version differs from the Kyma CR's module status version. +2. The Manifest CR channel differs from the Kyma CR's module status channel. +3. The Manifest CR state differs from the Kyma CR's module status state. + +>[!NOTE] +>The module status is not present in the Kyma CR for mandatory modules, hence their Manifest CR is updated using SSA in every reconcile loop. + ## Configuration ### **.spec.remote** diff --git a/pkg/module/sync/runner.go b/pkg/module/sync/runner.go index 4d80a2bbfe..3bfe8f070c 100644 --- a/pkg/module/sync/runner.go +++ b/pkg/module/sync/runner.go @@ -64,7 +64,7 @@ func (r *Runner) ReconcileManifests(ctx context.Context, kyma *v1beta2.Kyma, results <- nil return } - if err := r.updateManifests(ctx, kyma, module); err != nil { + if err := r.updateManifest(ctx, kyma, module); err != nil { results <- fmt.Errorf("could not update module %s: %w", module.GetName(), err) return } @@ -95,7 +95,7 @@ func (r *Runner) getModule(ctx context.Context, module client.Object) error { return nil } -func (r *Runner) updateManifests(ctx context.Context, kyma *v1beta2.Kyma, +func (r *Runner) updateManifest(ctx context.Context, kyma *v1beta2.Kyma, module *common.Module, ) error { if err := r.setupModule(module, kyma); err != nil { @@ -110,8 +110,9 @@ func (r *Runner) updateManifests(ctx context.Context, kyma *v1beta2.Kyma, return commonerrs.ErrTypeAssert } + moduleStatus := kyma.GetModuleStatusMap()[module.ModuleName] if err := r.doUpdateWithStrategy(ctx, kyma.Labels[shared.ManagedBy], module.Enabled, - manifestObj); err != nil { + manifestObj, moduleStatus); err != nil { return err } module.Manifest = manifestObj @@ -119,8 +120,24 @@ func (r *Runner) updateManifests(ctx context.Context, kyma *v1beta2.Kyma, } func (r *Runner) doUpdateWithStrategy(ctx context.Context, owner string, isEnabledModule bool, - manifestObj *v1beta2.Manifest, + manifestObj *v1beta2.Manifest, kymaModuleStatus *v1beta2.ModuleStatus, ) error { + manifestInCluster := &v1beta2.Manifest{} + if err := r.Get(ctx, client.ObjectKey{ + Namespace: manifestObj.GetNamespace(), + Name: manifestObj.GetName(), + }, manifestInCluster); err != nil { + if !util.IsNotFound(err) { + return fmt.Errorf("error get manifest %s: %w", client.ObjectKeyFromObject(manifestObj), err) + } + manifestInCluster = nil + } + + if !NeedToUpdate(manifestInCluster, manifestObj, kymaModuleStatus) { + // Point to the current state from the cluster for the outside sync of the manifest + *manifestObj = *manifestInCluster + return nil + } if isEnabledModule { return r.patchManifest(ctx, owner, manifestObj) } @@ -144,14 +161,12 @@ func (r *Runner) patchManifest(ctx context.Context, owner string, manifestObj *v func (r *Runner) updateAvailableManifestSpec(ctx context.Context, manifestObj *v1beta2.Manifest) error { manifestInCluster := &v1beta2.Manifest{} - - if err := r.Get(ctx, client.ObjectKey{Namespace: manifestObj.GetNamespace(), Name: manifestObj.GetName()}, - manifestInCluster); err != nil { + if err := r.Get(ctx, client.ObjectKey{ + Namespace: manifestObj.GetNamespace(), + Name: manifestObj.GetName(), + }, manifestInCluster); err != nil { return fmt.Errorf("error get manifest %s: %w", client.ObjectKeyFromObject(manifestObj), err) } - if !needToUpdate(manifestInCluster, manifestObj) { - return nil - } manifestInCluster.Spec = manifestObj.Spec if err := r.Update(ctx, manifestInCluster); err != nil { return fmt.Errorf("error update manifest %s: %w", client.ObjectKeyFromObject(manifestObj), err) @@ -159,8 +174,14 @@ func (r *Runner) updateAvailableManifestSpec(ctx context.Context, manifestObj *v return nil } -func needToUpdate(manifestInCluster, manifestObj *v1beta2.Manifest) bool { - return manifestInCluster.Spec.Version != manifestObj.Spec.Version +func NeedToUpdate(manifestInCluster, manifestObj *v1beta2.Manifest, moduleStatus *v1beta2.ModuleStatus) bool { + if manifestInCluster == nil || moduleStatus == nil { // moduleStatus is nil in case of mandatory module + return true + } + + return manifestObj.Spec.Version != moduleStatus.Version || + manifestObj.Labels[shared.ChannelLabel] != moduleStatus.Channel || + moduleStatus.State != manifestInCluster.Status.State } func (r *Runner) deleteManifest(ctx context.Context, module *common.Module) error { diff --git a/pkg/module/sync/runner_test.go b/pkg/module/sync/runner_test.go index 2a442e2517..e5754b6680 100644 --- a/pkg/module/sync/runner_test.go +++ b/pkg/module/sync/runner_test.go @@ -9,9 +9,11 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" apierrors "k8s.io/apimachinery/pkg/api/errors" + apimetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "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/pkg/module/sync" "github.com/kyma-project/lifecycle-manager/pkg/testutils" @@ -186,3 +188,83 @@ func configureModuleInKyma( kyma.Status.Modules = append(kyma.Status.Modules, module) } } + +func TestNeedToUpdate(t *testing.T) { + type args struct { + manifestInCluster *v1beta2.Manifest + manifestObj *v1beta2.Manifest + moduleStatus *v1beta2.ModuleStatus + } + tests := []struct { + name string + args args + want bool + }{ + { + "When manifest in cluster is nil, expect need to update", + args{nil, &v1beta2.Manifest{}, &v1beta2.ModuleStatus{}}, + true, + }, + { + "When new module version available, expect need to update", + args{ + &v1beta2.Manifest{}, + &v1beta2.Manifest{ + ObjectMeta: apimetav1.ObjectMeta{ + Labels: map[string]string{shared.ChannelLabel: "regular"}, + }, + Spec: v1beta2.ManifestSpec{Version: "0.2"}, + }, &v1beta2.ModuleStatus{Version: "0.1", Channel: "regular"}, + }, + true, + }, + { + "When channel switch, expect need to update", + args{ + &v1beta2.Manifest{}, + &v1beta2.Manifest{ + ObjectMeta: apimetav1.ObjectMeta{ + Labels: map[string]string{shared.ChannelLabel: "fast"}, + }, + Spec: v1beta2.ManifestSpec{Version: "0.1"}, + }, &v1beta2.ModuleStatus{Version: "0.1", Channel: "regular"}, + }, + true, + }, + { + "When cluster Manifest in divergent state, expect need to update", + args{ + &v1beta2.Manifest{Status: shared.Status{ + State: "Warning", + }}, + &v1beta2.Manifest{}, + &v1beta2.ModuleStatus{State: "Ready"}, + }, + true, + }, + { + "When no update required, expect no update", + args{ + &v1beta2.Manifest{ + Status: shared.Status{ + State: "Ready", + }, + Spec: v1beta2.ManifestSpec{Version: "0.1"}, + }, + &v1beta2.Manifest{ + ObjectMeta: apimetav1.ObjectMeta{ + Labels: map[string]string{shared.ChannelLabel: "regular"}, + }, + Spec: v1beta2.ManifestSpec{Version: "0.1"}, + }, + &v1beta2.ModuleStatus{State: "Ready", Version: "0.1", Channel: "regular"}, + }, + false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equalf(t, tt.want, sync.NeedToUpdate(tt.args.manifestInCluster, tt.args.manifestObj, tt.args.moduleStatus), "needToUpdate(%v, %v, %v)", tt.args.manifestInCluster, tt.args.manifestObj, tt.args.moduleStatus) + }) + } +} diff --git a/pkg/testutils/builder/moduletemplate.go b/pkg/testutils/builder/moduletemplate.go index a73f32a6de..f7904e3248 100644 --- a/pkg/testutils/builder/moduletemplate.go +++ b/pkg/testutils/builder/moduletemplate.go @@ -143,6 +143,11 @@ func ComponentDescriptorFactoryFromSchema(schemaVersion compdesc.SchemaVersion) return moduleTemplate.Spec.Descriptor } +func ReadComponentDescriptorFromFile(template string, moduleTemplate *v1beta2.ModuleTemplate) { + // needs to be encapsulated in an outside call to make the runtime.Caller(1) find the proper path + readComponentDescriptorFromYaml(template, moduleTemplate) +} + func readComponentDescriptorFromYaml(template string, moduleTemplate *v1beta2.ModuleTemplate) { _, filename, _, ok := runtime.Caller(1) if !ok { diff --git a/tests/integration/controller/kyma/manifest_test.go b/tests/integration/controller/kyma/manifest_test.go index 9e26973420..01621d4d07 100644 --- a/tests/integration/controller/kyma/manifest_test.go +++ b/tests/integration/controller/kyma/manifest_test.go @@ -16,6 +16,7 @@ import ( "github.com/open-component-model/ocm/pkg/contexts/ocm/repositories/genericocireg/componentmapping" "github.com/open-component-model/ocm/pkg/runtime" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "sigs.k8s.io/controller-runtime/pkg/client" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -25,6 +26,7 @@ import ( "github.com/kyma-project/lifecycle-manager/internal/pkg/flags" . "github.com/kyma-project/lifecycle-manager/pkg/testutils" + "github.com/kyma-project/lifecycle-manager/pkg/testutils/builder" ) const ( @@ -91,41 +93,26 @@ var _ = Describe("Update Manifest CR", Ordered, func() { WithArguments(kyma.GetName(), kyma.GetNamespace(), kcpClient, shared.StateReady). Should(Succeed()) - By("Update Module Template spec.data.spec field") - valueUpdated := "valueUpdated" - Eventually(updateKCPModuleTemplateSpecData(kyma.Name, valueUpdated), Timeout, Interval).Should(Succeed()) + By("Update Module Template spec") + var moduleTemplateFromFile v1beta2.ModuleTemplate + builder.ReadComponentDescriptorFromFile("operator_v1beta2_moduletemplate_kcp-module_updated.yaml", &moduleTemplateFromFile) - By("CR updated with new value in spec.resource.spec") - Eventually(expectManifestSpecDataEquals(kyma.Name, valueUpdated), Timeout, Interval).Should(Succeed()) - - By("Update Module Template spec.descriptor.component values") - { - newComponentDescriptorRepositoryURL := func(moduleTemplate *v1beta2.ModuleTemplate) error { - descriptor, err := descriptorProvider.GetDescriptor(moduleTemplate) - if err != nil { - return err - } - - repositoryContext := descriptor.GetEffectiveRepositoryContext().Object - _, ok := repositoryContext["baseUrl"].(string) - if !ok { - Fail("Can't find \"baseUrl\" property in ModuleTemplate spec") - } - repositoryContext["baseUrl"] = updateRepositoryURL + moduleTemplateInCluster := &v1beta2.ModuleTemplate{} + err := kcpClient.Get(ctx, client.ObjectKey{ + Name: createModuleTemplateName(module), + Namespace: kyma.GetNamespace(), + }, moduleTemplateInCluster) + Expect(err).ToNot(HaveOccurred()) - newDescriptorRaw, err := compdesc.Encode(descriptor.ComponentDescriptor, compdesc.DefaultJSONCodec) - Expect(err).ToNot(HaveOccurred()) - moduleTemplate.Spec.Descriptor.Raw = newDescriptorRaw + moduleTemplateInCluster.Spec = moduleTemplateFromFile.Spec - return nil - } + Eventually(kcpClient.Update, Timeout, Interval). + WithContext(ctx). + WithArguments(moduleTemplateInCluster). + Should(Succeed()) - updateKCPModuleTemplateWith := updateKCPModuleTemplate(module, kyma.Spec.Channel) - update := func() error { - return updateKCPModuleTemplateWith(newComponentDescriptorRepositoryURL) - } - Eventually(update, Timeout, Interval).Should(Succeed()) - } + By("CR updated with new value in spec.resource.spec") + Eventually(expectManifestSpecDataEquals(kyma.Name, "valueUpdated"), Timeout, Interval).Should(Succeed()) By("Manifest is updated with new value in spec.install.source") { @@ -209,6 +196,7 @@ var _ = Describe("Manifest.Spec is reset after manual update", Ordered, func() { manifestImageSpec := extractInstallImageSpec(manifest.Spec.Install) manifestImageSpec.Repo = updateRepositoryURL + manifest.Spec.Version = "v1.7.0" // required to allow for SSA of manifest // is there a simpler way to update manifest.Spec.Install? updatedBytes, err := json.Marshal(manifestImageSpec) From 2a4b1a7fd625d637de7411203761dbfc4abbc9cb Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 10 Jul 2024 10:40:28 +0200 Subject: [PATCH 04/14] chore(dependabot): bump github.com/google/go-containerregistry from 0.19.2 to 0.20.0 (#1670) chore(dependabot): bump github.com/google/go-containerregistry Bumps [github.com/google/go-containerregistry](https://github.com/google/go-containerregistry) from 0.19.2 to 0.20.0. - [Release notes](https://github.com/google/go-containerregistry/releases) - [Changelog](https://github.com/google/go-containerregistry/blob/main/.goreleaser.yml) - [Commits](https://github.com/google/go-containerregistry/compare/v0.19.2...v0.20.0) --- updated-dependencies: - dependency-name: github.com/google/go-containerregistry dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index f79218ef37..0f9ef3213e 100644 --- a/go.mod +++ b/go.mod @@ -10,7 +10,7 @@ require ( github.com/go-logr/logr v1.4.2 github.com/go-logr/zapr v1.3.0 github.com/golang/mock v1.6.0 - github.com/google/go-containerregistry v0.19.2 + github.com/google/go-containerregistry v0.20.0 github.com/google/go-containerregistry/pkg/authn/kubernetes v0.0.0-20231202142526-55ffb0092afd github.com/jellydator/ttlcache/v3 v3.2.0 github.com/kyma-project/lifecycle-manager/api v0.0.0-00010101000000-000000000000 diff --git a/go.sum b/go.sum index c138bd0acd..2dc937011a 100644 --- a/go.sum +++ b/go.sum @@ -467,8 +467,8 @@ github.com/google/go-cmp v0.5.8/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeN github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= -github.com/google/go-containerregistry v0.19.2 h1:TannFKE1QSajsP6hPWb5oJNgKe1IKjHukIKDUmvsV6w= -github.com/google/go-containerregistry v0.19.2/go.mod h1:YCMFNQeeXeLF+dnhhWkqDItx/JSkH01j1Kis4PsjzFI= +github.com/google/go-containerregistry v0.20.0 h1:wRqHpOeVh3DnenOrPy9xDOLdnLatiGuuNRVelR2gSbg= +github.com/google/go-containerregistry v0.20.0/go.mod h1:YCMFNQeeXeLF+dnhhWkqDItx/JSkH01j1Kis4PsjzFI= github.com/google/go-containerregistry/pkg/authn/kubernetes v0.0.0-20231202142526-55ffb0092afd h1:RkbnRtHTdBpYmp0Simm3fDUTYNVbmX4aVwdgflHLfdg= github.com/google/go-containerregistry/pkg/authn/kubernetes v0.0.0-20231202142526-55ffb0092afd/go.mod h1:5sSbf/SbGGvjWIlMlt2bkEqOq+ufOIBYrBevLuxbfSs= github.com/google/go-github/v45 v45.2.0 h1:5oRLszbrkvxDDqBCNj2hjDZMKmvexaZ1xw/FCD+K3FI= From 9ea69369803228773af6fdd224e677487fdf16f2 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 10 Jul 2024 14:38:28 +0200 Subject: [PATCH 05/14] chore(dependabot): bump google.golang.org/grpc from 1.64.0 to 1.64.1 in the go_modules group (#1671) chore(dependabot): bump google.golang.org/grpc in the go_modules group Bumps the go_modules group with 1 update: [google.golang.org/grpc](https://github.com/grpc/grpc-go). Updates `google.golang.org/grpc` from 1.64.0 to 1.64.1 - [Release notes](https://github.com/grpc/grpc-go/releases) - [Commits](https://github.com/grpc/grpc-go/compare/v1.64.0...v1.64.1) --- updated-dependencies: - dependency-name: google.golang.org/grpc dependency-type: indirect dependency-group: go_modules ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 0f9ef3213e..6bd336fd4c 100644 --- a/go.mod +++ b/go.mod @@ -310,7 +310,7 @@ require ( google.golang.org/api v0.181.0 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20240515191416-fc5f0ca64291 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20240515191416-fc5f0ca64291 // indirect - google.golang.org/grpc v1.64.0 // indirect + google.golang.org/grpc v1.64.1 // indirect google.golang.org/protobuf v1.34.1 // indirect gopkg.in/evanphx/json-patch.v4 v4.12.0 // indirect gopkg.in/go-jose/go-jose.v2 v2.6.3 // indirect diff --git a/go.sum b/go.sum index 2dc937011a..b153808eea 100644 --- a/go.sum +++ b/go.sum @@ -1157,8 +1157,8 @@ google.golang.org/grpc v1.25.1/go.mod h1:c3i+UQWmh7LiEpx4sFZnkU36qjEYZ0imhYfXVyQ google.golang.org/grpc v1.27.0/go.mod h1:qbnxyOmOxrQa7FizSgH+ReBfzJrCY1pSN7KXBS8abTk= google.golang.org/grpc v1.31.0/go.mod h1:N36X2cJ7JwdamYAgDz+s+rVMFjt3numwzf/HckM8pak= google.golang.org/grpc v1.33.2/go.mod h1:JMHMWHQWaTccqQQlmk3MJZS+GWXOdAesneDmEnv2fbc= -google.golang.org/grpc v1.64.0 h1:KH3VH9y/MgNQg1dE7b3XfVK0GsPSIzJwdF617gUSbvY= -google.golang.org/grpc v1.64.0/go.mod h1:oxjF8E3FBnjp+/gVFYdWacaLDx9na1aqy9oovLpxQYg= +google.golang.org/grpc v1.64.1 h1:LKtvyfbX3UGVPFcGqJ9ItpVWW6oN/2XqTxfAnwRRXiA= +google.golang.org/grpc v1.64.1/go.mod h1:hiQF4LFZelK2WKaP6W0L92zGHtiQdZxk8CrSdvyjeP0= google.golang.org/protobuf v0.0.0-20200109180630-ec00e32a8dfd/go.mod h1:DFci5gLYBciE7Vtevhsrf46CRTquxDuWsQurQQe4oz8= google.golang.org/protobuf v0.0.0-20200221191635-4d8936d0db64/go.mod h1:kwYJMbMJ01Woi6D6+Kah6886xMZcty6N08ah7+eCXa0= google.golang.org/protobuf v0.0.0-20200228230310-ab0ca4ff8a60/go.mod h1:cfTl7dwQJ+fmap5saPgwCLgHXTUD7jkjRqWcaiX5VyM= From 13621dd301b9e1f0bf8cdc1b53acf9bbb41cf59d Mon Sep 17 00:00:00 2001 From: Benjamin Lindner <50365642+lindnerby@users.noreply.github.com> Date: Thu, 11 Jul 2024 10:48:28 +0200 Subject: [PATCH 06/14] chore: Remove diff manifest diff checker (#1674) * chore: Remove diff manifest diff checker * retrigger jobs --- .github/workflows/check-manifest-diffs.yaml | 99 --------------------- 1 file changed, 99 deletions(-) delete mode 100644 .github/workflows/check-manifest-diffs.yaml diff --git a/.github/workflows/check-manifest-diffs.yaml b/.github/workflows/check-manifest-diffs.yaml deleted file mode 100644 index a491a12d2b..0000000000 --- a/.github/workflows/check-manifest-diffs.yaml +++ /dev/null @@ -1,99 +0,0 @@ -name: "Check for diff in manifests" - -env: - PR_CACHE_KEY: pr-manifests-${{ github.run_id }}-${{ github.run_attempt }} - MAIN_CACHE_KEY: main-manifests-${{ github.run_id }}-${{ github.run_attempt }} - -on: - pull_request: - branches: - - main - - feat/** - types: - - "opened" - - "synchronize" - - "reopened" - - "labeled" - - "unlabeled" - -jobs: - create-pr-manifests: - if: ${{ contains(github.event.pull_request.labels.*.name, 'confirm/helm-update') == false }} - name: Create PR manifests - runs-on: ubuntu-latest - steps: - - name: Checkout lifecycle-manager - uses: actions/checkout@v4 - - - name: Run 'make dry-run-control-plane' - id: make-pr-manifests - run: | - make dry-run-control-plane - mkdir -p ./cache/pr - mv ./dry-run/manifests.yaml ./cache/pr/manifests.yaml - - - name: Save PR manifests in cache - id: cache-pr-manifests - uses: actions/cache/save@v4 - with: - path: ./cache/pr/ - key: ${{ env.PR_CACHE_KEY }} - - create-main-manifests: - if: ${{ contains(github.event.pull_request.labels.*.name, 'confirm/helm-update') == false }} - name: Create 'main' manifests - runs-on: ubuntu-latest - steps: - - name: Checkout lifecycle-manager - uses: actions/checkout@v4 - with: - ref: main - - - name: Run 'make dry-run-control-plane' - id: make-main-manifests - run: | - make dry-run-control-plane - mkdir -p ./cache/main - sudo mv ./dry-run/manifests.yaml ./cache/main/manifests.yaml - - - name: Save 'main' manifests in cache - id: cache-main-manifests - uses: actions/cache/save@v4 - with: - path: ./cache/main/ - key: ${{ env.MAIN_CACHE_KEY }} - - diff-manifests: - needs: - - create-pr-manifests - - create-main-manifests - name: Diff manifests - runs-on: ubuntu-latest - steps: - - name: Restore PR manifests cache - uses: actions/cache/restore@v4 - id: restore-pr-cache - with: - path: ./cache/pr/ - key: ${{ env.PR_CACHE_KEY }} - - - name: Restore 'main' manifests cache - uses: actions/cache/restore@v4 - id: restore-main-cache - with: - path: ./cache/main/ - key: ${{ env.MAIN_CACHE_KEY }} - - - name: Diff - run: | - set +e - SCRIPT_OUTPUT=$(diff ./cache/pr/manifests.yaml ./cache/main/manifests.yaml) - SCRIPT_EXIT_CODE=$? - if [[ $SCRIPT_EXIT_CODE != 0 ]]; then - echo "Detected diff in manifests. Make sure to update Helm charts accordingly and add the'confirm/helm-update' label to the PR when okay." - echo "$SCRIPT_OUTPUT" - exit $SCRIPT_EXIT_CODE - fi - set -e - - echo "No diff in manifests, all good." From 46d1e4b685fc1bfccf0240ceb19e38ba858c64e7 Mon Sep 17 00:00:00 2001 From: Benjamin Lindner <50365642+lindnerby@users.noreply.github.com> Date: Thu, 11 Jul 2024 14:34:01 +0200 Subject: [PATCH 07/14] refactor: Simplify declarative reconciler (#1676) * extract default finalizer from opts * extract field owner and namespace * extract skip label funx * replace obj with manifest for manifest reconcile loop * add skip reconcile check as method. internalize cr deletion check * simplify SpecResolver * fix integration test setup * linting * linting * remove generace cache key --- api/v1beta2/manifest_types.go | 6 + internal/controller/manifest/controller.go | 8 +- internal/declarative/v2/inmemory_rendered.go | 3 +- .../declarative/v2/moduleCR_deletion_check.go | 22 -- internal/declarative/v2/options.go | 106 ----- internal/declarative/v2/reconciler.go | 366 +++++++++--------- internal/declarative/v2/spec.go | 37 +- internal/manifest/cr_deletion_check.go | 55 --- internal/manifest/custom_resource.go | 10 +- internal/manifest/spec_resolver.go | 81 +--- .../custom_resource_check/suite_test.go | 7 +- .../controller/manifest/suite_test.go | 7 +- 12 files changed, 228 insertions(+), 480 deletions(-) delete mode 100644 internal/declarative/v2/moduleCR_deletion_check.go delete mode 100644 internal/manifest/cr_deletion_check.go diff --git a/api/v1beta2/manifest_types.go b/api/v1beta2/manifest_types.go index ac52970a71..1c34d53bb8 100644 --- a/api/v1beta2/manifest_types.go +++ b/api/v1beta2/manifest_types.go @@ -17,6 +17,8 @@ limitations under the License. package v1beta2 import ( + "strconv" + apimetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" machineryruntime "k8s.io/apimachinery/pkg/runtime" @@ -130,3 +132,7 @@ type ManifestList struct { func init() { SchemeBuilder.Register(&Manifest{}, &ManifestList{}) } + +func (manifest *Manifest) SkipReconciliation() bool { + return manifest.GetLabels() != nil && manifest.GetLabels()[shared.SkipReconcileLabel] == strconv.FormatBool(true) +} diff --git a/internal/controller/manifest/controller.go b/internal/controller/manifest/controller.go index be6af3b9be..3fe47c12ae 100644 --- a/internal/controller/manifest/controller.go +++ b/internal/controller/manifest/controller.go @@ -3,7 +3,6 @@ package manifest import ( "sigs.k8s.io/controller-runtime/pkg/manager" - "github.com/kyma-project/lifecycle-manager/api/v1beta2" declarativev2 "github.com/kyma-project/lifecycle-manager/internal/declarative/v2" "github.com/kyma-project/lifecycle-manager/internal/manifest" "github.com/kyma-project/lifecycle-manager/internal/pkg/metrics" @@ -22,15 +21,12 @@ func NewReconciler(mgr manager.Manager, extractor := manifest.NewPathExtractor(nil) lookup := &manifest.RemoteClusterLookup{KCP: kcp} return declarativev2.NewFromManager( - mgr, &v1beta2.Manifest{}, requeueIntervals, manifestMetrics, mandatoryModulesMetrics, - declarativev2.WithSpecResolver( - manifest.NewSpecResolver(kcp, extractor), - ), + mgr, requeueIntervals, manifestMetrics, mandatoryModulesMetrics, + manifest.NewSpecResolver(kcp.Client, extractor), declarativev2.WithCustomReadyCheck(manifest.NewDeploymentReadyCheck()), declarativev2.WithRemoteTargetCluster(lookup.ConfigResolver), manifest.WithClientCacheKey(), declarativev2.WithPostRun{manifest.PostRunCreateCR}, declarativev2.WithPreDelete{manifest.PreDeleteDeleteCR}, - declarativev2.WithModuleCRDeletionCheck(manifest.NewModuleCRDeletionCheck()), ) } diff --git a/internal/declarative/v2/inmemory_rendered.go b/internal/declarative/v2/inmemory_rendered.go index 07fe19708a..1abcd8af98 100644 --- a/internal/declarative/v2/inmemory_rendered.go +++ b/internal/declarative/v2/inmemory_rendered.go @@ -62,6 +62,5 @@ func (c *InMemoryManifestCache) Parse(spec *Spec, } func generateCacheKey(spec *Spec) string { - file := filepath.Join(ManifestFilePrefix, spec.Path, spec.ManifestName) - return fmt.Sprintf("%s-%s", file, spec.Mode) + return filepath.Join(ManifestFilePrefix, spec.Path, spec.ManifestName) } diff --git a/internal/declarative/v2/moduleCR_deletion_check.go b/internal/declarative/v2/moduleCR_deletion_check.go deleted file mode 100644 index e5fdc63d96..0000000000 --- a/internal/declarative/v2/moduleCR_deletion_check.go +++ /dev/null @@ -1,22 +0,0 @@ -package v2 - -import ( - "context" - - "sigs.k8s.io/controller-runtime/pkg/client" -) - -type ModuleCRDeletionCheck interface { - Run(ctx context.Context, clnt client.Client, obj Object) (bool, error) -} - -// NewDefaultDeletionCheck creates a check that verifies that the Resource CR in the remote cluster is deleted. -func NewDefaultDeletionCheck() *DefaultDeletionCheck { - return &DefaultDeletionCheck{} -} - -type DefaultDeletionCheck struct{} - -func (c *DefaultDeletionCheck) Run(ctx context.Context, clnt client.Client, obj Object) (bool, error) { - return true, nil -} diff --git a/internal/declarative/v2/options.go b/internal/declarative/v2/options.go index 59f8fe1fef..ef3a160b8d 100644 --- a/internal/declarative/v2/options.go +++ b/internal/declarative/v2/options.go @@ -3,34 +3,23 @@ package v2 import ( "context" "os" - "strconv" "time" - apimetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" k8slabels "k8s.io/apimachinery/pkg/labels" "k8s.io/client-go/rest" "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/client" - logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/manager" - - "github.com/kyma-project/lifecycle-manager/api/shared" - "github.com/kyma-project/lifecycle-manager/internal" ) const ( - FinalizerDefault = "declarative.kyma-project.io/finalizer" - FieldOwnerDefault = "declarative.kyma-project.io/applier" EventRecorderDefault = "declarative.kyma-project.io/events" DefaultInMemoryParseTTL = 24 * time.Hour ) func DefaultOptions() *Options { return (&Options{}).Apply( - WithNamespace(apimetav1.NamespaceDefault, false), - WithFinalizer(FinalizerDefault), - WithFieldOwner(FieldOwnerDefault), WithPostRenderTransform( ManagedByDeclarativeV2, watchedByOwnedBy, @@ -39,9 +28,7 @@ func DefaultOptions() *Options { ), WithSingletonClientCache(NewMemoryClientCache()), WithManifestCache(os.TempDir()), - WithSkipReconcileOn(SkipReconcileOnDefaultLabelPresentAndTrue), WithManifestParser(NewInMemoryCachedManifestParser(DefaultInMemoryParseTTL)), - WithModuleCRDeletionCheck(NewDefaultDeletionCheck()), ) } @@ -51,31 +38,16 @@ type Options struct { client.Client TargetCluster ClusterFn - SpecResolver ClientCache ClientCacheKeyFn ManifestParser ManifestCache CustomReadyCheck ReadyCheck - Namespace string - CreateNamespace bool - - Finalizer string - - ServerSideApply bool - FieldOwner client.FieldOwner - PostRenderTransforms []ObjectTransform PostRuns []PostRun PreDeletes []PreDelete - - DeletionCheck ModuleCRDeletionCheck - - DeletePrerequisites bool - - ShouldSkip SkipReconcile } type Option interface { @@ -89,35 +61,6 @@ func (o *Options) Apply(options ...Option) *Options { return o } -type WithNamespaceOption struct { - name string - createIfMissing bool -} - -func WithNamespace(name string, createIfMissing bool) WithNamespaceOption { - return WithNamespaceOption{ - name: name, - createIfMissing: createIfMissing, - } -} - -func (o WithNamespaceOption) Apply(options *Options) { - options.Namespace = o.name - options.CreateNamespace = o.createIfMissing -} - -type WithFieldOwner client.FieldOwner - -func (o WithFieldOwner) Apply(options *Options) { - options.FieldOwner = client.FieldOwner(o) -} - -type WithFinalizer string - -func (o WithFinalizer) Apply(options *Options) { - options.Finalizer = string(o) -} - type WithManagerOption struct { manager.Manager } @@ -151,18 +94,6 @@ func (o WithCustomResourceLabels) Apply(options *Options) { options.PostRenderTransforms = append(options.PostRenderTransforms, labelTransform) } -func WithSpecResolver(resolver SpecResolver) SpecResolverOption { - return SpecResolverOption{resolver} -} - -type SpecResolverOption struct { - SpecResolver -} - -func (o SpecResolverOption) Apply(options *Options) { - options.SpecResolver = o -} - type ObjectTransform = func(context.Context, Object, []*unstructured.Unstructured) error func WithPostRenderTransform(transforms ...ObjectTransform) PostRenderTransformOption { @@ -207,18 +138,6 @@ func (o WithPreDelete) Apply(options *Options) { options.PreDeletes = append(options.PreDeletes, o...) } -func WithModuleCRDeletionCheck(deletionCheckFn ModuleCRDeletionCheck) WithModuleCRDeletionCheckOption { - return WithModuleCRDeletionCheckOption{ModuleCRDeletionCheck: deletionCheckFn} -} - -type WithModuleCRDeletionCheckOption struct { - ModuleCRDeletionCheck -} - -func (o WithModuleCRDeletionCheckOption) Apply(options *Options) { - options.DeletionCheck = o -} - type WithSingletonClientCacheOption struct { ClientCache } @@ -277,31 +196,6 @@ func (o WithRemoteTargetClusterOption) Apply(options *Options) { options.TargetCluster = o.ClusterFn } -func WithSkipReconcileOn(skipReconcile SkipReconcile) WithSkipReconcileOnOption { - return WithSkipReconcileOnOption{skipReconcile: skipReconcile} -} - -type SkipReconcile func(context.Context, Object) (skip bool) - -// SkipReconcileOnDefaultLabelPresentAndTrue determines SkipReconcile by checking if DefaultSkipReconcileLabel is true. -func SkipReconcileOnDefaultLabelPresentAndTrue(ctx context.Context, object Object) bool { - if object.GetLabels() != nil && object.GetLabels()[shared.SkipReconcileLabel] == strconv.FormatBool(true) { - logf.FromContext(ctx, "skip-label", shared.SkipReconcileLabel). - V(internal.DebugLogLevel).Info("resource gets skipped because of label") - return true - } - - return false -} - -type WithSkipReconcileOnOption struct { - skipReconcile SkipReconcile -} - -func (o WithSkipReconcileOnOption) Apply(options *Options) { - options.ShouldSkip = o.skipReconcile -} - type ClientCacheKeyFn func(ctx context.Context, obj Object) (string, bool) type WithClientCacheKeyOption struct { diff --git a/internal/declarative/v2/reconciler.go b/internal/declarative/v2/reconciler.go index c7cf54ce30..91de58d41b 100644 --- a/internal/declarative/v2/reconciler.go +++ b/internal/declarative/v2/reconciler.go @@ -7,9 +7,10 @@ import ( "strconv" "time" - apicorev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" apimetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/cli-runtime/pkg/resource" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -19,6 +20,7 @@ import ( "github.com/kyma-project/lifecycle-manager/api/shared" "github.com/kyma-project/lifecycle-manager/api/v1beta2" + "github.com/kyma-project/lifecycle-manager/internal" "github.com/kyma-project/lifecycle-manager/internal/pkg/metrics" "github.com/kyma-project/lifecycle-manager/internal/pkg/resources" "github.com/kyma-project/lifecycle-manager/pkg/common" @@ -37,29 +39,35 @@ var ( ) const ( - namespaceNotBeRemoved = "kyma-system" - CustomResourceManager = "resource.kyma-project.io/finalizer" - SyncedOCIRefAnnotation = "sync-oci-ref" + namespaceNotBeRemoved = "kyma-system" + CustomResourceManagerFinalizer = "resource.kyma-project.io/finalizer" + SyncedOCIRefAnnotation = "sync-oci-ref" + defaultFinalizer = "declarative.kyma-project.io/finalizer" + defaultFieldOwner client.FieldOwner = "declarative.kyma-project.io/applier" ) -func NewFromManager(mgr manager.Manager, prototype Object, requeueIntervals queue.RequeueIntervals, - metrics *metrics.ManifestMetrics, mandatoryModulesMetrics *metrics.MandatoryModulesMetrics, options ...Option, +func NewFromManager(mgr manager.Manager, + requeueIntervals queue.RequeueIntervals, + metrics *metrics.ManifestMetrics, + mandatoryModulesMetrics *metrics.MandatoryModulesMetrics, + specResolver SpecResolver, + options ...Option, ) *Reconciler { reconciler := &Reconciler{} - reconciler.prototype = prototype reconciler.ManifestMetrics = metrics reconciler.MandatoryModuleMetrics = mandatoryModulesMetrics reconciler.RequeueIntervals = requeueIntervals + reconciler.specResolver = specResolver reconciler.Options = DefaultOptions().Apply(WithManager(mgr)).Apply(options...) return reconciler } type Reconciler struct { - prototype Object queue.RequeueIntervals *Options ManifestMetrics *metrics.ManifestMetrics MandatoryModuleMetrics *metrics.MandatoryModulesMetrics + specResolver SpecResolver } type ConditionType string @@ -76,23 +84,23 @@ const ( ConditionReasonReady ConditionReason = "Ready" ) -func newInstallationCondition(obj Object) apimetav1.Condition { +func newInstallationCondition(manifest *v1beta2.Manifest) apimetav1.Condition { return apimetav1.Condition{ Type: string(ConditionTypeInstallation), Reason: string(ConditionReasonReady), Status: apimetav1.ConditionFalse, Message: "installation is ready and resources can be used", - ObservedGeneration: obj.GetGeneration(), + ObservedGeneration: manifest.GetGeneration(), } } -func newResourcesCondition(obj Object) apimetav1.Condition { +func newResourcesCondition(manifest *v1beta2.Manifest) apimetav1.Condition { return apimetav1.Condition{ Type: string(ConditionTypeResources), Reason: string(ConditionReasonResourcesAreAvailable), Status: apimetav1.ConditionFalse, Message: "resources are parsed and ready for use", - ObservedGeneration: obj.GetGeneration(), + ObservedGeneration: manifest.GetGeneration(), } } @@ -100,12 +108,9 @@ func newResourcesCondition(obj Object) apimetav1.Condition { func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { startTime := time.Now() defer r.recordReconciliationDuration(startTime, req.Name) - obj, ok := r.prototype.DeepCopyObject().(Object) - if !ok { - r.ManifestMetrics.RecordRequeueReason(metrics.ManifestTypeCast, queue.UnexpectedRequeue) - return ctrl.Result{}, common.ErrTypeAssert - } - if err := r.Get(ctx, req.NamespacedName, obj); err != nil { + + manifest := &v1beta2.Manifest{} + if err := r.Get(ctx, req.NamespacedName, manifest); err != nil { if util.IsNotFound(err) { logf.FromContext(ctx).Info(req.NamespacedName.String() + " got deleted!") return ctrl.Result{}, nil @@ -113,130 +118,133 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu r.ManifestMetrics.RecordRequeueReason(metrics.ManifestRetrieval, queue.UnexpectedRequeue) return ctrl.Result{}, fmt.Errorf("manifestController: %w", err) } - currentObjStatus := obj.GetStatus() + manifestStatus := manifest.GetStatus() - if r.ShouldSkip(ctx, obj) { + if manifest.SkipReconciliation() { + logf.FromContext(ctx, "skip-label", shared.SkipReconcileLabel). + V(internal.DebugLogLevel).Info("resource gets skipped because of label") return ctrl.Result{RequeueAfter: r.Success}, nil } - if err := r.initialize(obj); err != nil { - return r.finishReconcile(ctx, obj, metrics.ManifestInit, currentObjStatus, err) + if err := r.initialize(manifest); err != nil { + return r.finishReconcile(ctx, manifest, metrics.ManifestInit, manifestStatus, err) } - if obj.GetLabels() != nil && obj.GetLabels()[shared.IsMandatoryModule] == strconv.FormatBool(true) { - state := obj.GetStatus().State - kymaName := obj.GetLabels()[shared.KymaName] - moduleName := obj.GetLabels()[shared.ModuleName] + if manifest.GetLabels() != nil && manifest.GetLabels()[shared.IsMandatoryModule] == strconv.FormatBool(true) { + state := manifest.GetStatus().State + kymaName := manifest.GetLabels()[shared.KymaName] + moduleName := manifest.GetLabels()[shared.ModuleName] r.MandatoryModuleMetrics.RecordMandatoryModuleState(kymaName, moduleName, state) } - if obj.GetDeletionTimestamp().IsZero() { - objMeta := r.partialObjectMetadata(obj) - if controllerutil.AddFinalizer(objMeta, r.Finalizer) { - return r.ssaSpec(ctx, objMeta, metrics.ManifestAddFinalizer) + if manifest.GetDeletionTimestamp().IsZero() { + partialMeta := r.partialObjectMetadata(manifest) + if controllerutil.AddFinalizer(partialMeta, defaultFinalizer) { + return r.ssaSpec(ctx, partialMeta, metrics.ManifestAddFinalizer) } } - spec, err := r.Spec(ctx, obj) + spec, err := r.specResolver.GetSpec(ctx, manifest) if err != nil { - if !obj.GetDeletionTimestamp().IsZero() { - return r.cleanupManifest(ctx, req, obj, currentObjStatus, metrics.ManifestParseSpec, err) + manifest.SetStatus(manifest.GetStatus().WithState(shared.StateError).WithErr(err)) + if !manifest.GetDeletionTimestamp().IsZero() { + return r.cleanupManifest(ctx, req, manifest, manifestStatus, metrics.ManifestParseSpec, err) } - return r.finishReconcile(ctx, obj, metrics.ManifestParseSpec, currentObjStatus, err) + return r.finishReconcile(ctx, manifest, metrics.ManifestParseSpec, manifestStatus, err) } - if notContainsSyncedOCIRefAnnotation(obj) { - updateSyncedOCIRefAnnotation(obj, spec.OCIRef) - return r.updateObject(ctx, obj, metrics.ManifestInitSyncedOCIRef) + if notContainsSyncedOCIRefAnnotation(manifest) { + updateSyncedOCIRefAnnotation(manifest, spec.OCIRef) + return r.updateObject(ctx, manifest, metrics.ManifestInitSyncedOCIRef) } - clnt, err := r.getTargetClient(ctx, obj) + skrClient, err := r.getTargetClient(ctx, manifest) if err != nil { - if !obj.GetDeletionTimestamp().IsZero() && errors.Is(err, ErrAccessSecretNotFound) { - return r.cleanupManifest(ctx, req, obj, currentObjStatus, metrics.ManifestClientInit, + if !manifest.GetDeletionTimestamp().IsZero() && errors.Is(err, ErrAccessSecretNotFound) { + return r.cleanupManifest(ctx, req, manifest, manifestStatus, metrics.ManifestClientInit, err) } - obj.SetStatus(obj.GetStatus().WithState(shared.StateError).WithErr(err)) - return r.finishReconcile(ctx, obj, metrics.ManifestClientInit, currentObjStatus, err) + manifest.SetStatus(manifest.GetStatus().WithState(shared.StateError).WithErr(err)) + return r.finishReconcile(ctx, manifest, metrics.ManifestClientInit, manifestStatus, err) } - target, current, err := r.renderResources(ctx, clnt, obj, spec) + target, current, err := r.renderResources(ctx, skrClient, manifest, spec) if err != nil { if util.IsConnectionRelatedError(err) { - r.invalidateClientCache(ctx, obj) - return r.finishReconcile(ctx, obj, metrics.ManifestUnauthorized, currentObjStatus, err) + r.invalidateClientCache(ctx, manifest) + return r.finishReconcile(ctx, manifest, metrics.ManifestUnauthorized, manifestStatus, err) } - return r.finishReconcile(ctx, obj, metrics.ManifestRenderResources, currentObjStatus, err) + return r.finishReconcile(ctx, manifest, metrics.ManifestRenderResources, manifestStatus, err) } - if err := r.pruneDiff(ctx, clnt, obj, current, target, spec); errors.Is(err, resources.ErrDeletionNotFinished) { + if err := r.pruneDiff(ctx, skrClient, manifest, current, target, spec); errors.Is(err, resources.ErrDeletionNotFinished) { r.ManifestMetrics.RecordRequeueReason(metrics.ManifestPruneDiffNotFinished, queue.IntendedRequeue) return ctrl.Result{Requeue: true}, nil } else if err != nil { - return r.finishReconcile(ctx, obj, metrics.ManifestPruneDiff, currentObjStatus, err) + return r.finishReconcile(ctx, manifest, metrics.ManifestPruneDiff, manifestStatus, err) } - if err := r.removeModuleCR(ctx, clnt, obj); err != nil { + if err := r.removeModuleCR(ctx, skrClient, manifest); err != nil { if errors.Is(err, ErrRequeueRequired) { r.ManifestMetrics.RecordRequeueReason(metrics.ManifestPreDeleteEnqueueRequired, queue.IntendedRequeue) return ctrl.Result{Requeue: true}, nil } - return r.finishReconcile(ctx, obj, metrics.ManifestPreDelete, currentObjStatus, err) + return r.finishReconcile(ctx, manifest, metrics.ManifestPreDelete, manifestStatus, err) } - if err = r.syncResources(ctx, clnt, obj, target); err != nil { + if err = r.syncResources(ctx, skrClient, manifest, target); err != nil { if errors.Is(err, ErrRequeueRequired) { r.ManifestMetrics.RecordRequeueReason(metrics.ManifestSyncResourcesEnqueueRequired, queue.IntendedRequeue) return ctrl.Result{Requeue: true}, nil } if errors.Is(err, ErrClientUnauthorized) { - r.invalidateClientCache(ctx, obj) + r.invalidateClientCache(ctx, manifest) } - return r.finishReconcile(ctx, obj, metrics.ManifestSyncResources, currentObjStatus, err) + return r.finishReconcile(ctx, manifest, metrics.ManifestSyncResources, manifestStatus, err) } // This situation happens when manifest get new installation layer to update resources, // we need to make sure all updates successfully before we can update synced oci ref - if requireUpdateSyncedOCIRefAnnotation(obj, spec.OCIRef) { - updateSyncedOCIRefAnnotation(obj, spec.OCIRef) - return r.updateObject(ctx, obj, metrics.ManifestUpdateSyncedOCIRef) + if requireUpdateSyncedOCIRefAnnotation(manifest, spec.OCIRef) { + updateSyncedOCIRefAnnotation(manifest, spec.OCIRef) + return r.updateObject(ctx, manifest, metrics.ManifestUpdateSyncedOCIRef) } - if !obj.GetDeletionTimestamp().IsZero() { - return r.cleanupManifest(ctx, req, obj, currentObjStatus, metrics.ManifestReconcileFinished, nil) + if !manifest.GetDeletionTimestamp().IsZero() { + return r.cleanupManifest(ctx, req, manifest, manifestStatus, metrics.ManifestReconcileFinished, nil) } - return r.finishReconcile(ctx, obj, metrics.ManifestReconcileFinished, currentObjStatus, nil) + return r.finishReconcile(ctx, manifest, metrics.ManifestReconcileFinished, manifestStatus, nil) } -func (r *Reconciler) cleanupManifest(ctx context.Context, req ctrl.Request, obj Object, currentObjStatus shared.Status, +func (r *Reconciler) cleanupManifest(ctx context.Context, req ctrl.Request, manifest *v1beta2.Manifest, manifestStatus shared.Status, requeueReason metrics.ManifestRequeueReason, originalErr error, ) (ctrl.Result, error) { r.ManifestMetrics.RemoveManifestDuration(req.Name) - r.cleanUpMandatoryModuleMetrics(obj) - if removeFinalizers(obj, r.finalizerToRemove(originalErr, obj)) { - return r.updateObject(ctx, obj, requeueReason) + r.cleanUpMandatoryModuleMetrics(manifest) + if removeFinalizers(manifest, r.finalizerToRemove(originalErr, manifest)) { + return r.updateObject(ctx, manifest, requeueReason) } - if obj.GetStatus().State != shared.StateWarning { - obj.SetStatus(obj.GetStatus().WithState(shared.StateDeleting). - WithOperation(fmt.Sprintf("waiting as other finalizers are present: %s", obj.GetFinalizers()))) + if manifest.GetStatus().State != shared.StateWarning { + manifest.SetStatus(manifest.GetStatus().WithState(shared.StateDeleting). + WithOperation(fmt.Sprintf("waiting as other finalizers are present: %s", manifest.GetFinalizers()))) } - return r.finishReconcile(ctx, obj, requeueReason, currentObjStatus, originalErr) + return r.finishReconcile(ctx, manifest, requeueReason, manifestStatus, originalErr) } -func (r *Reconciler) finalizerToRemove(originalErr error, obj Object) []string { - finalizersToRemove := []string{r.Finalizer} +func (r *Reconciler) finalizerToRemove(originalErr error, manifest *v1beta2.Manifest) []string { + finalizersToRemove := []string{defaultFinalizer} if errors.Is(originalErr, ErrAccessSecretNotFound) { - finalizersToRemove = obj.GetFinalizers() + finalizersToRemove = manifest.GetFinalizers() } return finalizersToRemove } -func (r *Reconciler) invalidateClientCache(ctx context.Context, obj Object) { +func (r *Reconciler) invalidateClientCache(ctx context.Context, manifest *v1beta2.Manifest) { if r.ClientCacheKeyFn != nil { - clientsCacheKey, ok := r.ClientCacheKeyFn(ctx, obj) + clientsCacheKey, ok := r.ClientCacheKeyFn(ctx, manifest) if ok { logf.FromContext(ctx).Info("Invalidating manifest-controller client cache entry for key: " + fmt.Sprintf("%#v", clientsCacheKey)) @@ -245,10 +253,10 @@ func (r *Reconciler) invalidateClientCache(ctx context.Context, obj Object) { } } -func removeFinalizers(obj Object, finalizersToRemove []string) bool { +func removeFinalizers(manifest *v1beta2.Manifest, finalizersToRemove []string) bool { finalizerRemoved := false for _, f := range finalizersToRemove { - if controllerutil.RemoveFinalizer(obj, f) { + if controllerutil.RemoveFinalizer(manifest, f) { finalizerRemoved = true } } @@ -256,21 +264,21 @@ func removeFinalizers(obj Object, finalizersToRemove []string) bool { return finalizerRemoved } -func (r *Reconciler) partialObjectMetadata(obj Object) *apimetav1.PartialObjectMetadata { +func (r *Reconciler) partialObjectMetadata(manifest *v1beta2.Manifest) *apimetav1.PartialObjectMetadata { objMeta := &apimetav1.PartialObjectMetadata{} - objMeta.SetName(obj.GetName()) - objMeta.SetNamespace(obj.GetNamespace()) - objMeta.SetGroupVersionKind(obj.GetObjectKind().GroupVersionKind()) - objMeta.SetFinalizers(obj.GetFinalizers()) + objMeta.SetName(manifest.GetName()) + objMeta.SetNamespace(manifest.GetNamespace()) + objMeta.SetGroupVersionKind(manifest.GetObjectKind().GroupVersionKind()) + objMeta.SetFinalizers(manifest.GetFinalizers()) return objMeta } -func (r *Reconciler) initialize(obj Object) error { - status := obj.GetStatus() +func (r *Reconciler) initialize(manifest *v1beta2.Manifest) error { + status := manifest.GetStatus() for _, condition := range []apimetav1.Condition{ - newResourcesCondition(obj), - newInstallationCondition(obj), + newResourcesCondition(manifest), + newInstallationCondition(manifest), } { if meta.FindStatusCondition(status.Conditions, condition.Type) == nil { meta.SetStatusCondition(&status.Conditions, condition) @@ -282,64 +290,56 @@ func (r *Reconciler) initialize(obj Object) error { } if status.State == "" { - obj.SetStatus(status.WithState(shared.StateProcessing).WithErr(ErrObjectHasEmptyState)) + manifest.SetStatus(status.WithState(shared.StateProcessing).WithErr(ErrObjectHasEmptyState)) return ErrObjectHasEmptyState } - obj.SetStatus(status) + manifest.SetStatus(status) return nil } -func (r *Reconciler) Spec(ctx context.Context, obj Object) (*Spec, error) { - spec, err := r.SpecResolver.Spec(ctx, obj) - if err != nil { - obj.SetStatus(obj.GetStatus().WithState(shared.StateError).WithErr(err)) - } - return spec, err -} - func (r *Reconciler) renderResources( ctx context.Context, - clnt Client, - obj Object, + skrClient Client, + manifest *v1beta2.Manifest, spec *Spec, ) ([]*resource.Info, []*resource.Info, error) { - resourceCondition := newResourcesCondition(obj) - status := obj.GetStatus() + resourceCondition := newResourcesCondition(manifest) + status := manifest.GetStatus() var err error var target, current ResourceList - converter := NewResourceToInfoConverter(ResourceInfoConverter(clnt), r.Namespace) + converter := NewResourceToInfoConverter(ResourceInfoConverter(skrClient), apimetav1.NamespaceDefault) - if target, err = r.renderTargetResources(ctx, clnt, converter, obj, spec); err != nil { - obj.SetStatus(status.WithState(shared.StateError).WithErr(err)) + if target, err = r.renderTargetResources(ctx, skrClient, converter, manifest, spec); err != nil { + manifest.SetStatus(status.WithState(shared.StateError).WithErr(err)) return nil, nil, err } current, err = converter.ResourcesToInfos(status.Synced) if err != nil { - obj.SetStatus(status.WithState(shared.StateError).WithErr(err)) + manifest.SetStatus(status.WithState(shared.StateError).WithErr(err)) return nil, nil, err } if !meta.IsStatusConditionTrue(status.Conditions, resourceCondition.Type) { resourceCondition.Status = apimetav1.ConditionTrue meta.SetStatusCondition(&status.Conditions, resourceCondition) - obj.SetStatus(status.WithOperation(resourceCondition.Message)) + manifest.SetStatus(status.WithOperation(resourceCondition.Message)) } return target, current, nil } -func (r *Reconciler) syncResources(ctx context.Context, clnt Client, obj Object, +func (r *Reconciler) syncResources(ctx context.Context, clnt Client, manifest *v1beta2.Manifest, target []*resource.Info, ) error { - status := obj.GetStatus() + status := manifest.GetStatus() - if err := ConcurrentSSA(clnt, r.FieldOwner).Run(ctx, target); err != nil { - obj.SetStatus(status.WithState(shared.StateError).WithErr(err)) + if err := ConcurrentSSA(clnt, defaultFieldOwner).Run(ctx, target); err != nil { + manifest.SetStatus(status.WithState(shared.StateError).WithErr(err)) return err } @@ -348,27 +348,27 @@ func (r *Reconciler) syncResources(ctx context.Context, clnt Client, obj Object, status.Synced = newSynced if hasDiff(oldSynced, newSynced) { - if obj.GetDeletionTimestamp().IsZero() { - obj.SetStatus(status.WithState(shared.StateProcessing).WithOperation(ErrWarningResourceSyncStateDiff.Error())) + if manifest.GetDeletionTimestamp().IsZero() { + manifest.SetStatus(status.WithState(shared.StateProcessing).WithOperation(ErrWarningResourceSyncStateDiff.Error())) } else if status.State != shared.StateWarning { - obj.SetStatus(status.WithState(shared.StateDeleting).WithOperation(ErrWarningResourceSyncStateDiff.Error())) + manifest.SetStatus(status.WithState(shared.StateDeleting).WithOperation(ErrWarningResourceSyncStateDiff.Error())) } return ErrWarningResourceSyncStateDiff } for i := range r.PostRuns { - if err := r.PostRuns[i](ctx, clnt, r.Client, obj); err != nil { - obj.SetStatus(status.WithState(shared.StateError).WithErr(err)) + if err := r.PostRuns[i](ctx, clnt, r.Client, manifest); err != nil { + manifest.SetStatus(status.WithState(shared.StateError).WithErr(err)) return err } } deploymentState, err := r.checkDeploymentState(ctx, clnt, target) if err != nil { - obj.SetStatus(status.WithState(shared.StateError).WithErr(err)) + manifest.SetStatus(status.WithState(shared.StateError).WithErr(err)) return err } - return r.setManifestState(obj, deploymentState) + return r.setManifestState(manifest, deploymentState) } func hasDiff(oldResources []shared.Resource, newResources []shared.Resource) bool { @@ -409,7 +409,7 @@ func (r *Reconciler) checkDeploymentState( return deploymentState, nil } -func (r *Reconciler) setManifestState(manifest Object, state shared.State) error { +func (r *Reconciler) setManifestState(manifest *v1beta2.Manifest, state shared.State) error { status := manifest.GetStatus() if state == shared.StateProcessing { @@ -435,12 +435,12 @@ func (r *Reconciler) setManifestState(manifest Object, state shared.State) error return nil } -func (r *Reconciler) removeModuleCR(ctx context.Context, clnt Client, obj Object) error { - if !obj.GetDeletionTimestamp().IsZero() { +func (r *Reconciler) removeModuleCR(ctx context.Context, clnt Client, manifest *v1beta2.Manifest) error { + if !manifest.GetDeletionTimestamp().IsZero() { for _, preDelete := range r.PreDeletes { - if err := preDelete(ctx, clnt, r.Client, obj); err != nil { + if err := preDelete(ctx, clnt, r.Client, manifest); err != nil { // we do not set a status here since it will be deleting if timestamp is set. - obj.SetStatus(obj.GetStatus().WithErr(err)) + manifest.SetStatus(manifest.GetStatus().WithErr(err)) return err } } @@ -450,13 +450,13 @@ func (r *Reconciler) removeModuleCR(ctx context.Context, clnt Client, obj Object func (r *Reconciler) renderTargetResources( ctx context.Context, - clnt client.Client, + skrClient client.Client, converter ResourceToInfoConverter, - obj Object, + manifest *v1beta2.Manifest, spec *Spec, ) ([]*resource.Info, error) { - if !obj.GetDeletionTimestamp().IsZero() { - deleted, err := r.DeletionCheck.Run(ctx, clnt, obj) + if !manifest.GetDeletionTimestamp().IsZero() { + deleted, err := checkCRDeletion(ctx, skrClient, manifest) if err != nil { return nil, err } @@ -465,99 +465,119 @@ func (r *Reconciler) renderTargetResources( } } - status := obj.GetStatus() + status := manifest.GetStatus() targetResources, err := r.ManifestParser.Parse(spec) if err != nil { - obj.SetStatus(status.WithState(shared.StateError).WithErr(err)) + manifest.SetStatus(status.WithState(shared.StateError).WithErr(err)) return nil, err } for _, transform := range r.PostRenderTransforms { - if err := transform(ctx, obj, targetResources.Items); err != nil { - obj.SetStatus(status.WithState(shared.StateError).WithErr(err)) + if err := transform(ctx, manifest, targetResources.Items); err != nil { + manifest.SetStatus(status.WithState(shared.StateError).WithErr(err)) return nil, err } } target, err := converter.UnstructuredToInfos(targetResources.Items) if err != nil { - obj.SetStatus(status.WithState(shared.StateError).WithErr(err)) + manifest.SetStatus(status.WithState(shared.StateError).WithErr(err)) return nil, err } return target, nil } +func checkCRDeletion(ctx context.Context, skrClient client.Client, manifest *v1beta2.Manifest) (bool, error) { + if manifest.Spec.Resource == nil { + return true, nil + } + + name := manifest.Spec.Resource.GetName() + namespace := manifest.Spec.Resource.GetNamespace() + gvk := manifest.Spec.Resource.GroupVersionKind() + + resourceCR := &unstructured.Unstructured{} + resourceCR.SetGroupVersionKind(schema.GroupVersionKind{ + Group: gvk.Group, + Version: gvk.Version, + Kind: gvk.Kind, + }) + + if err := skrClient.Get(ctx, + client.ObjectKey{Name: name, Namespace: namespace}, resourceCR); err != nil { + if util.IsNotFound(err) { + return true, nil + } + return false, fmt.Errorf("%w: failed to fetch default resource CR", err) + } + return false, nil +} + func (r *Reconciler) pruneDiff( ctx context.Context, clnt Client, - obj Object, + manifest *v1beta2.Manifest, current, target []*resource.Info, spec *Spec, ) error { diff, err := pruneResource(ResourceList(current).Difference(target), "Namespace", namespaceNotBeRemoved) if err != nil { - obj.SetStatus(obj.GetStatus().WithErr(err)) + manifest.SetStatus(manifest.GetStatus().WithErr(err)) return err } if len(diff) == 0 { return nil } - if manifestNotInDeletingAndOciRefNotChangedButDiffDetected(diff, obj, spec) { + if manifestNotInDeletingAndOciRefNotChangedButDiffDetected(diff, manifest, spec) { // This case should not happen normally, but if happens, it means the resources read from cache is incomplete, // and we should prevent diff resources to be deleted. // Meanwhile, evict cache to hope newly created resources back to normal. - obj.SetStatus(obj.GetStatus().WithState(shared.StateWarning).WithOperation(ErrResourceSyncDiffInSameOCILayer.Error())) + manifest.SetStatus(manifest.GetStatus().WithState(shared.StateWarning).WithOperation(ErrResourceSyncDiffInSameOCILayer.Error())) r.ManifestParser.EvictCache(spec) return ErrResourceSyncDiffInSameOCILayer } - // Remove this type casting while in progress this issue: https://github.com/kyma-project/lifecycle-manager/issues/1006 - manifest, ok := obj.(*v1beta2.Manifest) - if !ok { - obj.SetStatus(obj.GetStatus().WithErr(v1beta2.ErrTypeAssertManifest)) - return v1beta2.ErrTypeAssertManifest - } err = resources.NewConcurrentCleanup(clnt, manifest).DeleteDiffResources(ctx, diff) if err != nil { - obj.SetStatus(obj.GetStatus().WithErr(err)) + manifest.SetStatus(manifest.GetStatus().WithErr(err)) } return err } -func manifestNotInDeletingAndOciRefNotChangedButDiffDetected(diff []*resource.Info, obj Object, +func manifestNotInDeletingAndOciRefNotChangedButDiffDetected(diff []*resource.Info, manifest *v1beta2.Manifest, spec *Spec, ) bool { - return len(diff) > 0 && ociRefNotChanged(obj, spec.OCIRef) && obj.GetDeletionTimestamp().IsZero() + return len(diff) > 0 && ociRefNotChanged(manifest, spec.OCIRef) && manifest.GetDeletionTimestamp().IsZero() } -func ociRefNotChanged(obj Object, ref string) bool { - syncedOCIRef, found := obj.GetAnnotations()[SyncedOCIRefAnnotation] +func ociRefNotChanged(manifest *v1beta2.Manifest, ref string) bool { + syncedOCIRef, found := manifest.GetAnnotations()[SyncedOCIRefAnnotation] return found && syncedOCIRef == ref } -func requireUpdateSyncedOCIRefAnnotation(obj Object, ref string) bool { - syncedOCIRef, found := obj.GetAnnotations()[SyncedOCIRefAnnotation] +func requireUpdateSyncedOCIRefAnnotation(manifest *v1beta2.Manifest, ref string) bool { + syncedOCIRef, found := manifest.GetAnnotations()[SyncedOCIRefAnnotation] if found && syncedOCIRef != ref { return true } return false } -func notContainsSyncedOCIRefAnnotation(obj Object) bool { - _, found := obj.GetAnnotations()[SyncedOCIRefAnnotation] +func notContainsSyncedOCIRefAnnotation(manifest *v1beta2.Manifest) bool { + _, found := manifest.GetAnnotations()[SyncedOCIRefAnnotation] return !found } -func updateSyncedOCIRefAnnotation(obj Object, ref string) { - annotations := obj.GetAnnotations() +func updateSyncedOCIRefAnnotation(manifest *v1beta2.Manifest, ref string) { + annotations := manifest.GetAnnotations() if annotations == nil { annotations = make(map[string]string) } annotations[SyncedOCIRefAnnotation] = ref - obj.SetAnnotations(annotations) + manifest.SetAnnotations(annotations) } func pruneResource(diff []*resource.Info, resourceType string, resourceName string) ([]*resource.Info, error) { @@ -574,42 +594,30 @@ func pruneResource(diff []*resource.Info, resourceType string, resourceName stri return diff, nil } -func (r *Reconciler) getTargetClient(ctx context.Context, obj Object) (Client, error) { +func (r *Reconciler) getTargetClient(ctx context.Context, manifest *v1beta2.Manifest) (Client, error) { var err error var clnt Client if r.ClientCacheKeyFn == nil { - return r.configClient(ctx, obj) + return r.configClient(ctx, manifest) } - clientsCacheKey, found := r.ClientCacheKeyFn(ctx, obj) + clientsCacheKey, found := r.ClientCacheKeyFn(ctx, manifest) if found { clnt = r.GetClient(clientsCacheKey) } if clnt == nil { - clnt, err = r.configClient(ctx, obj) + clnt, err = r.configClient(ctx, manifest) if err != nil { return nil, err } r.AddClient(clientsCacheKey, clnt) } - if r.Namespace != apimetav1.NamespaceNone && r.Namespace != apimetav1.NamespaceDefault { - err := clnt.Patch( - ctx, &apicorev1.Namespace{ - TypeMeta: apimetav1.TypeMeta{APIVersion: "v1", Kind: "Namespace"}, - ObjectMeta: apimetav1.ObjectMeta{Name: r.Namespace}, - }, client.Apply, client.ForceOwnership, r.FieldOwner, - ) - if err != nil { - return nil, fmt.Errorf("failed to patch namespace: %w", err) - } - } - return clnt, nil } -func (r *Reconciler) configClient(ctx context.Context, obj Object) (Client, error) { +func (r *Reconciler) configClient(ctx context.Context, manifest *v1beta2.Manifest) (Client, error) { var err error cluster := &ClusterInfo{ @@ -618,7 +626,7 @@ func (r *Reconciler) configClient(ctx context.Context, obj Object) (Client, erro } if r.TargetCluster != nil { - cluster, err = r.TargetCluster(ctx, obj) + cluster, err = r.TargetCluster(ctx, manifest) if err != nil { return nil, err } @@ -632,11 +640,11 @@ func (r *Reconciler) configClient(ctx context.Context, obj Object) (Client, erro return clnt, nil } -func (r *Reconciler) finishReconcile(ctx context.Context, obj Object, +func (r *Reconciler) finishReconcile(ctx context.Context, manifest *v1beta2.Manifest, requeueReason metrics.ManifestRequeueReason, previousStatus shared.Status, originalErr error, ) (ctrl.Result, error) { - if err := r.patchStatusIfDiffExist(ctx, obj, previousStatus); err != nil { - r.Event(obj, "Warning", "PatchStatus", err.Error()) + if err := r.patchStatusIfDiffExist(ctx, manifest, previousStatus); err != nil { + r.Event(manifest, "Warning", "PatchStatus", err.Error()) return ctrl.Result{}, fmt.Errorf("failed to patch status: %w", err) } if originalErr != nil { @@ -647,10 +655,10 @@ func (r *Reconciler) finishReconcile(ctx context.Context, obj Object, return ctrl.Result{RequeueAfter: r.Success}, nil } -func (r *Reconciler) patchStatusIfDiffExist(ctx context.Context, obj Object, previousStatus shared.Status) error { - if hasStatusDiff(obj.GetStatus(), previousStatus) { - resetNonPatchableField(obj) - if err := r.Status().Patch(ctx, obj, client.Apply, client.ForceOwnership, r.FieldOwner); err != nil { +func (r *Reconciler) patchStatusIfDiffExist(ctx context.Context, manifest *v1beta2.Manifest, previousStatus shared.Status) error { + if hasStatusDiff(manifest.GetStatus(), previousStatus) { + resetNonPatchableField(manifest) + if err := r.Status().Patch(ctx, manifest, client.Apply, client.ForceOwnership, defaultFieldOwner); err != nil { return fmt.Errorf("failed to patch status: %w", err) } } @@ -667,7 +675,7 @@ func (r *Reconciler) ssaSpec(ctx context.Context, obj client.Object, ) (ctrl.Result, error) { resetNonPatchableField(obj) r.ManifestMetrics.RecordRequeueReason(requeueReason, queue.IntendedRequeue) - if err := r.Patch(ctx, obj, client.Apply, client.ForceOwnership, r.FieldOwner); err != nil { + if err := r.Patch(ctx, obj, client.Apply, client.ForceOwnership, defaultFieldOwner); err != nil { r.Event(obj, "Warning", "PatchObject", err.Error()) return ctrl.Result{}, fmt.Errorf("failed to patch object: %w", err) } @@ -700,10 +708,10 @@ func (r *Reconciler) recordReconciliationDuration(startTime time.Time, name stri } } -func (r *Reconciler) cleanUpMandatoryModuleMetrics(obj Object) { - if obj.GetLabels()[shared.IsMandatoryModule] == strconv.FormatBool(true) { - kymaName := obj.GetLabels()[shared.KymaName] - moduleName := obj.GetLabels()[shared.ModuleName] +func (r *Reconciler) cleanUpMandatoryModuleMetrics(manifest *v1beta2.Manifest) { + if manifest.GetLabels()[shared.IsMandatoryModule] == strconv.FormatBool(true) { + kymaName := manifest.GetLabels()[shared.KymaName] + moduleName := manifest.GetLabels()[shared.ModuleName] r.MandatoryModuleMetrics.CleanupMetrics(kymaName, moduleName) } } diff --git a/internal/declarative/v2/spec.go b/internal/declarative/v2/spec.go index e012e5f05b..6e3044bf8e 100644 --- a/internal/declarative/v2/spec.go +++ b/internal/declarative/v2/spec.go @@ -2,47 +2,16 @@ package v2 import ( "context" + + "github.com/kyma-project/lifecycle-manager/api/v1beta2" ) type SpecResolver interface { - Spec(ctx context.Context, object Object) (*Spec, error) + GetSpec(ctx context.Context, manifest *v1beta2.Manifest) (*Spec, error) } -type RenderMode string - -const ( - RenderModeRaw RenderMode = "raw" -) - type Spec struct { ManifestName string Path string OCIRef string - Mode RenderMode -} - -func DefaultSpec(path, ociref string, mode RenderMode) *CustomSpecFns { - return &CustomSpecFns{ - ManifestNameFn: func(_ context.Context, obj Object) string { return obj.GetName() }, - PathFn: func(_ context.Context, _ Object) string { return path }, - OCIRefFn: func(_ context.Context, _ Object) string { return ociref }, - ModeFn: func(_ context.Context, _ Object) RenderMode { return mode }, - } -} - -// CustomSpecFns is a simple static resolver that always uses the same chart and values. -type CustomSpecFns struct { - ManifestNameFn func(ctx context.Context, obj Object) string - PathFn func(ctx context.Context, obj Object) string - OCIRefFn func(ctx context.Context, obj Object) string - ModeFn func(ctx context.Context, obj Object) RenderMode -} - -func (s *CustomSpecFns) Spec(ctx context.Context, obj Object) (*Spec, error) { - return &Spec{ - ManifestName: s.ManifestNameFn(ctx, obj), - Path: s.PathFn(ctx, obj), - OCIRef: s.OCIRefFn(ctx, obj), - Mode: s.ModeFn(ctx, obj), - }, nil } diff --git a/internal/manifest/cr_deletion_check.go b/internal/manifest/cr_deletion_check.go deleted file mode 100644 index d694dcd83a..0000000000 --- a/internal/manifest/cr_deletion_check.go +++ /dev/null @@ -1,55 +0,0 @@ -package manifest - -import ( - "context" - "fmt" - - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime/schema" - "sigs.k8s.io/controller-runtime/pkg/client" - - "github.com/kyma-project/lifecycle-manager/api/v1beta2" - declarativev2 "github.com/kyma-project/lifecycle-manager/internal/declarative/v2" - "github.com/kyma-project/lifecycle-manager/pkg/util" -) - -// NewModuleCRDeletionCheck creates a check that verifies that the Resource CR in the remote cluster is deleted. -func NewModuleCRDeletionCheck() *ModuleCRDeletionCheck { - return &ModuleCRDeletionCheck{} -} - -type ModuleCRDeletionCheck struct{} - -func (c *ModuleCRDeletionCheck) Run( - ctx context.Context, - clnt client.Client, - obj declarativev2.Object, -) (bool, error) { - manifest, ok := obj.(*v1beta2.Manifest) - if !ok { - return false, v1beta2.ErrTypeAssertManifest - } - if manifest.Spec.Resource == nil { - return true, nil - } - - name := manifest.Spec.Resource.GetName() - namespace := manifest.Spec.Resource.GetNamespace() - gvk := manifest.Spec.Resource.GroupVersionKind() - - resourceCR := &unstructured.Unstructured{} - resourceCR.SetGroupVersionKind(schema.GroupVersionKind{ - Group: gvk.Group, - Version: gvk.Version, - Kind: gvk.Kind, - }) - - if err := clnt.Get(ctx, - client.ObjectKey{Name: name, Namespace: namespace}, resourceCR); err != nil { - if util.IsNotFound(err) { - return true, nil - } - return false, fmt.Errorf("%w: failed to fetch default resource CR", err) - } - return false, nil -} diff --git a/internal/manifest/custom_resource.go b/internal/manifest/custom_resource.go index 6f9513b6ce..b97f9f2323 100644 --- a/internal/manifest/custom_resource.go +++ b/internal/manifest/custom_resource.go @@ -31,7 +31,7 @@ func PostRunCreateCR( } resource := manifest.Spec.Resource.DeepCopy() - err := skr.Create(ctx, resource, client.FieldOwner(declarativev2.CustomResourceManager)) + err := skr.Create(ctx, resource, client.FieldOwner(declarativev2.CustomResourceManagerFinalizer)) if err != nil && !apierrors.IsAlreadyExists(err) { return fmt.Errorf("failed to create resource: %w", err) } @@ -41,9 +41,9 @@ func PostRunCreateCR( oMeta.SetGroupVersionKind(obj.GetObjectKind().GroupVersionKind()) oMeta.SetNamespace(obj.GetNamespace()) oMeta.SetFinalizers(obj.GetFinalizers()) - if added := controllerutil.AddFinalizer(oMeta, declarativev2.CustomResourceManager); added { + if added := controllerutil.AddFinalizer(oMeta, declarativev2.CustomResourceManagerFinalizer); added { if err := kcp.Patch( - ctx, oMeta, client.Apply, client.ForceOwnership, client.FieldOwner(declarativev2.CustomResourceManager), + ctx, oMeta, client.Apply, client.ForceOwnership, client.FieldOwner(declarativev2.CustomResourceManagerFinalizer), ); err != nil { return fmt.Errorf("failed to patch resource: %w", err) } @@ -83,9 +83,9 @@ func PreDeleteDeleteCR( if err != nil { return fmt.Errorf("failed to fetch resource: %w", err) } - if removed := controllerutil.RemoveFinalizer(onCluster, declarativev2.CustomResourceManager); removed { + if removed := controllerutil.RemoveFinalizer(onCluster, declarativev2.CustomResourceManagerFinalizer); removed { if err := kcp.Update( - ctx, onCluster, client.FieldOwner(declarativev2.CustomResourceManager), + ctx, onCluster, client.FieldOwner(declarativev2.CustomResourceManagerFinalizer), ); err != nil { return fmt.Errorf("failed to update resource: %w", err) } diff --git a/internal/manifest/spec_resolver.go b/internal/manifest/spec_resolver.go index 1066e3b9d4..c13362c1aa 100644 --- a/internal/manifest/spec_resolver.go +++ b/internal/manifest/spec_resolver.go @@ -4,8 +4,6 @@ import ( "context" "errors" "fmt" - "os" - "reflect" "github.com/google/go-containerregistry/pkg/authn" "github.com/google/go-containerregistry/pkg/v1/google" @@ -17,74 +15,32 @@ import ( "github.com/kyma-project/lifecycle-manager/pkg/ocmextensions" ) -// RawManifestInfo defines raw manifest information. -type RawManifestInfo struct { - Path string - OCIRef string -} - type SpecResolver struct { - KCP *declarativev2.ClusterInfo + kcpClient client.Client manifestPathExtractor *PathExtractor - ChartCache string - cachedCharts map[string]string } -func NewSpecResolver(kcp *declarativev2.ClusterInfo, extractor *PathExtractor) *SpecResolver { +func NewSpecResolver(kcpClient client.Client, extractor *PathExtractor) *SpecResolver { return &SpecResolver{ - KCP: kcp, + kcpClient: kcpClient, manifestPathExtractor: extractor, - ChartCache: os.TempDir(), - cachedCharts: make(map[string]string), } } -var ( - ErrRenderModeInvalid = errors.New("render mode is invalid") - ErrInvalidObjectPassedToSpecResolution = errors.New("invalid object passed to spec resolution") -) - -func (s *SpecResolver) Spec(ctx context.Context, obj declarativev2.Object) (*declarativev2.Spec, error) { - manifest, ok := obj.(*v1beta2.Manifest) - if !ok { - return nil, fmt.Errorf( - "invalid type %s: %w", reflect.TypeOf(obj), - ErrInvalidObjectPassedToSpecResolution, - ) - } +var errRenderModeInvalid = errors.New("render mode is invalid") +func (s *SpecResolver) GetSpec(ctx context.Context, manifest *v1beta2.Manifest) (*declarativev2.Spec, error) { var imageSpec v1beta2.ImageSpec if err := yaml.Unmarshal(manifest.Spec.Install.Source.Raw, &imageSpec); err != nil { return nil, fmt.Errorf("failed to unmarshal data: %w", err) } - var mode declarativev2.RenderMode - switch imageSpec.Type { - case v1beta2.OciRefType: - mode = declarativev2.RenderModeRaw - default: + if imageSpec.Type != v1beta2.OciRefType { return nil, fmt.Errorf("could not determine render mode for %s: %w", - client.ObjectKeyFromObject(manifest), ErrRenderModeInvalid) + client.ObjectKeyFromObject(manifest), errRenderModeInvalid) } - rawManifestInfo, err := s.getRawManifestForInstall(ctx, imageSpec, s.KCP.Client) - if err != nil { - return nil, err - } - - return &declarativev2.Spec{ - ManifestName: manifest.Spec.Install.Name, - Path: rawManifestInfo.Path, - OCIRef: rawManifestInfo.OCIRef, - Mode: mode, - }, nil -} - -func (s *SpecResolver) getRawManifestForInstall(ctx context.Context, - imageSpec v1beta2.ImageSpec, - targetClient client.Client, -) (*RawManifestInfo, error) { - keyChain, err := s.lookupKeyChain(ctx, imageSpec, targetClient) + keyChain, err := s.lookupKeyChain(ctx, imageSpec) if err != nil { return nil, fmt.Errorf("failed to fetch keyChain: %w", err) } @@ -93,23 +49,22 @@ func (s *SpecResolver) getRawManifestForInstall(ctx context.Context, if err != nil { return nil, fmt.Errorf("failed to extract raw manifest from layer digest: %w", err) } - return &RawManifestInfo{ - Path: rawManifestPath, - OCIRef: imageSpec.Ref, + + return &declarativev2.Spec{ + ManifestName: manifest.Spec.Install.Name, + Path: rawManifestPath, + OCIRef: imageSpec.Ref, }, nil } -func (s *SpecResolver) lookupKeyChain( - ctx context.Context, imageSpec v1beta2.ImageSpec, targetClient client.Client, -) (authn.Keychain, error) { +func (s *SpecResolver) lookupKeyChain(ctx context.Context, imageSpec v1beta2.ImageSpec) (authn.Keychain, error) { var keyChain authn.Keychain var err error - if imageSpec.CredSecretSelector != nil { - if keyChain, err = ocmextensions.GetAuthnKeychain(ctx, imageSpec.CredSecretSelector, targetClient); err != nil { - return nil, err - } - } else { + if imageSpec.CredSecretSelector == nil { keyChain = authn.DefaultKeychain + } else if keyChain, err = ocmextensions.GetAuthnKeychain(ctx, imageSpec.CredSecretSelector, s.kcpClient); err != nil { + return nil, err } + return authn.NewMultiKeychain(google.Keychain, keyChain), nil } diff --git a/tests/integration/controller/manifest/custom_resource_check/suite_test.go b/tests/integration/controller/manifest/custom_resource_check/suite_test.go index 2d4ee5533f..39ae7f4576 100644 --- a/tests/integration/controller/manifest/custom_resource_check/suite_test.go +++ b/tests/integration/controller/manifest/custom_resource_check/suite_test.go @@ -136,14 +136,13 @@ var _ = BeforeSuite(func() { kcp := &declarativev2.ClusterInfo{Config: cfg, Client: kcpClient} extractor := manifest.NewPathExtractor(nil) - reconciler = declarativev2.NewFromManager(mgr, &v1beta2.Manifest{}, queue.RequeueIntervals{ + reconciler = declarativev2.NewFromManager(mgr, queue.RequeueIntervals{ Success: 1 * time.Second, Error: 1 * time.Second, }, metrics.NewManifestMetrics(metrics.NewSharedMetrics()), metrics.NewMandatoryModulesMetrics(), - declarativev2.WithSpecResolver( - manifest.NewSpecResolver(kcp, extractor), - ), declarativev2.WithRemoteTargetCluster( + manifest.NewSpecResolver(kcp.Client, extractor), + declarativev2.WithRemoteTargetCluster( func(_ context.Context, _ declarativev2.Object) (*declarativev2.ClusterInfo, error) { return &declarativev2.ClusterInfo{Config: authUser.Config()}, nil }, diff --git a/tests/integration/controller/manifest/suite_test.go b/tests/integration/controller/manifest/suite_test.go index 4990d4b878..f8ce48125e 100644 --- a/tests/integration/controller/manifest/suite_test.go +++ b/tests/integration/controller/manifest/suite_test.go @@ -133,13 +133,12 @@ var _ = BeforeSuite(func() { kcp := &declarativev2.ClusterInfo{Config: cfg, Client: kcpClient} extractor := manifest.NewPathExtractor(nil) - reconciler = declarativev2.NewFromManager(mgr, &v1beta2.Manifest{}, queue.RequeueIntervals{ + reconciler = declarativev2.NewFromManager(mgr, queue.RequeueIntervals{ Success: 1 * time.Second, Busy: 1 * time.Second, }, metrics.NewManifestMetrics(metrics.NewSharedMetrics()), metrics.NewMandatoryModulesMetrics(), - declarativev2.WithSpecResolver( - manifest.NewSpecResolver(kcp, extractor), - ), declarativev2.WithRemoteTargetCluster( + manifest.NewSpecResolver(kcp.Client, extractor), + declarativev2.WithRemoteTargetCluster( func(_ context.Context, _ declarativev2.Object) (*declarativev2.ClusterInfo, error) { return &declarativev2.ClusterInfo{Config: authUser.Config()}, nil }, From 7988224cbca714a08a108f843d9ae9139d403f8b Mon Sep 17 00:00:00 2001 From: Xin Ruan Date: Mon, 15 Jul 2024 15:49:35 +0200 Subject: [PATCH 08/14] chore: Update Protecode (#1683) update protecode --- sec-scanners-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sec-scanners-config.yaml b/sec-scanners-config.yaml index 958ea56d8b..973ad4bec0 100644 --- a/sec-scanners-config.yaml +++ b/sec-scanners-config.yaml @@ -1,7 +1,7 @@ module-name: lifecycle-manager protecode: - europe-docker.pkg.dev/kyma-project/prod/lifecycle-manager:latest - - europe-docker.pkg.dev/kyma-project/prod/lifecycle-manager:1.0.0 + - europe-docker.pkg.dev/kyma-project/prod/lifecycle-manager:1.1.0 whitesource: language: golang-mod exclude: From 618673e01cf462189cb2a1bbc326c2aad8a9bdd1 Mon Sep 17 00:00:00 2001 From: Xin Ruan Date: Thu, 18 Jul 2024 16:00:39 +0200 Subject: [PATCH 09/14] chore: Refactor NewCachedDescriptorProvider (#1695) * remove parameter for NewCachedDescriptorProvider * fix dead link * adjust unit test coverage * fix flaky test --- .golangci.yaml | 2 +- cmd/main.go | 9 ++++--- .../01-10-control-plane-quick-start.md | 4 ++- internal/descriptor/provider/provider.go | 19 ++++++-------- internal/descriptor/provider/provider_test.go | 25 ++++++++++--------- pkg/testutils/kyma.go | 5 ++++ pkg/testutils/moduletemplate.go | 4 +-- .../moduletemplate_crd_validation_test.go | 2 +- .../controller/eventfilters/suite_test.go | 5 ++-- .../controller/kcp/remote_sync_test.go | 5 ++-- .../integration/controller/kcp/suite_test.go | 9 +++---- .../integration/controller/kyma/suite_test.go | 2 +- .../mandatorymodule/deletion/suite_test.go | 2 +- .../installation/suite_test.go | 2 +- .../controller/withwatcher/suite_test.go | 2 +- unit-test-coverage.yaml | 2 +- 16 files changed, 52 insertions(+), 47 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index d5c184f292..a9d6a735c1 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -190,7 +190,7 @@ issues: linters: [ gci ] # Disable gci due to the test utilities dot import. - path: tests/integration/declarative/declarative_test.go linters: [ gci ] # Disable gci due to the test utilities dot import. - - path: tests/integration/controller/(controlplane|eventfilters|kyma|withwatcher|purge|mandatorymodule)/(.*)_test.go + - path: tests/integration/controller/(eventfilters|kyma|withwatcher|purge|mandatorymodule|kcp)/(.*)_test.go linters: [ gci ] # Disable gci due to the test utilities dot import. - linters: - importas diff --git a/cmd/main.go b/cmd/main.go index 45323217d1..c1ba929591 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -177,10 +177,11 @@ func setupManager(flagVar *flags.FlagVar, cacheOptions cache.Options, scheme *ma } sharedMetrics := metrics.NewSharedMetrics() - descriptorProvider := provider.NewCachedDescriptorProvider(nil) + descriptorProvider := provider.NewCachedDescriptorProvider() kymaMetrics := metrics.NewKymaMetrics(sharedMetrics) mandatoryModulesMetrics := metrics.NewMandatoryModulesMetrics() - setupKymaReconciler(mgr, descriptorProvider, skrContextProvider, eventRecorder, flagVar, options, skrWebhookManager, kymaMetrics, + setupKymaReconciler(mgr, descriptorProvider, skrContextProvider, eventRecorder, flagVar, options, skrWebhookManager, + kymaMetrics, setupLog) setupManifestReconciler(mgr, flagVar, options, sharedMetrics, mandatoryModulesMetrics, setupLog) setupMandatoryModuleReconciler(mgr, descriptorProvider, flagVar, options, mandatoryModulesMetrics, setupLog) @@ -315,7 +316,9 @@ func setupKymaReconciler(mgr ctrl.Manager, } } -func createSkrWebhookManager(mgr ctrl.Manager, skrContextFactory remote.SkrContextProvider, flagVar *flags.FlagVar) (*watcher.SKRWebhookManifestManager, error) { +func createSkrWebhookManager(mgr ctrl.Manager, skrContextFactory remote.SkrContextProvider, + flagVar *flags.FlagVar, +) (*watcher.SKRWebhookManifestManager, error) { caCertificateCache := watcher.NewCACertificateCache(flagVar.CaCertCacheTTL) config := watcher.SkrWebhookManagerConfig{ SKRWatcherPath: flagVar.WatcherResourcesPath, diff --git a/docs/user-tutorials/01-10-control-plane-quick-start.md b/docs/user-tutorials/01-10-control-plane-quick-start.md index d7506fe4f7..8cc8354d40 100644 --- a/docs/user-tutorials/01-10-control-plane-quick-start.md +++ b/docs/user-tutorials/01-10-control-plane-quick-start.md @@ -39,7 +39,9 @@ To use Lifecycle Manager in a local setup, you need the following prerequisites: kubectl apply -f https://raw.githubusercontent.com/prometheus-community/helm-charts/main/charts/kube-prometheus-stack/charts/crds/crds/crd-servicemonitors.yaml ``` -You can also follow the official [Prometheus Operator quick start](https://prometheus-operator.dev/docs/getting-started/quick-start/) guide to deploy a full set of Prometheus Operator features into your cluster. +You can also follow the +official [Prometheus Operator quick start](https://prometheus-operator.dev/docs/getting-started/) guide to deploy a full +set of Prometheus Operator features into your cluster. ### Deploy Lifecycle Manager diff --git a/internal/descriptor/provider/provider.go b/internal/descriptor/provider/provider.go index 138d10d3db..786dddae7a 100644 --- a/internal/descriptor/provider/provider.go +++ b/internal/descriptor/provider/provider.go @@ -17,17 +17,12 @@ var ( ) type CachedDescriptorProvider struct { - descriptorCache *cache.DescriptorCache + DescriptorCache *cache.DescriptorCache } -func NewCachedDescriptorProvider(descriptorCache *cache.DescriptorCache) *CachedDescriptorProvider { - if descriptorCache != nil { - return &CachedDescriptorProvider{ - descriptorCache: descriptorCache, - } - } +func NewCachedDescriptorProvider() *CachedDescriptorProvider { return &CachedDescriptorProvider{ - descriptorCache: cache.NewDescriptorCache(), + DescriptorCache: cache.NewDescriptorCache(), } } @@ -49,7 +44,7 @@ func (c *CachedDescriptorProvider) GetDescriptor(template *v1beta2.ModuleTemplat return desc, nil } key := cache.GenerateDescriptorKey(template) - descriptor := c.descriptorCache.Get(key) + descriptor := c.DescriptorCache.Get(key) if descriptor != nil { return descriptor, nil } @@ -75,7 +70,7 @@ func (c *CachedDescriptorProvider) Add(template *v1beta2.ModuleTemplate) error { return ErrTemplateNil } key := cache.GenerateDescriptorKey(template) - descriptor := c.descriptorCache.Get(key) + descriptor := c.DescriptorCache.Get(key) if descriptor != nil { return nil } @@ -83,7 +78,7 @@ func (c *CachedDescriptorProvider) Add(template *v1beta2.ModuleTemplate) error { if template.Spec.Descriptor.Object != nil { desc, ok := template.Spec.Descriptor.Object.(*v1beta2.Descriptor) if ok && desc != nil { - c.descriptorCache.Set(key, desc) + c.DescriptorCache.Set(key, desc) return nil } } @@ -101,6 +96,6 @@ func (c *CachedDescriptorProvider) Add(template *v1beta2.ModuleTemplate) error { return ErrTypeAssert } - c.descriptorCache.Set(key, descriptor) + c.DescriptorCache.Set(key, descriptor) return nil } diff --git a/internal/descriptor/provider/provider_test.go b/internal/descriptor/provider/provider_test.go index 3925d74772..b526e35bf0 100644 --- a/internal/descriptor/provider/provider_test.go +++ b/internal/descriptor/provider/provider_test.go @@ -14,7 +14,7 @@ import ( ) func TestGetDescriptor_OnEmptySpec_ReturnsErrDecode(t *testing.T) { - descriptorProvider := provider.NewCachedDescriptorProvider(nil) // assuming it handles nil cache internally + descriptorProvider := provider.NewCachedDescriptorProvider() template := &v1beta2.ModuleTemplate{} _, err := descriptorProvider.GetDescriptor(template) @@ -24,7 +24,7 @@ func TestGetDescriptor_OnEmptySpec_ReturnsErrDecode(t *testing.T) { } func TestAdd_OnNilTemplate_ReturnsErrTemplateNil(t *testing.T) { - descriptorProvider := provider.NewCachedDescriptorProvider(nil) + descriptorProvider := provider.NewCachedDescriptorProvider() err := descriptorProvider.Add(nil) @@ -33,7 +33,7 @@ func TestAdd_OnNilTemplate_ReturnsErrTemplateNil(t *testing.T) { } func TestGetDescriptor_OnNilTemplate_ReturnsErrTemplateNil(t *testing.T) { - descriptorProvider := provider.NewCachedDescriptorProvider(nil) + descriptorProvider := provider.NewCachedDescriptorProvider() _, err := descriptorProvider.GetDescriptor(nil) @@ -42,7 +42,7 @@ func TestGetDescriptor_OnNilTemplate_ReturnsErrTemplateNil(t *testing.T) { } func TestGetDescriptor_OnInvalidRawDescriptor_ReturnsErrDescriptorNil(t *testing.T) { - descriptorProvider := provider.NewCachedDescriptorProvider(nil) + descriptorProvider := provider.NewCachedDescriptorProvider() template := builder.NewModuleTemplateBuilder().WithRawDescriptor([]byte("invalid descriptor")).WithDescriptor(nil).Build() _, err := descriptorProvider.GetDescriptor(template) @@ -52,8 +52,7 @@ func TestGetDescriptor_OnInvalidRawDescriptor_ReturnsErrDescriptorNil(t *testing } func TestGetDescriptor_OnEmptyCache_ReturnsParsedDescriptor(t *testing.T) { - descriptorCache := cache.NewDescriptorCache() - descriptorProvider := provider.NewCachedDescriptorProvider(descriptorCache) + descriptorProvider := provider.NewCachedDescriptorProvider() template := builder.NewModuleTemplateBuilder().Build() _, err := descriptorProvider.GetDescriptor(template) @@ -62,7 +61,7 @@ func TestGetDescriptor_OnEmptyCache_ReturnsParsedDescriptor(t *testing.T) { } func TestAdd_OnInvalidRawDescriptor_ReturnsErrDecode(t *testing.T) { - descriptorProvider := provider.NewCachedDescriptorProvider(nil) + descriptorProvider := provider.NewCachedDescriptorProvider() template := builder.NewModuleTemplateBuilder().WithRawDescriptor([]byte("invalid descriptor")).WithDescriptor(nil).Build() err := descriptorProvider.Add(template) @@ -72,7 +71,7 @@ func TestAdd_OnInvalidRawDescriptor_ReturnsErrDecode(t *testing.T) { } func TestAdd_OnDescriptorTypeButNull_ReturnsNoError(t *testing.T) { - descriptorProvider := provider.NewCachedDescriptorProvider(nil) + descriptorProvider := provider.NewCachedDescriptorProvider() template := builder.NewModuleTemplateBuilder().WithDescriptor(&v1beta2.Descriptor{}).Build() err := descriptorProvider.Add(template) @@ -82,12 +81,14 @@ func TestAdd_OnDescriptorTypeButNull_ReturnsNoError(t *testing.T) { func TestGetDescriptor_OnEmptyCache_AddsDescriptorFromTemplate(t *testing.T) { descriptorCache := cache.NewDescriptorCache() - descriptorProvider := provider.NewCachedDescriptorProvider(descriptorCache) + descriptorProvider := provider.CachedDescriptorProvider{DescriptorCache: descriptorCache} expected := &v1beta2.Descriptor{ - ComponentDescriptor: &compdesc.ComponentDescriptor{Metadata: compdesc.Metadata{ - ConfiguredVersion: "v2", - }}, + ComponentDescriptor: &compdesc.ComponentDescriptor{ + Metadata: compdesc.Metadata{ + ConfiguredVersion: "v2", + }, + }, } template := builder.NewModuleTemplateBuilder().WithDescriptor(expected).Build() diff --git a/pkg/testutils/kyma.go b/pkg/testutils/kyma.go index 889de1f954..2a39eb5175 100644 --- a/pkg/testutils/kyma.go +++ b/pkg/testutils/kyma.go @@ -139,6 +139,11 @@ func EnableModule(ctx context.Context, if err != nil { return err } + for _, enabledModule := range kyma.Spec.Modules { + if enabledModule.Name == module.Name { + return nil + } + } kyma.Spec.Modules = append( kyma.Spec.Modules, module) err = clnt.Update(ctx, kyma) diff --git a/pkg/testutils/moduletemplate.go b/pkg/testutils/moduletemplate.go index 91f82fea18..b820dc36f9 100644 --- a/pkg/testutils/moduletemplate.go +++ b/pkg/testutils/moduletemplate.go @@ -18,7 +18,7 @@ func GetModuleTemplate(ctx context.Context, module v1beta2.Module, defaultChannel string, ) (*v1beta2.ModuleTemplate, error) { - descriptorProvider := provider.NewCachedDescriptorProvider(nil) + descriptorProvider := provider.NewCachedDescriptorProvider() templateLookup := templatelookup.NewTemplateLookup(clnt, descriptorProvider) templateInfo := templateLookup.GetAndValidate(ctx, module.Name, module.Channel, defaultChannel) if templateInfo.Err != nil { @@ -94,7 +94,7 @@ func ReadModuleVersionFromModuleTemplate(ctx context.Context, clnt client.Client return "", fmt.Errorf("failed to fetch ModuleTemplate: %w", err) } - descriptorProvider := provider.NewCachedDescriptorProvider(nil) + descriptorProvider := provider.NewCachedDescriptorProvider() ocmDesc, err := descriptorProvider.GetDescriptor(moduleTemplate) if err != nil { return "", fmt.Errorf("failed to get descriptor: %w", err) diff --git a/tests/integration/apiwebhook/moduletemplate_crd_validation_test.go b/tests/integration/apiwebhook/moduletemplate_crd_validation_test.go index 90d29e25de..843c13ab3c 100644 --- a/tests/integration/apiwebhook/moduletemplate_crd_validation_test.go +++ b/tests/integration/apiwebhook/moduletemplate_crd_validation_test.go @@ -77,7 +77,7 @@ var _ = Describe("Webhook ValidationCreate Strict", Ordered, func() { WithChannel(v1beta2.DefaultChannel). WithOCM(compdescv2.SchemaVersion).Build() Expect(k8sClient.Create(webhookServerContext, template)).Should(Succeed()) - descriptorProvider := provider.NewCachedDescriptorProvider(nil) + descriptorProvider := provider.NewCachedDescriptorProvider() descriptor, err := descriptorProvider.GetDescriptor(template) Expect(err).ToNot(HaveOccurred()) version, err := semver.NewVersion(descriptor.Version) diff --git a/tests/integration/controller/eventfilters/suite_test.go b/tests/integration/controller/eventfilters/suite_test.go index a249b7a928..cb2628c55e 100644 --- a/tests/integration/controller/eventfilters/suite_test.go +++ b/tests/integration/controller/eventfilters/suite_test.go @@ -34,6 +34,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/manager" metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" + "sigs.k8s.io/controller-runtime/pkg/client" + "github.com/kyma-project/lifecycle-manager/api" "github.com/kyma-project/lifecycle-manager/api/shared" "github.com/kyma-project/lifecycle-manager/internal" @@ -47,7 +49,6 @@ import ( "github.com/kyma-project/lifecycle-manager/pkg/queue" testskrcontext "github.com/kyma-project/lifecycle-manager/pkg/testutils/skrcontextimpl" "github.com/kyma-project/lifecycle-manager/tests/integration" - "sigs.k8s.io/controller-runtime/pkg/client" _ "github.com/open-component-model/ocm/pkg/contexts/ocm" @@ -142,7 +143,7 @@ var _ = BeforeSuite(func() { SkrContextFactory: testSkrContextFactory, Event: testEventRec, RequeueIntervals: intervals, - DescriptorProvider: provider.NewCachedDescriptorProvider(nil), + DescriptorProvider: provider.NewCachedDescriptorProvider(), SyncRemoteCrds: remote.NewSyncCrdsUseCase(kcpClient, testSkrContextFactory, nil), InKCPMode: false, RemoteSyncNamespace: flags.DefaultRemoteSyncNamespace, diff --git a/tests/integration/controller/kcp/remote_sync_test.go b/tests/integration/controller/kcp/remote_sync_test.go index 9fca8e7469..788ac42a85 100644 --- a/tests/integration/controller/kcp/remote_sync_test.go +++ b/tests/integration/controller/kcp/remote_sync_test.go @@ -15,9 +15,10 @@ import ( "github.com/kyma-project/lifecycle-manager/internal/pkg/flags" "github.com/kyma-project/lifecycle-manager/pkg/testutils/builder" - . "github.com/kyma-project/lifecycle-manager/pkg/testutils" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + + . "github.com/kyma-project/lifecycle-manager/pkg/testutils" ) var ( @@ -223,7 +224,7 @@ func buildSkrKyma() *v1beta2.Kyma { func IsDescriptorCached(template *v1beta2.ModuleTemplate) bool { key := cache.GenerateDescriptorKey(template) - result := descriptorCache.Get(key) + result := descriptorProvider.DescriptorCache.Get(key) return result != nil } diff --git a/tests/integration/controller/kcp/suite_test.go b/tests/integration/controller/kcp/suite_test.go index 3b98f072ab..7e1a8d2118 100644 --- a/tests/integration/controller/kcp/suite_test.go +++ b/tests/integration/controller/kcp/suite_test.go @@ -40,7 +40,6 @@ import ( "github.com/kyma-project/lifecycle-manager/internal" "github.com/kyma-project/lifecycle-manager/internal/controller/kyma" "github.com/kyma-project/lifecycle-manager/internal/crd" - "github.com/kyma-project/lifecycle-manager/internal/descriptor/cache" "github.com/kyma-project/lifecycle-manager/internal/descriptor/provider" "github.com/kyma-project/lifecycle-manager/internal/event" "github.com/kyma-project/lifecycle-manager/internal/pkg/flags" @@ -48,12 +47,12 @@ import ( "github.com/kyma-project/lifecycle-manager/internal/remote" "github.com/kyma-project/lifecycle-manager/pkg/log" "github.com/kyma-project/lifecycle-manager/pkg/queue" + "github.com/kyma-project/lifecycle-manager/pkg/testutils" testskrcontext "github.com/kyma-project/lifecycle-manager/pkg/testutils/skrcontextimpl" "github.com/kyma-project/lifecycle-manager/tests/integration" _ "github.com/open-component-model/ocm/pkg/contexts/ocm" - . "github.com/kyma-project/lifecycle-manager/pkg/testutils" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -71,7 +70,6 @@ var ( ctx context.Context cancel context.CancelFunc cfg *rest.Config - descriptorCache *cache.DescriptorCache descriptorProvider *provider.CachedDescriptorProvider crdCache *crd.Cache ) @@ -88,7 +86,7 @@ var _ = BeforeSuite(func() { var err error By("bootstrapping test environment") - externalCRDs, err := AppendExternalCRDs( + externalCRDs, err := testutils.AppendExternalCRDs( filepath.Join(integration.GetProjectRoot(), "config", "samples", "tests", "crds"), "cert-manager-v1.10.1.crds.yaml", "istio-v1.17.1.crds.yaml") @@ -140,8 +138,7 @@ var _ = BeforeSuite(func() { testEventRec := event.NewRecorderWrapper(mgr.GetEventRecorderFor(shared.OperatorName)) testSkrContextFactory = testskrcontext.NewDualClusterFactory(kcpClient.Scheme(), testEventRec) - descriptorCache = cache.NewDescriptorCache() - descriptorProvider = provider.NewCachedDescriptorProvider(descriptorCache) + descriptorProvider = provider.NewCachedDescriptorProvider() crdCache = crd.NewCache(nil) err = (&kyma.Reconciler{ Client: kcpClient, diff --git a/tests/integration/controller/kyma/suite_test.go b/tests/integration/controller/kyma/suite_test.go index 5920854b7b..5d110d07d9 100644 --- a/tests/integration/controller/kyma/suite_test.go +++ b/tests/integration/controller/kyma/suite_test.go @@ -130,7 +130,7 @@ var _ = BeforeSuite(func() { Warning: 100 * time.Millisecond, } - descriptorProvider = provider.NewCachedDescriptorProvider(nil) + descriptorProvider = provider.NewCachedDescriptorProvider() kcpClient = mgr.GetClient() testEventRec := event.NewRecorderWrapper(mgr.GetEventRecorderFor(shared.OperatorName)) testSkrContextFactory := testskrcontext.NewSingleClusterFactory(kcpClient, mgr.GetConfig(), testEventRec) diff --git a/tests/integration/controller/mandatorymodule/deletion/suite_test.go b/tests/integration/controller/mandatorymodule/deletion/suite_test.go index 1795888e62..15163b843f 100644 --- a/tests/integration/controller/mandatorymodule/deletion/suite_test.go +++ b/tests/integration/controller/mandatorymodule/deletion/suite_test.go @@ -115,7 +115,7 @@ var _ = BeforeSuite(func() { Warning: 100 * time.Millisecond, } - descriptorProvider := provider.NewCachedDescriptorProvider(nil) + descriptorProvider := provider.NewCachedDescriptorProvider() reconciler = &mandatorymodule.DeletionReconciler{ Client: mgr.GetClient(), Event: event.NewRecorderWrapper(mgr.GetEventRecorderFor(shared.OperatorName)), diff --git a/tests/integration/controller/mandatorymodule/installation/suite_test.go b/tests/integration/controller/mandatorymodule/installation/suite_test.go index 781eea10f0..23240dfd7a 100644 --- a/tests/integration/controller/mandatorymodule/installation/suite_test.go +++ b/tests/integration/controller/mandatorymodule/installation/suite_test.go @@ -106,7 +106,7 @@ var _ = BeforeSuite(func() { Warning: 100 * time.Millisecond, } - descriptorProvider := provider.NewCachedDescriptorProvider(nil) + descriptorProvider := provider.NewCachedDescriptorProvider() reconciler = &mandatorymodule.InstallationReconciler{ Client: mgr.GetClient(), DescriptorProvider: descriptorProvider, diff --git a/tests/integration/controller/withwatcher/suite_test.go b/tests/integration/controller/withwatcher/suite_test.go index 65902930a8..3241563bfa 100644 --- a/tests/integration/controller/withwatcher/suite_test.go +++ b/tests/integration/controller/withwatcher/suite_test.go @@ -213,7 +213,7 @@ var _ = BeforeSuite(func() { Event: testEventRec, RequeueIntervals: intervals, SKRWebhookManager: skrWebhookChartManager, - DescriptorProvider: provider.NewCachedDescriptorProvider(nil), + DescriptorProvider: provider.NewCachedDescriptorProvider(), SyncRemoteCrds: remote.NewSyncCrdsUseCase(kcpClient, testSkrContextFactory, nil), RemoteSyncNamespace: flags.DefaultRemoteSyncNamespace, InKCPMode: true, diff --git a/unit-test-coverage.yaml b/unit-test-coverage.yaml index ab5f80c47b..6824037cfa 100644 --- a/unit-test-coverage.yaml +++ b/unit-test-coverage.yaml @@ -1,7 +1,7 @@ packages: internal/crd: 92 internal/descriptor/cache: 93 - internal/descriptor/provider: 68 + internal/descriptor/provider: 66 internal/event: 100 internal/manifest/filemutex: 100 internal/istio: 63 From 895587dfb0c3889a28f8beae1d749271366adef5 Mon Sep 17 00:00:00 2001 From: Amritanshu Sikdar Date: Fri, 19 Jul 2024 13:48:38 +0200 Subject: [PATCH 10/14] docs: Update KLM Local Test Setup Guide (#1680) fix errors in local test setup documentation add version info --- docs/developer-tutorials/local-test-setup.md | 63 +++++++++++--------- 1 file changed, 36 insertions(+), 27 deletions(-) diff --git a/docs/developer-tutorials/local-test-setup.md b/docs/developer-tutorials/local-test-setup.md index 0dfb213166..f4ef4f6c6c 100644 --- a/docs/developer-tutorials/local-test-setup.md +++ b/docs/developer-tutorials/local-test-setup.md @@ -1,5 +1,17 @@ # Local test Setup in the control-plane Mode Using k3d +> ### Supported Versions +> * Golang: `v1.22.5` +> * k3d: `v5.6.0` +> * k3s: `v1.27.4-k3s1 (default)` +> * kubectl: +> * Client Version: `v1.30.2` +> * Server Version: `v1.27.4+k3s1` +> * docker: +> * Client Version: `v26.1.4` +> * Server: `Docker Desktop v4.31.0` +> * Engine Version: `v26.1.4` + ## Context This tutorial shows how to configure a fully working e2e test setup including the following components: @@ -184,36 +196,33 @@ k3d cluster create skr-local Running Lifecycle Manager on a local machine and not on a cluster If you are running Lifecycle Manager on your local machine and not as a deployment on a cluster, use the following to create a Kyma CR and Secret: - ```shell - cat << EOF | kubectl apply -f - - --- - apiVersion: v1 - kind: Secret - metadata: - name: kyma-sample - namespace: kcp-system - labels: + ```shell + cat << EOF | kubectl apply -f - + --- + apiVersion: v1 + kind: Secret + metadata: + name: kyma-sample + namespace: kcp-system + labels: "operator.kyma-project.io/kyma-name": "kyma-sample" "operator.kyma-project.io/managed-by": "lifecycle-manager" - data: - config: $(k3d kubeconfig get skr-local | base64 | tr -d '\n') - --- - apiVersion: operator.kyma-project.io/v1beta2 - kind: Kyma - metadata: - annotations: - skr-domain: "example.domain.com" - name: kyma-sample - namespace: kcp-system - spec: - channel: regular - sync: - enabled: true - modules: - - name: template-operator - EOF + data: + config: $(k3d kubeconfig get skr-local | base64 | tr -d '\n') + --- + apiVersion: operator.kyma-project.io/v1beta2 + kind: Kyma + metadata: + annotations: + skr-domain: "example.domain.com" + name: kyma-sample + namespace: kcp-system + spec: + channel: regular + modules: + - name: template-operator + EOF ``` - ### Watcher and Module Installation Verification From d5ea47444a096f9371af18d160a3e6eb867e451e Mon Sep 17 00:00:00 2001 From: Amritanshu Sikdar Date: Fri, 19 Jul 2024 15:24:38 +0200 Subject: [PATCH 11/14] feat: Drop multiple ways to reference modules in Kyma CR (#1672) * remove module reference by namespace/name * remove module reference by objectmeta name * remove module reference by FQDN * add initial test structure * add test cases for different module reference scenarios * fix tests * update documentation * address review comments * address more review comments * fix linting issues * rearrange imports * adjust documentation --- api/v1beta2/kyma_types.go | 5 +- .../bases/operator.kyma-project.io_kymas.yaml | 10 +--- docs/technical-reference/api/kyma-cr.md | 47 ++++--------------- pkg/templatelookup/regular.go | 19 +------- .../controller/kyma/manifest_test.go | 44 +++++++++++++++-- 5 files changed, 54 insertions(+), 71 deletions(-) diff --git a/api/v1beta2/kyma_types.go b/api/v1beta2/kyma_types.go index f836bc0ecb..dff905ad65 100644 --- a/api/v1beta2/kyma_types.go +++ b/api/v1beta2/kyma_types.go @@ -61,10 +61,7 @@ type Module struct { // Name is a unique identifier of the module. // It is used to resolve a ModuleTemplate for creating a set of resources on the cluster. // - // Name can be one of 3 kinds: - // - The ModuleName label value of the module-template, e.g. operator.kyma-project.io/module-name=my-module - // - The Name or Namespace/Name of a ModuleTemplate, e.g. my-moduletemplate or kyma-system/my-moduletemplate - // - The FQDN, e.g. kyma-project.io/module/my-module as located in .spec.descriptor.component.name + // Name can only be the ModuleName label value of the module-template, e.g. operator.kyma-project.io/module-name=my-module Name string `json:"name"` // ControllerName is able to set the controller used for reconciliation of the module. It can be used diff --git a/config/crd/bases/operator.kyma-project.io_kymas.yaml b/config/crd/bases/operator.kyma-project.io_kymas.yaml index 1b2d77b66a..304b0cc8cd 100644 --- a/config/crd/bases/operator.kyma-project.io_kymas.yaml +++ b/config/crd/bases/operator.kyma-project.io_kymas.yaml @@ -90,10 +90,7 @@ spec: It is used to resolve a ModuleTemplate for creating a set of resources on the cluster. - Name can be one of 3 kinds: - - The ModuleName label value of the module-template, e.g. operator.kyma-project.io/module-name=my-module - - The Name or Namespace/Name of a ModuleTemplate, e.g. my-moduletemplate or kyma-system/my-moduletemplate - - The FQDN, e.g. kyma-project.io/module/my-module as located in .spec.descriptor.component.name + Name can only be the ModuleName label value of the module-template, e.g. operator.kyma-project.io/module-name=my-module type: string remoteModuleTemplateRef: description: |- @@ -526,10 +523,7 @@ spec: It is used to resolve a ModuleTemplate for creating a set of resources on the cluster. - Name can be one of 3 kinds: - - The ModuleName label value of the module-template, e.g. operator.kyma-project.io/module-name=my-module - - The Name or Namespace/Name of a ModuleTemplate, e.g. my-moduletemplate or kyma-system/my-moduletemplate - - The FQDN, e.g. kyma-project.io/module/my-module as located in .spec.descriptor.component.name + Name can only be the ModuleName label value of the module-template, e.g. operator.kyma-project.io/module-name=my-module type: string remoteModuleTemplateRef: description: |- diff --git a/docs/technical-reference/api/kyma-cr.md b/docs/technical-reference/api/kyma-cr.md index ddf9da8dfc..713c04248e 100644 --- a/docs/technical-reference/api/kyma-cr.md +++ b/docs/technical-reference/api/kyma-cr.md @@ -62,43 +62,16 @@ spec: name: kyma-project.io/module/sample ``` -The module mentioned above can be referenced in one of the following ways: - -* The label value of `operator.kyma-project.io/module-name`: - - ```yaml - spec: - channel: regular - modules: - - name: module-name-from-label - ``` - -* The name or namespace/name of a ModuleTemplate CR: - - ```yaml - spec: - channel: regular - modules: - - name: moduletemplate-sample - ``` - - or - - ```yaml - spec: - channel: regular - modules: - - name: default/moduletemplate-sample - ``` - -* The fully qualified name of the descriptor as located in **.spec.descriptor.component.name**: - - ```yaml - spec: - channel: regular - modules: - - name: kyma-project.io/module/sample - ``` +The module mentioned above can *only* be referenced using the label value of `operator.kyma-project.io/module-name`: +```yaml +spec: + channel: regular + modules: + - name: module-name-from-label +``` + +> **CAUTION:** +> Module referencing using NamespacedName and FQDN (Fully Qualified Domain Name) has been deprecated. ### **.spec.modules[].customResourcePolicy** diff --git a/pkg/templatelookup/regular.go b/pkg/templatelookup/regular.go index fc8c0221dc..ff58e6192d 100644 --- a/pkg/templatelookup/regular.go +++ b/pkg/templatelookup/regular.go @@ -138,7 +138,7 @@ func logUsedChannel(ctx context.Context, name string, actualChannel string, defa } func moduleMatch(moduleStatus *v1beta2.ModuleStatus, moduleName string) bool { - return moduleStatus.FQDN == moduleName || moduleStatus.Name == moduleName + return moduleStatus.Name == moduleName } // checkValidTemplateUpdate verifies if the given ModuleTemplate is valid for update and sets their IsValidUpdate Flag @@ -253,23 +253,6 @@ func (t *TemplateLookup) getTemplate(ctx context.Context, clnt client.Reader, na filteredTemplates = append(filteredTemplates, &template) continue } - if fmt.Sprintf("%s/%s", template.Namespace, template.Name) == name && - template.Spec.Channel == desiredChannel { - filteredTemplates = append(filteredTemplates, &template) - continue - } - if template.ObjectMeta.Name == name && template.Spec.Channel == desiredChannel { - filteredTemplates = append(filteredTemplates, &template) - continue - } - descriptor, err := t.descriptorProvider.GetDescriptor(&template) - if err != nil { - return nil, fmt.Errorf("invalid ModuleTemplate descriptor: %w", err) - } - if descriptor.Name == name && template.Spec.Channel == desiredChannel { - filteredTemplates = append(filteredTemplates, &template) - continue - } } if len(filteredTemplates) > 1 { diff --git a/tests/integration/controller/kyma/manifest_test.go b/tests/integration/controller/kyma/manifest_test.go index 01621d4d07..1c6bc56ffa 100644 --- a/tests/integration/controller/kyma/manifest_test.go +++ b/tests/integration/controller/kyma/manifest_test.go @@ -18,15 +18,15 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "sigs.k8s.io/controller-runtime/pkg/client" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - "github.com/kyma-project/lifecycle-manager/api/shared" "github.com/kyma-project/lifecycle-manager/api/v1beta2" "github.com/kyma-project/lifecycle-manager/internal/pkg/flags" + "github.com/kyma-project/lifecycle-manager/pkg/templatelookup" + "github.com/kyma-project/lifecycle-manager/pkg/testutils/builder" . "github.com/kyma-project/lifecycle-manager/pkg/testutils" - "github.com/kyma-project/lifecycle-manager/pkg/testutils/builder" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" ) const ( @@ -329,6 +329,42 @@ var _ = Describe("Test Reconciliation Skip label for Manifest", Ordered, func() }) }) +var _ = Describe("Modules can only be referenced via module name", Ordered, func() { + kyma := NewTestKyma("random-kyma") + + moduleReferencedWithLabel := NewTestModuleWithFixName("random-module", v1beta2.DefaultChannel) + moduleReferencedWithNamespacedName := NewTestModuleWithFixName( + v1beta2.DefaultChannel+shared.Separator+"random-module", v1beta2.DefaultChannel) + moduleReferencedWithFQDN := NewTestModuleWithFixName("kyma-project.io/module/"+"random-module", v1beta2.DefaultChannel) + + kyma.Spec.Modules = append(kyma.Spec.Modules, moduleReferencedWithLabel) + RegisterDefaultLifecycleForKyma(kyma) + + 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) + Expect(err).ToNot(HaveOccurred()) + + foundModuleName := moduleTemplate.Labels[shared.ModuleName] + Expect(foundModuleName).To(Equal(moduleReferencedWithLabel.Name)) + }) + }) + + Context("When operator is referenced by Namespace/Name", func() { + It("cannot find the operator", func() { + _, err := GetModuleTemplate(ctx, kcpClient, moduleReferencedWithNamespacedName, kyma.Spec.Channel) + 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) + Expect(err.Error()).Should(ContainSubstring(templatelookup.ErrNoTemplatesInListResult.Error())) + }) + }) +}) + func findRawManifestResource(reslist []compdesc.Resource) *compdesc.Resource { for _, r := range reslist { if r.Name == v1beta2.RawManifestLayerName { From ba2737464efdb9365cf7510ac4de981ba03fee7d Mon Sep 17 00:00:00 2001 From: Nesma Badr Date: Mon, 22 Jul 2024 11:46:40 +0200 Subject: [PATCH 12/14] chore: Configure different requeue intervals for Manifest reconciliation (#1690) * Add different requeue intervals for Manifest reconciliation * Empty-Commit * code review comments --- cmd/main.go | 4 +- internal/declarative/v2/reconciler.go | 41 ++++++++----------- internal/pkg/flags/flags.go | 15 +++++++ internal/pkg/flags/flags_test.go | 15 +++++++ pkg/queue/requeue_intervals.go | 6 +-- .../custom_resource_check/suite_test.go | 2 + .../controller/manifest/suite_test.go | 5 ++- 7 files changed, 59 insertions(+), 29 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index c1ba929591..45ad9ae23b 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -400,7 +400,9 @@ func setupManifestReconciler(mgr ctrl.Manager, flagVar *flags.FlagVar, options c if err := manifest.SetupWithManager( mgr, options, queue.RequeueIntervals{ Success: flagVar.ManifestRequeueSuccessInterval, - Busy: flagVar.KymaRequeueBusyInterval, + Busy: flagVar.ManifestRequeueBusyInterval, + Error: flagVar.ManifestRequeueErrInterval, + Warning: flagVar.ManifestRequeueWarningInterval, }, manifest.SetupOptions{ ListenerAddr: flagVar.ManifestListenerAddr, EnableDomainNameVerification: flagVar.EnableDomainNameVerification, diff --git a/internal/declarative/v2/reconciler.go b/internal/declarative/v2/reconciler.go index 91de58d41b..f7d93cb2d2 100644 --- a/internal/declarative/v2/reconciler.go +++ b/internal/declarative/v2/reconciler.go @@ -179,7 +179,8 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu return r.finishReconcile(ctx, manifest, metrics.ManifestRenderResources, manifestStatus, err) } - if err := r.pruneDiff(ctx, skrClient, manifest, current, target, spec); errors.Is(err, resources.ErrDeletionNotFinished) { + if err := r.pruneDiff(ctx, skrClient, manifest, current, target, spec); errors.Is(err, + resources.ErrDeletionNotFinished) { r.ManifestMetrics.RecordRequeueReason(metrics.ManifestPruneDiffNotFinished, queue.IntendedRequeue) return ctrl.Result{Requeue: true}, nil } else if err != nil { @@ -219,8 +220,8 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu return r.finishReconcile(ctx, manifest, metrics.ManifestReconcileFinished, manifestStatus, nil) } -func (r *Reconciler) cleanupManifest(ctx context.Context, req ctrl.Request, manifest *v1beta2.Manifest, manifestStatus shared.Status, - requeueReason metrics.ManifestRequeueReason, originalErr error, +func (r *Reconciler) cleanupManifest(ctx context.Context, req ctrl.Request, manifest *v1beta2.Manifest, + manifestStatus shared.Status, requeueReason metrics.ManifestRequeueReason, originalErr error, ) (ctrl.Result, error) { r.ManifestMetrics.RemoveManifestDuration(req.Name) r.cleanUpMandatoryModuleMetrics(manifest) @@ -299,10 +300,7 @@ func (r *Reconciler) initialize(manifest *v1beta2.Manifest) error { return nil } -func (r *Reconciler) renderResources( - ctx context.Context, - skrClient Client, - manifest *v1beta2.Manifest, +func (r *Reconciler) renderResources(ctx context.Context, skrClient Client, manifest *v1beta2.Manifest, spec *Spec, ) ([]*resource.Info, []*resource.Info, error) { resourceCondition := newResourcesCondition(manifest) @@ -392,9 +390,9 @@ func hasDiff(oldResources []shared.Resource, newResources []shared.Resource) boo return false } -func (r *Reconciler) checkDeploymentState( - ctx context.Context, clnt Client, target []*resource.Info, -) (shared.State, error) { +func (r *Reconciler) checkDeploymentState(ctx context.Context, clnt Client, target []*resource.Info) (shared.State, + error, +) { resourceReadyCheck := r.CustomReadyCheck deploymentState, err := resourceReadyCheck.Run(ctx, clnt, target) @@ -448,12 +446,8 @@ func (r *Reconciler) removeModuleCR(ctx context.Context, clnt Client, manifest * return nil } -func (r *Reconciler) renderTargetResources( - ctx context.Context, - skrClient client.Client, - converter ResourceToInfoConverter, - manifest *v1beta2.Manifest, - spec *Spec, +func (r *Reconciler) renderTargetResources(ctx context.Context, skrClient client.Client, + converter ResourceToInfoConverter, manifest *v1beta2.Manifest, spec *Spec, ) ([]*resource.Info, error) { if !manifest.GetDeletionTimestamp().IsZero() { deleted, err := checkCRDeletion(ctx, skrClient, manifest) @@ -515,12 +509,8 @@ func checkCRDeletion(ctx context.Context, skrClient client.Client, manifest *v1b return false, nil } -func (r *Reconciler) pruneDiff( - ctx context.Context, - clnt Client, - manifest *v1beta2.Manifest, - current, target []*resource.Info, - spec *Spec, +func (r *Reconciler) pruneDiff(ctx context.Context, clnt Client, manifest *v1beta2.Manifest, + current, target []*resource.Info, spec *Spec, ) error { diff, err := pruneResource(ResourceList(current).Difference(target), "Namespace", namespaceNotBeRemoved) if err != nil { @@ -652,10 +642,13 @@ func (r *Reconciler) finishReconcile(ctx context.Context, manifest *v1beta2.Mani return ctrl.Result{}, originalErr } r.ManifestMetrics.RecordRequeueReason(requeueReason, queue.IntendedRequeue) - return ctrl.Result{RequeueAfter: r.Success}, nil + requeueAfter := queue.DetermineRequeueInterval(manifest.GetStatus().State, r.RequeueIntervals) + return ctrl.Result{RequeueAfter: requeueAfter}, nil } -func (r *Reconciler) patchStatusIfDiffExist(ctx context.Context, manifest *v1beta2.Manifest, previousStatus shared.Status) error { +func (r *Reconciler) patchStatusIfDiffExist(ctx context.Context, manifest *v1beta2.Manifest, + previousStatus shared.Status, +) error { if hasStatusDiff(manifest.GetStatus(), previousStatus) { resetNonPatchableField(manifest) if err := r.Status().Patch(ctx, manifest, client.Apply, client.ForceOwnership, defaultFieldOwner); err != nil { diff --git a/internal/pkg/flags/flags.go b/internal/pkg/flags/flags.go index 374cda47cd..8e2c327ce6 100644 --- a/internal/pkg/flags/flags.go +++ b/internal/pkg/flags/flags.go @@ -16,6 +16,9 @@ const ( DefaultKymaRequeueWarningInterval = 30 * time.Second DefaultKymaRequeueBusyInterval = 5 * time.Second DefaultManifestRequeueSuccessInterval = 30 * time.Second + DefaultManifestRequeueErrInterval = 2 * time.Second + DefaultManifestRequeueWarningInterval = 30 * time.Second + DefaultManifestRequeueBusyInterval = 5 * time.Second DefaultMandatoryModuleRequeueSuccessInterval = 30 * time.Second DefaultMandatoryModuleDeletionRequeueSuccessInterval = 30 * time.Second DefaultWatcherRequeueSuccessInterval = 30 * time.Second @@ -119,6 +122,15 @@ func DefineFlagVar() *FlagVar { flag.DurationVar(&flagVar.ManifestRequeueSuccessInterval, "manifest-requeue-success-interval", DefaultManifestRequeueSuccessInterval, "determines the duration a Manifest in Ready state is enqueued for reconciliation.") + flag.DurationVar(&flagVar.ManifestRequeueErrInterval, "manifest-requeue-error-interval", + DefaultManifestRequeueErrInterval, + "determines the duration a Manifest in Error state is enqueued for reconciliation.") + flag.DurationVar(&flagVar.ManifestRequeueWarningInterval, "manifest-requeue-warning-interval", + DefaultManifestRequeueWarningInterval, + "determines the duration a Manifest in Warning state is enqueued for reconciliation.") + flag.DurationVar(&flagVar.ManifestRequeueBusyInterval, "manifest-requeue-busy-interval", + DefaultManifestRequeueBusyInterval, + "determines the duration a Manifest in Processing state is enqueued for reconciliation.") flag.DurationVar(&flagVar.MandatoryModuleDeletionRequeueSuccessInterval, "mandatory-module-deletion-requeue-success-interval", DefaultMandatoryModuleDeletionRequeueSuccessInterval, @@ -230,6 +242,9 @@ type FlagVar struct { KymaRequeueBusyInterval time.Duration KymaRequeueWarningInterval time.Duration ManifestRequeueSuccessInterval time.Duration + ManifestRequeueErrInterval time.Duration + ManifestRequeueBusyInterval time.Duration + ManifestRequeueWarningInterval time.Duration WatcherRequeueSuccessInterval time.Duration MandatoryModuleRequeueSuccessInterval time.Duration MandatoryModuleDeletionRequeueSuccessInterval time.Duration diff --git a/internal/pkg/flags/flags_test.go b/internal/pkg/flags/flags_test.go index 706d029974..8f34ec5364 100644 --- a/internal/pkg/flags/flags_test.go +++ b/internal/pkg/flags/flags_test.go @@ -43,6 +43,21 @@ func Test_ConstantFlags(t *testing.T) { constValue: DefaultManifestRequeueSuccessInterval.String(), expectedValue: (30 * time.Second).String(), }, + { + constName: "DefaultManifestRequeueErrInterval", + constValue: DefaultManifestRequeueErrInterval.String(), + expectedValue: (2 * time.Second).String(), + }, + { + constName: "DefaultManifestRequeueWarningInterval", + constValue: DefaultManifestRequeueWarningInterval.String(), + expectedValue: (30 * time.Second).String(), + }, + { + constName: "DefaultManifestRequeueBusyInterval", + constValue: DefaultManifestRequeueBusyInterval.String(), + expectedValue: (5 * time.Second).String(), + }, { constName: "DefaultMandatoryModuleRequeueSuccessInterval", constValue: DefaultMandatoryModuleRequeueSuccessInterval.String(), diff --git a/pkg/queue/requeue_intervals.go b/pkg/queue/requeue_intervals.go index 1b91a78a92..1eeb8d77e7 100644 --- a/pkg/queue/requeue_intervals.go +++ b/pkg/queue/requeue_intervals.go @@ -18,13 +18,13 @@ func DetermineRequeueInterval(state shared.State, intervals RequeueIntervals) ti case shared.StateError: return intervals.Error case shared.StateDeleting: - fallthrough + return intervals.Busy case shared.StateProcessing: return intervals.Busy - case shared.StateReady: - fallthrough case shared.StateWarning: return intervals.Warning + case shared.StateReady: + return intervals.Success default: return intervals.Success } diff --git a/tests/integration/controller/manifest/custom_resource_check/suite_test.go b/tests/integration/controller/manifest/custom_resource_check/suite_test.go index 39ae7f4576..9c1fcdb5c8 100644 --- a/tests/integration/controller/manifest/custom_resource_check/suite_test.go +++ b/tests/integration/controller/manifest/custom_resource_check/suite_test.go @@ -138,7 +138,9 @@ var _ = BeforeSuite(func() { extractor := manifest.NewPathExtractor(nil) reconciler = declarativev2.NewFromManager(mgr, queue.RequeueIntervals{ Success: 1 * time.Second, + Busy: 1 * time.Second, Error: 1 * time.Second, + Warning: 1 * time.Second, }, metrics.NewManifestMetrics(metrics.NewSharedMetrics()), metrics.NewMandatoryModulesMetrics(), manifest.NewSpecResolver(kcp.Client, extractor), diff --git a/tests/integration/controller/manifest/suite_test.go b/tests/integration/controller/manifest/suite_test.go index f8ce48125e..03aac994ce 100644 --- a/tests/integration/controller/manifest/suite_test.go +++ b/tests/integration/controller/manifest/suite_test.go @@ -134,7 +134,10 @@ var _ = BeforeSuite(func() { kcp := &declarativev2.ClusterInfo{Config: cfg, Client: kcpClient} extractor := manifest.NewPathExtractor(nil) reconciler = declarativev2.NewFromManager(mgr, queue.RequeueIntervals{ - Success: 1 * time.Second, Busy: 1 * time.Second, + Success: 1 * time.Second, + Busy: 1 * time.Second, + Error: 1 * time.Second, + Warning: 1 * time.Second, }, metrics.NewManifestMetrics(metrics.NewSharedMetrics()), metrics.NewMandatoryModulesMetrics(), manifest.NewSpecResolver(kcp.Client, extractor), From a3af93e8b635d59f3cb3bb21f6eb14b12fbecf66 Mon Sep 17 00:00:00 2001 From: Benjamin Lindner <50365642+lindnerby@users.noreply.github.com> Date: Mon, 22 Jul 2024 13:38:39 +0200 Subject: [PATCH 13/14] chore: Bump k8s deps (#1703) * chore: Bump k8s deps * retrigger jobs * bump api folder as well --------- Co-authored-by: Nesma Badr --- api/go.mod | 2 +- api/go.sum | 4 ++-- go.mod | 20 ++++++++++---------- go.sum | 40 ++++++++++++++++++++-------------------- 4 files changed, 33 insertions(+), 33 deletions(-) diff --git a/api/go.mod b/api/go.mod index 261b578a8d..250ed799eb 100644 --- a/api/go.mod +++ b/api/go.mod @@ -5,7 +5,7 @@ go 1.22.4 require ( github.com/Masterminds/semver/v3 v3.2.1 github.com/open-component-model/ocm v0.11.0 - k8s.io/apimachinery v0.30.2 + k8s.io/apimachinery v0.30.3 sigs.k8s.io/controller-runtime v0.18.4 ) diff --git a/api/go.sum b/api/go.sum index 671718c1ae..a1d22d3099 100644 --- a/api/go.sum +++ b/api/go.sum @@ -1052,8 +1052,8 @@ k8s.io/api v0.30.1 h1:kCm/6mADMdbAxmIh0LBjS54nQBE+U4KmbCfIkF5CpJY= k8s.io/api v0.30.1/go.mod h1:ddbN2C0+0DIiPntan/bye3SW3PdwLa11/0yqwvuRrJM= k8s.io/apiextensions-apiserver v0.30.1 h1:4fAJZ9985BmpJG6PkoxVRpXv9vmPUOVzl614xarePws= k8s.io/apiextensions-apiserver v0.30.1/go.mod h1:R4GuSrlhgq43oRY9sF2IToFh7PVlF1JjfWdoG3pixk4= -k8s.io/apimachinery v0.30.2 h1:fEMcnBj6qkzzPGSVsAZtQThU62SmQ4ZymlXRC5yFSCg= -k8s.io/apimachinery v0.30.2/go.mod h1:iexa2somDaxdnj7bha06bhb43Zpa6eWH8N8dbqVjTUc= +k8s.io/apimachinery v0.30.3 h1:q1laaWCmrszyQuSQCfNB8cFgCuDAoPszKY4ucAjDwHc= +k8s.io/apimachinery v0.30.3/go.mod h1:iexa2somDaxdnj7bha06bhb43Zpa6eWH8N8dbqVjTUc= k8s.io/client-go v0.30.1 h1:uC/Ir6A3R46wdkgCV3vbLyNOYyCJ8oZnjtJGKfytl/Q= k8s.io/client-go v0.30.1/go.mod h1:wrAqLNs2trwiCH/wxxmT/x3hKVH9PuV0GGW0oDoHVqc= k8s.io/klog/v2 v2.120.1 h1:QXU6cPEOIslTGvZaXvFWiP9VKyeet3sawzTOvdXb4Vw= diff --git a/go.mod b/go.mod index 6bd336fd4c..d7a9b4b03f 100644 --- a/go.mod +++ b/go.mod @@ -10,7 +10,7 @@ require ( github.com/go-logr/logr v1.4.2 github.com/go-logr/zapr v1.3.0 github.com/golang/mock v1.6.0 - github.com/google/go-containerregistry v0.20.0 + github.com/google/go-containerregistry v0.20.1 github.com/google/go-containerregistry/pkg/authn/kubernetes v0.0.0-20231202142526-55ffb0092afd github.com/jellydator/ttlcache/v3 v3.2.0 github.com/kyma-project/lifecycle-manager/api v0.0.0-00010101000000-000000000000 @@ -29,20 +29,20 @@ require ( ) require ( - istio.io/api v1.22.2 - istio.io/client-go v1.22.2 + istio.io/api v1.22.3 + istio.io/client-go v1.22.3 ) require ( github.com/go-co-op/gocron v1.37.0 github.com/kyma-project/template-operator/api v0.0.0-20240404131948-52c84f14e73c github.com/prometheus/client_model v0.6.1 - k8s.io/api v0.30.2 - k8s.io/apiextensions-apiserver v0.30.2 - k8s.io/apimachinery v0.30.2 - k8s.io/cli-runtime v0.30.2 - k8s.io/client-go v0.30.2 - k8s.io/kubectl v0.30.2 + k8s.io/api v0.30.3 + k8s.io/apiextensions-apiserver v0.30.3 + k8s.io/apimachinery v0.30.3 + k8s.io/cli-runtime v0.30.3 + k8s.io/client-go v0.30.3 + k8s.io/kubectl v0.30.3 ) require ( @@ -319,7 +319,7 @@ require ( gopkg.in/yaml.v2 v2.4.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect helm.sh/helm/v3 v3.15.1 // indirect - k8s.io/component-base v0.30.2 // indirect + k8s.io/component-base v0.30.3 // indirect k8s.io/klog/v2 v2.120.1 // indirect k8s.io/kube-openapi v0.0.0-20240430033511-f0e62f92d13f // indirect oras.land/oras-go v1.2.5 // indirect diff --git a/go.sum b/go.sum index b153808eea..64035e7e5f 100644 --- a/go.sum +++ b/go.sum @@ -467,8 +467,8 @@ github.com/google/go-cmp v0.5.8/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeN github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= -github.com/google/go-containerregistry v0.20.0 h1:wRqHpOeVh3DnenOrPy9xDOLdnLatiGuuNRVelR2gSbg= -github.com/google/go-containerregistry v0.20.0/go.mod h1:YCMFNQeeXeLF+dnhhWkqDItx/JSkH01j1Kis4PsjzFI= +github.com/google/go-containerregistry v0.20.1 h1:eTgx9QNYugV4DN5mz4U8hiAGTi1ybXn0TPi4Smd8du0= +github.com/google/go-containerregistry v0.20.1/go.mod h1:YCMFNQeeXeLF+dnhhWkqDItx/JSkH01j1Kis4PsjzFI= github.com/google/go-containerregistry/pkg/authn/kubernetes v0.0.0-20231202142526-55ffb0092afd h1:RkbnRtHTdBpYmp0Simm3fDUTYNVbmX4aVwdgflHLfdg= github.com/google/go-containerregistry/pkg/authn/kubernetes v0.0.0-20231202142526-55ffb0092afd/go.mod h1:5sSbf/SbGGvjWIlMlt2bkEqOq+ufOIBYrBevLuxbfSs= github.com/google/go-github/v45 v45.2.0 h1:5oRLszbrkvxDDqBCNj2hjDZMKmvexaZ1xw/FCD+K3FI= @@ -1214,28 +1214,28 @@ helm.sh/helm/v3 v3.15.1 h1:22ztacHz4gMqhXNqCQ9NAg6BFWoRUryNLvnkz6OVyw0= helm.sh/helm/v3 v3.15.1/go.mod h1:fvfoRcB8UKRUV5jrIfOTaN/pG1TPhuqSb56fjYdTKXg= honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= -istio.io/api v1.22.2 h1:b02rTNfbnsEK2HMH/kfuXHTzovSmqcL5cAj2TSklPcQ= -istio.io/api v1.22.2/go.mod h1:S3l8LWqNYS9yT+d4bH+jqzH2lMencPkW7SKM1Cu9EyM= -istio.io/client-go v1.22.2 h1:BiE7itlXFTHpZwOv0t2aZQGga7oCox8lYOdaYbyWNEo= -istio.io/client-go v1.22.2/go.mod h1:Fxt0tVZLXQRKyrBv7uwm4zCZE0qayejG0bSwZy9K6Hg= -k8s.io/api v0.30.2 h1:+ZhRj+28QT4UOH+BKznu4CBgPWgkXO7XAvMcMl0qKvI= -k8s.io/api v0.30.2/go.mod h1:ULg5g9JvOev2dG0u2hig4Z7tQ2hHIuS+m8MNZ+X6EmI= -k8s.io/apiextensions-apiserver v0.30.2 h1:l7Eue2t6QiLHErfn2vwK4KgF4NeDgjQkCXtEbOocKIE= -k8s.io/apiextensions-apiserver v0.30.2/go.mod h1:lsJFLYyK40iguuinsb3nt+Sj6CmodSI4ACDLep1rgjw= -k8s.io/apimachinery v0.30.2 h1:fEMcnBj6qkzzPGSVsAZtQThU62SmQ4ZymlXRC5yFSCg= -k8s.io/apimachinery v0.30.2/go.mod h1:iexa2somDaxdnj7bha06bhb43Zpa6eWH8N8dbqVjTUc= -k8s.io/cli-runtime v0.30.2 h1:ooM40eEJusbgHNEqnHziN9ZpLN5U4WcQGsdLKVxpkKE= -k8s.io/cli-runtime v0.30.2/go.mod h1:Y4g/2XezFyTATQUbvV5WaChoUGhojv/jZAtdp5Zkm0A= -k8s.io/client-go v0.30.2 h1:sBIVJdojUNPDU/jObC+18tXWcTJVcwyqS9diGdWHk50= -k8s.io/client-go v0.30.2/go.mod h1:JglKSWULm9xlJLx4KCkfLLQ7XwtlbflV6uFFSHTMgVs= -k8s.io/component-base v0.30.2 h1:pqGBczYoW1sno8q9ObExUqrYSKhtE5rW3y6gX88GZII= -k8s.io/component-base v0.30.2/go.mod h1:yQLkQDrkK8J6NtP+MGJOws+/PPeEXNpwFixsUI7h/OE= +istio.io/api v1.22.3 h1:V59wgcCm2fK2r137QBsddCDHNg0efg/DauIWEB9DFz8= +istio.io/api v1.22.3/go.mod h1:S3l8LWqNYS9yT+d4bH+jqzH2lMencPkW7SKM1Cu9EyM= +istio.io/client-go v1.22.3 h1:4WocGQYVTASpfn7tj1yGE8f0sgxzbxOkg56HX1LJQ5U= +istio.io/client-go v1.22.3/go.mod h1:D/vNne1n5586423NgGXMnPgshE/99mQgnjnxK/Vw2yM= +k8s.io/api v0.30.3 h1:ImHwK9DCsPA9uoU3rVh4QHAHHK5dTSv1nxJUapx8hoQ= +k8s.io/api v0.30.3/go.mod h1:GPc8jlzoe5JG3pb0KJCSLX5oAFIW3/qNJITlDj8BH04= +k8s.io/apiextensions-apiserver v0.30.3 h1:oChu5li2vsZHx2IvnGP3ah8Nj3KyqG3kRSaKmijhB9U= +k8s.io/apiextensions-apiserver v0.30.3/go.mod h1:uhXxYDkMAvl6CJw4lrDN4CPbONkF3+XL9cacCT44kV4= +k8s.io/apimachinery v0.30.3 h1:q1laaWCmrszyQuSQCfNB8cFgCuDAoPszKY4ucAjDwHc= +k8s.io/apimachinery v0.30.3/go.mod h1:iexa2somDaxdnj7bha06bhb43Zpa6eWH8N8dbqVjTUc= +k8s.io/cli-runtime v0.30.3 h1:aG69oRzJuP2Q4o8dm+f5WJIX4ZBEwrvdID0+MXyUY6k= +k8s.io/cli-runtime v0.30.3/go.mod h1:hwrrRdd9P84CXSKzhHxrOivAR9BRnkMt0OeP5mj7X30= +k8s.io/client-go v0.30.3 h1:bHrJu3xQZNXIi8/MoxYtZBBWQQXwy16zqJwloXXfD3k= +k8s.io/client-go v0.30.3/go.mod h1:8d4pf8vYu665/kUbsxWAQ/JDBNWqfFeZnvFiVdmx89U= +k8s.io/component-base v0.30.3 h1:Ci0UqKWf4oiwy8hr1+E3dsnliKnkMLZMVbWzeorlk7s= +k8s.io/component-base v0.30.3/go.mod h1:C1SshT3rGPCuNtBs14RmVD2xW0EhRSeLvBh7AGk1quA= k8s.io/klog/v2 v2.120.1 h1:QXU6cPEOIslTGvZaXvFWiP9VKyeet3sawzTOvdXb4Vw= k8s.io/klog/v2 v2.120.1/go.mod h1:3Jpz1GvMt720eyJH1ckRHK1EDfpxISzJ7I9OYgaDtPE= k8s.io/kube-openapi v0.0.0-20240430033511-f0e62f92d13f h1:0LQagt0gDpKqvIkAMPaRGcXawNMouPECM1+F9BVxEaM= k8s.io/kube-openapi v0.0.0-20240430033511-f0e62f92d13f/go.mod h1:S9tOR0FxgyusSNR+MboCuiDpVWkAifZvaYI1Q2ubgro= -k8s.io/kubectl v0.30.2 h1:cgKNIvsOiufgcs4yjvgkK0+aPCfa8pUwzXdJtkbhsH8= -k8s.io/kubectl v0.30.2/go.mod h1:rz7GHXaxwnigrqob0lJsiA07Df8RE3n1TSaC2CTeuB4= +k8s.io/kubectl v0.30.3 h1:YIBBvMdTW0xcDpmrOBzcpUVsn+zOgjMYIu7kAq+yqiI= +k8s.io/kubectl v0.30.3/go.mod h1:IcR0I9RN2+zzTRUa1BzZCm4oM0NLOawE6RzlDvd1Fpo= k8s.io/utils v0.0.0-20240502163921-fe8a2dddb1d0 h1:jgGTlFYnhF1PM1Ax/lAlxUPE+KfCIXHaathvJg1C3ak= k8s.io/utils v0.0.0-20240502163921-fe8a2dddb1d0/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0= oras.land/oras-go v1.2.5 h1:XpYuAwAb0DfQsunIyMfeET92emK8km3W4yEzZvUbsTo= From dc7ae44a6b6a579adee7d478880ec2cc47b9fef8 Mon Sep 17 00:00:00 2001 From: Xin Ruan Date: Thu, 25 Jul 2024 08:26:03 +0200 Subject: [PATCH 14/14] fix: Manifest CR should update by moduletemplate generation changes (#1702) * when moduletemplate generation updated, then manifest CR should also updated. * refactor regular_test.go --------- Co-authored-by: Benjamin Lindner <50365642+lindnerby@users.noreply.github.com> --- internal/descriptor/cache/key.go | 3 +- internal/descriptor/cache/key_test.go | 11 +- pkg/module/sync/runner.go | 45 ++- pkg/module/sync/runner_test.go | 86 +++++- pkg/templatelookup/regular.go | 176 +++++------ pkg/templatelookup/regular_test.go | 380 ++++++++++++++++++++++++ pkg/testutils/builder/kyma.go | 18 ++ pkg/testutils/builder/moduletemplate.go | 10 +- unit-test-coverage.yaml | 1 + 9 files changed, 600 insertions(+), 130 deletions(-) create mode 100644 pkg/templatelookup/regular_test.go diff --git a/internal/descriptor/cache/key.go b/internal/descriptor/cache/key.go index 500c197e8c..643dee5742 100644 --- a/internal/descriptor/cache/key.go +++ b/internal/descriptor/cache/key.go @@ -16,7 +16,8 @@ func GenerateDescriptorKey(template *v1beta2.ModuleTemplate) DescriptorKey { moduleVersion := template.Annotations[shared.ModuleVersionAnnotation] _, err := semver.NewVersion(moduleVersion) if moduleVersion != "" && err == nil { - return DescriptorKey(fmt.Sprintf("%s:%s:%s", template.Name, template.Spec.Channel, moduleVersion)) + return DescriptorKey(fmt.Sprintf("%s:%s:%d:%s", template.Name, template.Spec.Channel, template.Generation, + moduleVersion)) } } diff --git a/internal/descriptor/cache/key_test.go b/internal/descriptor/cache/key_test.go index c40dff7672..1261dd1d69 100644 --- a/internal/descriptor/cache/key_test.go +++ b/internal/descriptor/cache/key_test.go @@ -18,16 +18,17 @@ func TestGenerateDescriptorCacheKey(t *testing.T) { want cache.DescriptorKey }{ { - name: "Annotations is not nil and valid semver", + name: "ModuleVersionAnnotation is not nil and valid semver", template: builder.NewModuleTemplateBuilder(). WithName("name"). WithAnnotation(shared.ModuleVersionAnnotation, "1.0.0"). WithChannel("channel"). + WithGeneration(1). Build(), - want: "name:channel:1.0.0", + want: "name:channel:1:1.0.0", }, { - name: "Annotations is not nil but invalid semver", + name: "ModuleVersionAnnotation is not nil but invalid semver", template: builder.NewModuleTemplateBuilder(). WithName("name"). WithGeneration(1). @@ -37,7 +38,7 @@ func TestGenerateDescriptorCacheKey(t *testing.T) { want: "name:channel:1", }, { - name: "Annotations is not nil but module version is empty", + name: "ModuleVersionAnnotation is not nil but module version is empty", template: builder.NewModuleTemplateBuilder(). WithName("name"). WithGeneration(2). @@ -47,7 +48,7 @@ func TestGenerateDescriptorCacheKey(t *testing.T) { want: "name:channel:2", }, { - name: "Annotations is nil", + name: "ModuleVersionAnnotation is nil", template: builder.NewModuleTemplateBuilder(). WithName("name"). WithGeneration(3). diff --git a/pkg/module/sync/runner.go b/pkg/module/sync/runner.go index 3bfe8f070c..67128b563c 100644 --- a/pkg/module/sync/runner.go +++ b/pkg/module/sync/runner.go @@ -54,6 +54,11 @@ func (r *Runner) ReconcileManifests(ctx context.Context, kyma *v1beta2.Kyma, results := make(chan error, len(modules)) for _, module := range modules { go func(module *common.Module) { + // Should not happen, but in case of NPE, we should stop process further. + if module.Template == nil { + results <- nil + return + } // Due to module template visibility change, some module previously deployed should be removed. if errors.Is(module.Template.Err, templatelookup.ErrTemplateNotAllowed) { results <- r.deleteManifest(ctx, module) @@ -111,7 +116,7 @@ func (r *Runner) updateManifest(ctx context.Context, kyma *v1beta2.Kyma, } moduleStatus := kyma.GetModuleStatusMap()[module.ModuleName] - if err := r.doUpdateWithStrategy(ctx, kyma.Labels[shared.ManagedBy], module.Enabled, + if err := r.doUpdateWithStrategy(ctx, kyma.Labels[shared.ManagedBy], module, manifestObj, moduleStatus); err != nil { return err } @@ -119,7 +124,7 @@ func (r *Runner) updateManifest(ctx context.Context, kyma *v1beta2.Kyma, return nil } -func (r *Runner) doUpdateWithStrategy(ctx context.Context, owner string, isEnabledModule bool, +func (r *Runner) doUpdateWithStrategy(ctx context.Context, owner string, module *common.Module, manifestObj *v1beta2.Manifest, kymaModuleStatus *v1beta2.ModuleStatus, ) error { manifestInCluster := &v1beta2.Manifest{} @@ -133,12 +138,12 @@ func (r *Runner) doUpdateWithStrategy(ctx context.Context, owner string, isEnabl manifestInCluster = nil } - if !NeedToUpdate(manifestInCluster, manifestObj, kymaModuleStatus) { + if !NeedToUpdate(manifestInCluster, manifestObj, kymaModuleStatus, module.Template.GetGeneration()) { // Point to the current state from the cluster for the outside sync of the manifest *manifestObj = *manifestInCluster return nil } - if isEnabledModule { + if module.Enabled { return r.patchManifest(ctx, owner, manifestObj) } // For disabled module, the manifest CR is under deleting, in this case, we only update the spec when it's still not deleted. @@ -174,11 +179,15 @@ func (r *Runner) updateAvailableManifestSpec(ctx context.Context, manifestObj *v return nil } -func NeedToUpdate(manifestInCluster, manifestObj *v1beta2.Manifest, moduleStatus *v1beta2.ModuleStatus) bool { +func NeedToUpdate(manifestInCluster, manifestObj *v1beta2.Manifest, moduleStatus *v1beta2.ModuleStatus, + moduleTemplateGeneration int64, +) bool { if manifestInCluster == nil || moduleStatus == nil { // moduleStatus is nil in case of mandatory module return true } - + if moduleStatus.Template != nil && moduleStatus.Template.GetGeneration() != moduleTemplateGeneration { + return true + } return manifestObj.Spec.Version != moduleStatus.Version || manifestObj.Labels[shared.ChannelLabel] != moduleStatus.Channel || moduleStatus.State != manifestInCluster.Status.State @@ -263,8 +272,12 @@ func generateModuleStatus(module *common.Module, existStatus *v1beta2.ModuleStat moduleCRAPIVersion, moduleCRKind := manifestObject.Spec.Resource. GetObjectKind().GroupVersionKind().ToAPIVersionAndKind() moduleResource = &v1beta2.TrackingObject{ - PartialMeta: v1beta2.PartialMetaFromObject(manifestObject.Spec.Resource), - TypeMeta: apimetav1.TypeMeta{Kind: moduleCRKind, APIVersion: moduleCRAPIVersion}, + PartialMeta: v1beta2.PartialMeta{ + Name: manifestObject.Spec.Resource.GetName(), + Namespace: manifestObject.Spec.Resource.GetNamespace(), + Generation: manifestObject.Spec.Resource.GetGeneration(), + }, + TypeMeta: apimetav1.TypeMeta{Kind: moduleCRKind, APIVersion: moduleCRAPIVersion}, } if module.Template.Annotations[shared.IsClusterScopedAnnotation] == shared.EnableLabelValue { @@ -279,12 +292,20 @@ func generateModuleStatus(module *common.Module, existStatus *v1beta2.ModuleStat Channel: module.Template.Spec.Channel, Version: manifestObject.Spec.Version, Manifest: &v1beta2.TrackingObject{ - PartialMeta: v1beta2.PartialMetaFromObject(manifestObject), - TypeMeta: apimetav1.TypeMeta{Kind: manifestKind, APIVersion: manifestAPIVersion}, + PartialMeta: v1beta2.PartialMeta{ + Name: manifestObject.GetName(), + Namespace: manifestObject.GetNamespace(), + Generation: manifestObject.GetGeneration(), + }, + TypeMeta: apimetav1.TypeMeta{Kind: manifestKind, APIVersion: manifestAPIVersion}, }, Template: &v1beta2.TrackingObject{ - PartialMeta: v1beta2.PartialMetaFromObject(module.Template), - TypeMeta: apimetav1.TypeMeta{Kind: templateKind, APIVersion: templateAPIVersion}, + PartialMeta: v1beta2.PartialMeta{ + Name: module.Template.GetName(), + Namespace: module.Template.GetNamespace(), + Generation: module.Template.GetGeneration(), + }, + TypeMeta: apimetav1.TypeMeta{Kind: templateKind, APIVersion: templateAPIVersion}, }, Resource: moduleResource, } diff --git a/pkg/module/sync/runner_test.go b/pkg/module/sync/runner_test.go index e5754b6680..2784c3de95 100644 --- a/pkg/module/sync/runner_test.go +++ b/pkg/module/sync/runner_test.go @@ -191,10 +191,13 @@ func configureModuleInKyma( func TestNeedToUpdate(t *testing.T) { type args struct { - manifestInCluster *v1beta2.Manifest - manifestObj *v1beta2.Manifest - moduleStatus *v1beta2.ModuleStatus + manifestInCluster *v1beta2.Manifest + manifestObj *v1beta2.Manifest + moduleStatus *v1beta2.ModuleStatus + templateGeneration int64 } + const trackedModuleTemplateGeneration = 1 + const updatedModuleTemplateGeneration = 2 tests := []struct { name string args args @@ -202,7 +205,7 @@ func TestNeedToUpdate(t *testing.T) { }{ { "When manifest in cluster is nil, expect need to update", - args{nil, &v1beta2.Manifest{}, &v1beta2.ModuleStatus{}}, + args{nil, &v1beta2.Manifest{}, &v1beta2.ModuleStatus{}, trackedModuleTemplateGeneration}, true, }, { @@ -214,7 +217,17 @@ func TestNeedToUpdate(t *testing.T) { Labels: map[string]string{shared.ChannelLabel: "regular"}, }, Spec: v1beta2.ManifestSpec{Version: "0.2"}, - }, &v1beta2.ModuleStatus{Version: "0.1", Channel: "regular"}, + }, + &v1beta2.ModuleStatus{ + Version: "0.1", + Channel: "regular", + Template: &v1beta2.TrackingObject{ + PartialMeta: v1beta2.PartialMeta{ + Generation: trackedModuleTemplateGeneration, + }, + }, + }, + trackedModuleTemplateGeneration, }, true, }, @@ -227,18 +240,33 @@ func TestNeedToUpdate(t *testing.T) { Labels: map[string]string{shared.ChannelLabel: "fast"}, }, Spec: v1beta2.ManifestSpec{Version: "0.1"}, - }, &v1beta2.ModuleStatus{Version: "0.1", Channel: "regular"}, + }, &v1beta2.ModuleStatus{ + Version: "0.1", Channel: "regular", Template: &v1beta2.TrackingObject{ + PartialMeta: v1beta2.PartialMeta{ + Generation: trackedModuleTemplateGeneration, + }, + }, + }, + trackedModuleTemplateGeneration, }, true, }, { "When cluster Manifest in divergent state, expect need to update", args{ - &v1beta2.Manifest{Status: shared.Status{ - State: "Warning", - }}, + &v1beta2.Manifest{ + Status: shared.Status{ + State: "Warning", + }, + }, &v1beta2.Manifest{}, - &v1beta2.ModuleStatus{State: "Ready"}, + &v1beta2.ModuleStatus{ + State: "Ready", Template: &v1beta2.TrackingObject{ + PartialMeta: v1beta2.PartialMeta{ + Generation: trackedModuleTemplateGeneration, + }, + }, + }, trackedModuleTemplateGeneration, }, true, }, @@ -257,14 +285,48 @@ func TestNeedToUpdate(t *testing.T) { }, Spec: v1beta2.ManifestSpec{Version: "0.1"}, }, - &v1beta2.ModuleStatus{State: "Ready", Version: "0.1", Channel: "regular"}, + &v1beta2.ModuleStatus{ + State: "Ready", Version: "0.1", Channel: "regular", Template: &v1beta2.TrackingObject{ + PartialMeta: v1beta2.PartialMeta{ + Generation: trackedModuleTemplateGeneration, + }, + }, + }, trackedModuleTemplateGeneration, }, false, }, + { + "When moduleTemplate Generation updated, expect update", + args{ + &v1beta2.Manifest{ + Status: shared.Status{ + State: "Ready", + }, + Spec: v1beta2.ManifestSpec{Version: "0.1"}, + }, + &v1beta2.Manifest{ + ObjectMeta: apimetav1.ObjectMeta{ + Labels: map[string]string{shared.ChannelLabel: "regular"}, + }, + Spec: v1beta2.ManifestSpec{Version: "0.1"}, + }, + &v1beta2.ModuleStatus{ + State: "Ready", Version: "0.1", Channel: "regular", Template: &v1beta2.TrackingObject{ + PartialMeta: v1beta2.PartialMeta{ + Generation: trackedModuleTemplateGeneration, + }, + }, + }, updatedModuleTemplateGeneration, + }, + true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - assert.Equalf(t, tt.want, sync.NeedToUpdate(tt.args.manifestInCluster, tt.args.manifestObj, tt.args.moduleStatus), "needToUpdate(%v, %v, %v)", tt.args.manifestInCluster, tt.args.manifestObj, tt.args.moduleStatus) + assert.Equalf(t, tt.want, sync.NeedToUpdate(tt.args.manifestInCluster, tt.args.manifestObj, + tt.args.moduleStatus, tt.args.templateGeneration), "needToUpdate(%v, %v, %v)", + tt.args.manifestInCluster, tt.args.manifestObj, + tt.args.moduleStatus) }) } } diff --git a/pkg/templatelookup/regular.go b/pkg/templatelookup/regular.go index ff58e6192d..53a5c99610 100644 --- a/pkg/templatelookup/regular.go +++ b/pkg/templatelookup/regular.go @@ -51,54 +51,56 @@ func (t *TemplateLookup) GetRegularTemplates(ctx context.Context, kyma *v1beta2. if found { continue } - template := t.GetAndValidate(ctx, module.Name, module.Channel, kyma.Spec.Channel) - if template.Err != nil { - templates[module.Name] = &template + templateInfo := t.GetAndValidate(ctx, module.Name, module.Channel, kyma.Spec.Channel) + templateInfo = ValidateTemplateMode(templateInfo, kyma) + if templateInfo.Err != nil { + templates[module.Name] = &templateInfo continue } - if err := t.descriptorProvider.Add(template.ModuleTemplate); err != nil { - template.Err = fmt.Errorf("failed to get descriptor: %w", err) - } - - templates[module.Name] = &template - } - - for moduleName, moduleTemplate := range templates { - if moduleTemplate.Err != nil { + if err := t.descriptorProvider.Add(templateInfo.ModuleTemplate); err != nil { + templateInfo.Err = fmt.Errorf("failed to get descriptor: %w", err) + templates[module.Name] = &templateInfo continue } - - if moduleTemplate.IsInternal() && !kyma.IsInternal() { - moduleTemplate.Err = fmt.Errorf("%w: internal module", ErrTemplateNotAllowed) - templates[moduleName] = moduleTemplate - } - if moduleTemplate.IsBeta() && !kyma.IsBeta() { - moduleTemplate.Err = fmt.Errorf("%w: beta module", ErrTemplateNotAllowed) - templates[moduleName] = moduleTemplate - } - } - - for moduleName, moduleTemplate := range templates { - template := moduleTemplate for i := range kyma.Status.Modules { moduleStatus := &kyma.Status.Modules[i] - if moduleMatch(moduleStatus, moduleName) && template.ModuleTemplate != nil { - t.checkValidTemplateUpdate(ctx, template, moduleStatus) + if moduleMatch(moduleStatus, module.Name) { + descriptor, err := t.descriptorProvider.GetDescriptor(templateInfo.ModuleTemplate) + if err != nil { + msg := "could not handle channel skew as descriptor from template cannot be fetched" + templateInfo.Err = fmt.Errorf("%w: %s", ErrTemplateUpdateNotAllowed, msg) + continue + } + markInvalidChannelSkewUpdate(ctx, &templateInfo, moduleStatus, descriptor.Version) } } - templates[moduleName] = template + templates[module.Name] = &templateInfo } - return templates } +func ValidateTemplateMode(template ModuleTemplateInfo, kyma *v1beta2.Kyma) ModuleTemplateInfo { + if template.Err != nil { + return template + } + if template.IsInternal() && !kyma.IsInternal() { + template.Err = fmt.Errorf("%w: internal module", ErrTemplateNotAllowed) + return template + } + if template.IsBeta() && !kyma.IsBeta() { + template.Err = fmt.Errorf("%w: beta module", ErrTemplateNotAllowed) + return template + } + return template +} + func (t *TemplateLookup) GetAndValidate(ctx context.Context, name, channel, defaultChannel string) ModuleTemplateInfo { desiredChannel := getDesiredChannel(channel, defaultChannel) info := ModuleTemplateInfo{ DesiredChannel: desiredChannel, } - template, err := t.getTemplate(ctx, t, name, desiredChannel) + template, err := t.getTemplate(ctx, name, desiredChannel) if err != nil { info.Err = err return info @@ -141,85 +143,67 @@ func moduleMatch(moduleStatus *v1beta2.ModuleStatus, moduleName string) bool { return moduleStatus.Name == moduleName } -// checkValidTemplateUpdate verifies if the given ModuleTemplate is valid for update and sets their IsValidUpdate Flag -// based on provided Modules, provided by the Cluster as a status of the last known module state. -// It does this by looking into selected key properties: -// 1. If the generation of ModuleTemplate changes, it means the spec is outdated -// 2. If the channel of ModuleTemplate changes, it means the kyma has an old reference to a previous channel. -func (t *TemplateLookup) checkValidTemplateUpdate( - ctx context.Context, moduleTemplate *ModuleTemplateInfo, moduleStatus *v1beta2.ModuleStatus, +// markInvalidChannelSkewUpdate verifies if the given ModuleTemplate is invalid for update when channel switch is detected. +func markInvalidChannelSkewUpdate(ctx context.Context, moduleTemplateInfo *ModuleTemplateInfo, + moduleStatus *v1beta2.ModuleStatus, templateVersion string, ) { if moduleStatus.Template == nil { return } + if moduleTemplateInfo == nil || moduleTemplateInfo.Err != nil { + return + } + logger := logf.FromContext(ctx) checkLog := logger.WithValues("module", moduleStatus.FQDN, - "template", moduleTemplate.Name, - "newTemplateGeneration", moduleTemplate.GetGeneration(), - "previousTemplateGeneration", moduleStatus.Template.Generation, - "newTemplateChannel", moduleTemplate.Spec.Channel, + "template", moduleTemplateInfo.Name, + "newTemplateGeneration", moduleTemplateInfo.GetGeneration(), + "previousTemplateGeneration", moduleStatus.Template.GetGeneration(), + "newTemplateChannel", moduleTemplateInfo.Spec.Channel, "previousTemplateChannel", moduleStatus.Channel, ) - if moduleTemplate.Spec.Channel != moduleStatus.Channel { - checkLog.Info("outdated ModuleTemplate: channel skew") - - descriptor, err := t.descriptorProvider.GetDescriptor(moduleTemplate.ModuleTemplate) - if err != nil { - msg := "could not handle channel skew as descriptor from template cannot be fetched" - checkLog.Error(err, msg) - moduleTemplate.Err = fmt.Errorf("%w: %s", ErrTemplateUpdateNotAllowed, msg) - return - } - - versionInTemplate, err := semver.NewVersion(descriptor.Version) - if err != nil { - msg := "could not handle channel skew as descriptor from template contains invalid version" - checkLog.Error(err, msg) - moduleTemplate.Err = fmt.Errorf("%w: %s", ErrTemplateUpdateNotAllowed, msg) - return - } - - versionInStatus, err := semver.NewVersion(moduleStatus.Version) - if err != nil { - msg := "could not handle channel skew as Modules contains invalid version" - checkLog.Error(err, msg) - moduleTemplate.Err = fmt.Errorf("%w: %s", ErrTemplateUpdateNotAllowed, msg) - return - } - - checkLog = checkLog.WithValues( - "previousVersion", versionInTemplate.String(), - "newVersion", versionInStatus.String(), - ) + if moduleTemplateInfo.Spec.Channel == moduleStatus.Channel { + return + } - // channel skews have to be handled with more detail. If a channel is changed this means - // that the downstream kyma might have changed its target channel for the module, meaning - // the old moduleStatus is reflecting the previous desired state. - // when increasing channel stability, this means we could potentially have a downgrade - // of module versions here (fast: v2.0.0 get downgraded to regular: v1.0.0). In this - // case we want to suspend updating the module until we reach v2.0.0 in regular, since downgrades - // are not supported. To circumvent this, a module can be uninstalled and then reinstalled in the old channel. - if !v1beta2.IsValidVersionChange(versionInTemplate, versionInStatus) { - msg := fmt.Sprintf("ignore channel skew (from %s to %s), "+ - "as a higher version (%s) of the module was previously installed", - moduleStatus.Channel, moduleTemplate.Spec.Channel, versionInStatus.String()) - checkLog.Info(msg) - moduleTemplate.Err = fmt.Errorf("%w: %s", ErrTemplateUpdateNotAllowed, msg) - return - } + checkLog.Info("outdated ModuleTemplate: channel skew") + versionInTemplate, err := semver.NewVersion(templateVersion) + if err != nil { + msg := "could not handle channel skew as descriptor from template contains invalid version" + checkLog.Error(err, msg) + moduleTemplateInfo.Err = fmt.Errorf("%w: %s", ErrTemplateUpdateNotAllowed, msg) return } - // generation skews always have to be handled. We are not in need of checking downgrades here, - // since these are caught by our validating webhook. We do not support downgrades of Versions - // in ModuleTemplates, meaning the only way the generation can be changed is by changing the target - // channel (valid change) or a version increase - if moduleTemplate.GetGeneration() != moduleStatus.Template.Generation { - checkLog.Info("outdated ModuleTemplate: generation skew") + versionInStatus, err := semver.NewVersion(moduleStatus.Version) + if err != nil { + msg := "could not handle channel skew as Modules contains invalid version" + checkLog.Error(err, msg) + moduleTemplateInfo.Err = fmt.Errorf("%w: %s", ErrTemplateUpdateNotAllowed, msg) return } + + checkLog = checkLog.WithValues( + "previousVersion", versionInTemplate.String(), + "newVersion", versionInStatus.String(), + ) + + // channel skews have to be handled with more detail. If a channel is changed this means + // that the downstream kyma might have changed its target channel for the module, meaning + // the old moduleStatus is reflecting the previous desired state. + // when increasing channel stability, this means we could potentially have a downgrade + // of module versions here (fast: v2.0.0 get downgraded to regular: v1.0.0). In this + // case we want to suspend updating the module until we reach v2.0.0 in regular, since downgrades + // are not supported. To circumvent this, a module can be uninstalled and then reinstalled in the old channel. + if !v1beta2.IsValidVersionChange(versionInTemplate, versionInStatus) { + msg := fmt.Sprintf("ignore channel skew (from %s to %s), "+ + "as a higher version (%s) of the module was previously installed", + moduleStatus.Channel, moduleTemplateInfo.Spec.Channel, versionInStatus.String()) + checkLog.Info(msg) + moduleTemplateInfo.Err = fmt.Errorf("%w: %s", ErrTemplateUpdateNotAllowed, msg) + } } func getDesiredChannel(moduleChannel, globalChannel string) string { @@ -237,18 +221,18 @@ func getDesiredChannel(moduleChannel, globalChannel string) string { return desiredChannel } -func (t *TemplateLookup) getTemplate(ctx context.Context, clnt client.Reader, name, desiredChannel string) ( +func (t *TemplateLookup) getTemplate(ctx context.Context, name, desiredChannel string) ( *v1beta2.ModuleTemplate, error, ) { templateList := &v1beta2.ModuleTemplateList{} - err := clnt.List(ctx, templateList) + err := t.List(ctx, templateList) if err != nil { return nil, fmt.Errorf("failed to list module templates on lookup: %w", err) } var filteredTemplates []*v1beta2.ModuleTemplate for _, template := range templateList.Items { - template := template // capture unique address + template := template if template.Labels[shared.ModuleName] == name && template.Spec.Channel == desiredChannel { filteredTemplates = append(filteredTemplates, &template) continue diff --git a/pkg/templatelookup/regular_test.go b/pkg/templatelookup/regular_test.go new file mode 100644 index 0000000000..03170c4310 --- /dev/null +++ b/pkg/templatelookup/regular_test.go @@ -0,0 +1,380 @@ +package templatelookup_test + +import ( + "context" + "errors" + "testing" + + "github.com/open-component-model/ocm/pkg/contexts/ocm/compdesc" + ocmmetav1 "github.com/open-component-model/ocm/pkg/contexts/ocm/compdesc/meta/v1" + compdescv2 "github.com/open-component-model/ocm/pkg/contexts/ocm/compdesc/versions/v2" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "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/descriptor/provider" + "github.com/kyma-project/lifecycle-manager/pkg/templatelookup" + "github.com/kyma-project/lifecycle-manager/pkg/testutils" + "github.com/kyma-project/lifecycle-manager/pkg/testutils/builder" +) + +type FakeModuleTemplateReader struct { + templateList v1beta2.ModuleTemplateList +} + +func NewFakeModuleTemplateReader(templateList v1beta2.ModuleTemplateList) *FakeModuleTemplateReader { + return &FakeModuleTemplateReader{ + templateList: templateList, + } +} + +func (f *FakeModuleTemplateReader) List(_ context.Context, list client.ObjectList, _ ...client.ListOption) error { + castedList, ok := list.(*v1beta2.ModuleTemplateList) + if !ok { + return errors.New("list is not of type *v1beta2.ModuleTemplateList") + } + castedList.Items = append(castedList.Items, f.templateList.Items...) + return nil +} + +func (f *FakeModuleTemplateReader) Get(_ context.Context, _ client.ObjectKey, _ client.Object, + _ ...client.GetOption, +) error { + return nil +} + +func TestValidateTemplateMode(t *testing.T) { + tests := []struct { + name string + template templatelookup.ModuleTemplateInfo + kyma *v1beta2.Kyma + wantErr error + }{ + { + name: "When TemplateInfo contains Error, Then the output is same as input", + template: templatelookup.ModuleTemplateInfo{ + Err: templatelookup.ErrTemplateNotAllowed, + }, + wantErr: templatelookup.ErrTemplateNotAllowed, + }, + { + name: "When ModuleTemplate is internal but Kyma is not, Then result contains error", + template: templatelookup.ModuleTemplateInfo{ + ModuleTemplate: builder.NewModuleTemplateBuilder(). + WithLabel(shared.InternalLabel, "true").Build(), + }, + kyma: builder.NewKymaBuilder(). + WithLabel(shared.InternalLabel, "false"). + Build(), + wantErr: templatelookup.ErrTemplateNotAllowed, + }, + { + name: "When ModuleTemplate is beta but Kyma is not, Then result contains error", + template: templatelookup.ModuleTemplateInfo{ + ModuleTemplate: builder.NewModuleTemplateBuilder(). + WithLabel(shared.BetaLabel, "true").Build(), + }, + kyma: builder.NewKymaBuilder(). + WithLabel(shared.BetaLabel, "false"). + Build(), + wantErr: templatelookup.ErrTemplateNotAllowed, + }, + } + for _, testCase := range tests { + t.Run(testCase.name, func(t *testing.T) { + if got := templatelookup.ValidateTemplateMode(testCase.template, testCase.kyma); !errors.Is(got.Err, + testCase.wantErr) { + t.Errorf("ValidateTemplateMode() = %v, want %v", got, testCase.wantErr) + } + }) + } +} + +func TestTemplateLookup_GetRegularTemplates_WhenSwitchModuleChannel(t *testing.T) { + testModule := testutils.NewTestModule("module1", "new_channel") + + tests := []struct { + name string + kyma *v1beta2.Kyma + availableModuleTemplate v1beta2.ModuleTemplateList + want templatelookup.ModuleTemplatesByModuleName + }{ + { + name: "When upgrade version during channel switch, Then result contains no error", + kyma: builder.NewKymaBuilder(). + WithEnabledModule(testModule). + WithModuleStatus(v1beta2.ModuleStatus{ + Name: testModule.Name, + Channel: v1beta2.DefaultChannel, + Version: "1.0.0", + Template: &v1beta2.TrackingObject{ + PartialMeta: v1beta2.PartialMeta{ + Generation: 1, + }, + }, + }).Build(), + availableModuleTemplate: generateModuleTemplateListWithModule(testModule.Name, testModule.Channel, "1.1.0"), + want: templatelookup.ModuleTemplatesByModuleName{ + testModule.Name: &templatelookup.ModuleTemplateInfo{ + DesiredChannel: testModule.Channel, + Err: nil, + }, + }, + }, { + name: "When downgrade version during channel switch, Then result contains error", + kyma: builder.NewKymaBuilder(). + WithEnabledModule(testModule). + WithModuleStatus(v1beta2.ModuleStatus{ + Name: testModule.Name, + Channel: v1beta2.DefaultChannel, + Version: "1.1.0", + Template: &v1beta2.TrackingObject{ + PartialMeta: v1beta2.PartialMeta{ + Generation: 1, + }, + }, + }).Build(), + availableModuleTemplate: generateModuleTemplateListWithModule(testModule.Name, testModule.Channel, "1.0.0"), + want: templatelookup.ModuleTemplatesByModuleName{ + testModule.Name: &templatelookup.ModuleTemplateInfo{ + DesiredChannel: testModule.Channel, + Err: templatelookup.ErrTemplateUpdateNotAllowed, + }, + }, + }, + } + + for _, testCase := range tests { + t.Run(testCase.name, func(t *testing.T) { + lookup := templatelookup.NewTemplateLookup(NewFakeModuleTemplateReader(testCase.availableModuleTemplate), + provider.NewCachedDescriptorProvider()) + got := lookup.GetRegularTemplates(context.TODO(), testCase.kyma) + assert.Equal(t, len(got), len(testCase.want)) + for key, module := range got { + wantModule, ok := testCase.want[key] + assert.True(t, ok) + assert.Equal(t, wantModule.DesiredChannel, module.DesiredChannel) + require.ErrorIs(t, module.Err, wantModule.Err) + } + }) + } +} + +func generateModuleTemplateListWithModule(moduleName, moduleChannel, moduleVersion string) v1beta2.ModuleTemplateList { + templateList := v1beta2.ModuleTemplateList{} + templateList.Items = append(templateList.Items, *builder.NewModuleTemplateBuilder(). + WithModuleName(moduleName). + WithChannel(moduleChannel). + WithDescriptor(&v1beta2.Descriptor{ + ComponentDescriptor: &compdesc.ComponentDescriptor{ + Metadata: compdesc.Metadata{ + ConfiguredVersion: compdescv2.SchemaVersion, + }, + ComponentSpec: compdesc.ComponentSpec{ + ObjectMeta: ocmmetav1.ObjectMeta{ + Version: moduleVersion, + }, + }, + }, + }).Build()) + return templateList +} + +func TestNewTemplateLookup_GetRegularTemplates_WhenModuleTemplateContainsInvalidDescriptor(t *testing.T) { + testModule := testutils.NewTestModule("module1", v1beta2.DefaultChannel) + + tests := []struct { + name string + kyma *v1beta2.Kyma + want templatelookup.ModuleTemplatesByModuleName + }{ + { + name: "When module enabled in Spec, then return ModuleTemplatesByModuleName with error", + kyma: builder.NewKymaBuilder(). + WithEnabledModule(testModule).Build(), + want: templatelookup.ModuleTemplatesByModuleName{ + testModule.Name: &templatelookup.ModuleTemplateInfo{ + DesiredChannel: testModule.Channel, + Err: provider.ErrDecode, + }, + }, + }, + { + name: "When module exits in ModuleStatus only, then return ModuleTemplatesByModuleName with error", + kyma: builder.NewKymaBuilder(). + WithModuleStatus(v1beta2.ModuleStatus{ + Name: testModule.Name, + Channel: testModule.Channel, + Template: &v1beta2.TrackingObject{ + PartialMeta: v1beta2.PartialMeta{ + Generation: 1, + }, + }, + }).Build(), + want: templatelookup.ModuleTemplatesByModuleName{ + testModule.Name: &templatelookup.ModuleTemplateInfo{ + DesiredChannel: testModule.Channel, + Err: provider.ErrDecode, + }, + }, + }, + } + for _, testCase := range tests { + t.Run(testCase.name, func(t *testing.T) { + givenTemplateList := &v1beta2.ModuleTemplateList{} + for _, module := range testCase.kyma.GetAvailableModules() { + givenTemplateList.Items = append(givenTemplateList.Items, *builder.NewModuleTemplateBuilder(). + WithModuleName(module.Name). + WithChannel(module.Channel). + WithDescriptor(nil). + WithRawDescriptor([]byte("{invalid_json}")).Build()) + } + lookup := templatelookup.NewTemplateLookup(NewFakeModuleTemplateReader(*givenTemplateList), + provider.NewCachedDescriptorProvider()) + got := lookup.GetRegularTemplates(context.TODO(), testCase.kyma) + assert.Equal(t, len(got), len(testCase.want)) + for key, module := range got { + wantModule, ok := testCase.want[key] + assert.True(t, ok) + assert.Equal(t, wantModule.DesiredChannel, module.DesiredChannel) + require.ErrorIs(t, module.Err, wantModule.Err) + } + }) + } +} + +func TestTemplateLookup_GetRegularTemplates_WhenModuleTemplateNotFound(t *testing.T) { + testModule := testutils.NewTestModule("module1", v1beta2.DefaultChannel) + + tests := []struct { + name string + kyma *v1beta2.Kyma + want templatelookup.ModuleTemplatesByModuleName + }{ + { + name: "When no module enabled in Spec, then return empty ModuleTemplatesByModuleName", + kyma: builder.NewKymaBuilder().Build(), + want: templatelookup.ModuleTemplatesByModuleName{}, + }, + { + name: "When module enabled in Spec, then return ModuleTemplatesByModuleName with error", + kyma: builder.NewKymaBuilder(). + WithEnabledModule(testModule).Build(), + want: templatelookup.ModuleTemplatesByModuleName{ + testModule.Name: &templatelookup.ModuleTemplateInfo{ + DesiredChannel: testModule.Channel, + Err: templatelookup.ErrNoTemplatesInListResult, + }, + }, + }, + { + name: "When module exits in ModuleStatus only, then return ModuleTemplatesByModuleName with error", + kyma: builder.NewKymaBuilder(). + WithModuleStatus(v1beta2.ModuleStatus{ + Name: testModule.Name, + Channel: testModule.Channel, + Template: &v1beta2.TrackingObject{ + PartialMeta: v1beta2.PartialMeta{ + Generation: 1, + }, + }, + }).Build(), + want: templatelookup.ModuleTemplatesByModuleName{ + testModule.Name: &templatelookup.ModuleTemplateInfo{ + DesiredChannel: testModule.Channel, + Err: templatelookup.ErrNoTemplatesInListResult, + }, + }, + }, + } + for _, testCase := range tests { + t.Run(testCase.name, func(t *testing.T) { + givenTemplateList := &v1beta2.ModuleTemplateList{} + lookup := templatelookup.NewTemplateLookup(NewFakeModuleTemplateReader(*givenTemplateList), + provider.NewCachedDescriptorProvider()) + got := lookup.GetRegularTemplates(context.TODO(), testCase.kyma) + assert.Equal(t, len(got), len(testCase.want)) + for key, module := range got { + wantModule, ok := testCase.want[key] + assert.True(t, ok) + assert.Equal(t, wantModule.DesiredChannel, module.DesiredChannel) + require.ErrorIs(t, module.Err, wantModule.Err) + assert.Nil(t, module.ModuleTemplate) + } + }) + } +} + +func TestTemplateLookup_GetRegularTemplates_WhenModuleTemplateExists(t *testing.T) { + testModule := testutils.NewTestModule("module1", v1beta2.DefaultChannel) + + tests := []struct { + name string + kyma *v1beta2.Kyma + want templatelookup.ModuleTemplatesByModuleName + }{ + { + name: "When module enabled in Spec, then return expected moduleTemplateInfo", + kyma: builder.NewKymaBuilder(). + WithEnabledModule(testModule).Build(), + want: templatelookup.ModuleTemplatesByModuleName{ + testModule.Name: &templatelookup.ModuleTemplateInfo{ + DesiredChannel: testModule.Channel, + Err: nil, + ModuleTemplate: builder.NewModuleTemplateBuilder(). + WithModuleName(testModule.Name). + WithChannel(testModule.Channel). + Build(), + }, + }, + }, + { + name: "When module exits in ModuleStatus only, then return expected moduleTemplateInfo", + kyma: builder.NewKymaBuilder(). + WithEnabledModule(testModule). + WithModuleStatus(v1beta2.ModuleStatus{ + Name: testModule.Name, + Channel: testModule.Channel, + Template: &v1beta2.TrackingObject{ + PartialMeta: v1beta2.PartialMeta{ + Generation: 1, + }, + }, + }).Build(), + want: templatelookup.ModuleTemplatesByModuleName{ + testModule.Name: &templatelookup.ModuleTemplateInfo{ + DesiredChannel: testModule.Channel, + Err: nil, + ModuleTemplate: builder.NewModuleTemplateBuilder(). + WithModuleName(testModule.Name). + WithChannel(testModule.Channel). + Build(), + }, + }, + }, + } + for _, testCase := range tests { + t.Run(testCase.name, func(t *testing.T) { + givenTemplateList := &v1beta2.ModuleTemplateList{} + for _, module := range testCase.kyma.GetAvailableModules() { + givenTemplateList.Items = append(givenTemplateList.Items, *builder.NewModuleTemplateBuilder(). + WithModuleName(module.Name). + WithChannel(module.Channel). + WithOCM(compdescv2.SchemaVersion).Build()) + } + lookup := templatelookup.NewTemplateLookup(NewFakeModuleTemplateReader(*givenTemplateList), + provider.NewCachedDescriptorProvider()) + got := lookup.GetRegularTemplates(context.TODO(), testCase.kyma) + assert.Equal(t, len(got), len(testCase.want)) + for key, module := range got { + wantModule, ok := testCase.want[key] + assert.True(t, ok) + assert.Equal(t, wantModule.DesiredChannel, module.DesiredChannel) + require.ErrorIs(t, module.Err, wantModule.Err) + assert.Equal(t, wantModule.ModuleTemplate.Spec.Channel, module.ModuleTemplate.Spec.Channel) + } + }) + } +} diff --git a/pkg/testutils/builder/kyma.go b/pkg/testutils/builder/kyma.go index f6bb68c86a..dadf252081 100644 --- a/pkg/testutils/builder/kyma.go +++ b/pkg/testutils/builder/kyma.go @@ -38,6 +38,15 @@ func (kb KymaBuilder) WithName(name string) KymaBuilder { return kb } +// WithEnabledModule append module to v1beta2.Kyma.Spec.Modules. +func (kb KymaBuilder) WithEnabledModule(module v1beta2.Module) KymaBuilder { + if kb.kyma.Spec.Modules == nil { + kb.kyma.Spec.Modules = []v1beta2.Module{} + } + kb.kyma.Spec.Modules = append(kb.kyma.Spec.Modules, module) + return kb +} + // WithNamePrefix sets v1beta2.Kyma.ObjectMeta.Name. func (kb KymaBuilder) WithNamePrefix(prefix string) KymaBuilder { kb.kyma.ObjectMeta.Name = fmt.Sprintf("%s-%s", prefix, random.Name()) @@ -83,6 +92,15 @@ func (kb KymaBuilder) WithCondition(condition apimetav1.Condition) KymaBuilder { return kb } +// WithCondition adds a ModuleStatus to v1beta2.Kyma.Status.Modules. +func (kb KymaBuilder) WithModuleStatus(moduleStatus v1beta2.ModuleStatus) KymaBuilder { + if kb.kyma.Status.Modules == nil { + kb.kyma.Status.Modules = []v1beta2.ModuleStatus{} + } + kb.kyma.Status.Modules = append(kb.kyma.Status.Modules, moduleStatus) + return kb +} + // Build returns the built v1beta2.Kyma. func (kb KymaBuilder) Build() *v1beta2.Kyma { return kb.kyma diff --git a/pkg/testutils/builder/moduletemplate.go b/pkg/testutils/builder/moduletemplate.go index f7904e3248..4858da3c63 100644 --- a/pkg/testutils/builder/moduletemplate.go +++ b/pkg/testutils/builder/moduletemplate.go @@ -29,7 +29,7 @@ func NewModuleTemplateBuilder() ModuleTemplateBuilder { moduleTemplate: &v1beta2.ModuleTemplate{ TypeMeta: apimetav1.TypeMeta{ APIVersion: v1beta2.GroupVersion.String(), - Kind: string(shared.KymaKind), + Kind: string(shared.ModuleTemplateKind), }, ObjectMeta: apimetav1.ObjectMeta{ Name: random.Name(), @@ -39,9 +39,11 @@ func NewModuleTemplateBuilder() ModuleTemplateBuilder { Data: data, Descriptor: machineryruntime.RawExtension{ Object: &v1beta2.Descriptor{ - ComponentDescriptor: &compdesc.ComponentDescriptor{Metadata: compdesc.Metadata{ - ConfiguredVersion: compdescv2.SchemaVersion, - }}, + ComponentDescriptor: &compdesc.ComponentDescriptor{ + Metadata: compdesc.Metadata{ + ConfiguredVersion: compdescv2.SchemaVersion, + }, + }, }, }, }, diff --git a/unit-test-coverage.yaml b/unit-test-coverage.yaml index 6824037cfa..f57b0b809c 100644 --- a/unit-test-coverage.yaml +++ b/unit-test-coverage.yaml @@ -7,3 +7,4 @@ packages: internal/istio: 63 internal/pkg/resources: 85 internal/remote: 5 + pkg/templatelookup: 63