From aedda3633f108d4f557bfe96566a4315888ae764 Mon Sep 17 00:00:00 2001 From: Tomasz Smelcerz Date: Tue, 10 Dec 2024 12:03:05 +0100 Subject: [PATCH] fix: Modules should be in error state when SKR cluster is deleted (#2093) * adjust for linux * set all modules to error when SKR doesn't exist * Add e2e test * Add e2e test * Add e2e test * remove local changes * review fix --- .../action.yml | 3 +- .../test-e2e-with-modulereleasemeta.yml | 1 + internal/controller/kyma/controller.go | 12 ++++ pkg/module/sync/runner.go | 4 +- pkg/util/error.go | 14 ++++ tests/e2e/Makefile | 5 +- tests/e2e/skr_connection_lost_test.go | 69 +++++++++++++++++++ 7 files changed, 104 insertions(+), 4 deletions(-) create mode 100644 tests/e2e/skr_connection_lost_test.go diff --git a/.github/actions/deploy-template-operator-with-modulereleasemeta/action.yml b/.github/actions/deploy-template-operator-with-modulereleasemeta/action.yml index de513f4566..5a2aa849e1 100644 --- a/.github/actions/deploy-template-operator-with-modulereleasemeta/action.yml +++ b/.github/actions/deploy-template-operator-with-modulereleasemeta/action.yml @@ -63,7 +63,8 @@ runs: if: ${{ matrix.e2e-test == 'module-upgrade-channel-switch' || matrix.e2e-test == 'modulereleasemeta-module-upgrade-new-version' || matrix.e2e-test == 'modulereleasemeta-upgrade-under-deletion' || - matrix.e2e-test == 'modulereleasemeta-sync' + matrix.e2e-test == 'modulereleasemeta-sync' || + matrix.e2e-test == 'module-status-on-skr-connection-lost' }} shell: bash run: | diff --git a/.github/workflows/test-e2e-with-modulereleasemeta.yml b/.github/workflows/test-e2e-with-modulereleasemeta.yml index 505c68fd84..a02b057280 100644 --- a/.github/workflows/test-e2e-with-modulereleasemeta.yml +++ b/.github/workflows/test-e2e-with-modulereleasemeta.yml @@ -63,6 +63,7 @@ jobs: - rbac-privileges - modulereleasemeta-with-obsolete-moduletemplate - modulereleasemeta-sync + - module-status-on-skr-connection-lost - modulereleasemeta-watch-trigger runs-on: ubuntu-latest diff --git a/internal/controller/kyma/controller.go b/internal/controller/kyma/controller.go index bb8ddb8739..cf7e8efbcc 100644 --- a/internal/controller/kyma/controller.go +++ b/internal/controller/kyma/controller.go @@ -124,6 +124,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu skrContext, err := r.SkrContextFactory.Get(kyma.GetNamespacedName()) if err != nil { r.Metrics.RecordRequeueReason(metrics.SyncContextRetrieval, queue.UnexpectedRequeue) + setModuleStatusesToError(kyma, err.Error()) return r.requeueWithError(ctx, kyma, err) } @@ -137,6 +138,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu if err != nil { r.SkrContextFactory.InvalidateCache(kyma.GetNamespacedName()) r.Metrics.RecordRequeueReason(metrics.SyncContextRetrieval, queue.UnexpectedRequeue) + setModuleStatusesToError(kyma, util.NestedErrorMessage(err)) return r.requeueWithError(ctx, kyma, err) } } @@ -673,3 +675,13 @@ func needUpdateForMandatoryModuleLabel(moduleTemplate v1beta2.ModuleTemplate) bo return false } + +func setModuleStatusesToError(kyma *v1beta2.Kyma, message string) { + moduleStatuses := kyma.Status.Modules + for i := range moduleStatuses { + moduleStatuses[i].State = shared.StateError + if message != "" { + moduleStatuses[i].Message = message + } + } +} diff --git a/pkg/module/sync/runner.go b/pkg/module/sync/runner.go index ddb5afff06..d21829152a 100644 --- a/pkg/module/sync/runner.go +++ b/pkg/module/sync/runner.go @@ -246,14 +246,14 @@ func (r *Runner) setupModule(module *common.Module, kyma *v1beta2.Kyma) error { func (r *Runner) SyncModuleStatus(ctx context.Context, kyma *v1beta2.Kyma, modules common.Modules, kymaMetrics *metrics.KymaMetrics, moduleMetrics *metrics.ModuleMetrics, ) { - updateModuleStatusFromExistingModules(modules, kyma) + updateModuleStatusFromExistingModules(kyma, modules) DeleteNoLongerExistingModuleStatus(ctx, kyma, r.getModule, kymaMetrics.RemoveModuleStateMetrics, moduleMetrics.RemoveModuleCRWarningCondition) } func updateModuleStatusFromExistingModules( - modules common.Modules, kyma *v1beta2.Kyma, + modules common.Modules, ) { moduleStatusMap := kyma.GetModuleStatusMap() diff --git a/pkg/util/error.go b/pkg/util/error.go index e69697aeca..9450d9188e 100644 --- a/pkg/util/error.go +++ b/pkg/util/error.go @@ -59,3 +59,17 @@ func isNoSuchHostError(err error) bool { } return false } + +// NestedErrorMessage returns the error message of the wrapped error, if it exists. Otherwise, it returns an empty string. +func NestedErrorMessage(err error) string { + res := "" + + if err == nil { + return res + } + if uwErr := errors.Unwrap(err); uwErr != nil { + res = uwErr.Error() + } + + return res +} diff --git a/tests/e2e/Makefile b/tests/e2e/Makefile index 8ec372069c..2b6cbf447f 100644 --- a/tests/e2e/Makefile +++ b/tests/e2e/Makefile @@ -151,9 +151,12 @@ ocm-compatible-module-template: modulereleasemeta-with-obsolete-moduletemplate: go test -timeout 20m -ginkgo.v -ginkgo.focus "ModuleReleaseMeta With Obsolete ModuleTemplate" - modulereleasemeta-watch-trigger: go test -timeout 20m -ginkgo.v -ginkgo.focus "ModuleReleaseMeta Watch Trigger" modulereleasemeta-sync: go test -timeout 20m -ginkgo.v -ginkgo.focus "ModuleReleaseMeta Sync" + +module-status-on-skr-connection-lost: + go test -timeout 20m -ginkgo.v -ginkgo.focus "KCP Kyma Module status on SKR connection lost" + diff --git a/tests/e2e/skr_connection_lost_test.go b/tests/e2e/skr_connection_lost_test.go new file mode 100644 index 0000000000..506ce62ed7 --- /dev/null +++ b/tests/e2e/skr_connection_lost_test.go @@ -0,0 +1,69 @@ +package e2e_test + +import ( + "os/exec" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + . "github.com/kyma-project/lifecycle-manager/pkg/testutils" + + "github.com/kyma-project/lifecycle-manager/api/shared" + "github.com/kyma-project/lifecycle-manager/api/v1beta2" +) + +var _ = Describe("KCP Kyma Module status on SKR connection lost", Ordered, func() { + kyma := NewKymaWithSyncLabel("kyma-sample", ControlPlaneNamespace, v1beta2.DefaultChannel) + module := NewTemplateOperator(v1beta2.DefaultChannel) + moduleCR := NewTestModuleCR(RemoteNamespace) + + InitEmptyKymaBeforeAll(kyma) + + Context("Given SKR Cluster", func() { + It("When Kyma Module is enabled on SKR Cluster", func() { + Eventually(EnableModule). + WithContext(ctx). + WithArguments(skrClient, defaultRemoteKymaName, RemoteNamespace, module). + Should(Succeed()) + Eventually(ModuleCRExists). + WithContext(ctx). + WithArguments(skrClient, moduleCR). + Should(Succeed()) + }) + + It("Then KCP Kyma CR is in \"Ready\" State", func() { + Eventually(KymaIsInState). + WithContext(ctx). + WithArguments(kyma.GetName(), kyma.GetNamespace(), kcpClient, shared.StateReady). + Should(Succeed()) + }) + + It("And KCP Kyma CR status.modules are in \"Ready\" State", func() { + Eventually(CheckModuleState). + WithContext(ctx). + WithArguments(kcpClient, kyma.GetName(), kyma.GetNamespace(), module.Name, shared.StateReady). + Should(Succeed()) + }) + + It("When SKR Cluster is removed", func() { + cmd := exec.Command("k3d", "cluster", "stop", "skr") + out, err := cmd.CombinedOutput() + Expect(err).NotTo(HaveOccurred()) + GinkgoWriter.Printf(string(out)) + }) + + It("Then KCP Kyma CR is in \"Error\" State", func() { + Eventually(KymaIsInState). + WithContext(ctx). + WithArguments(kyma.GetName(), kyma.GetNamespace(), kcpClient, shared.StateError). + Should(Succeed()) + }) + + It("And KCP Kyma CR status.modules are in \"Error\" State", func() { + Eventually(CheckModuleState). + WithContext(ctx). + WithArguments(kcpClient, kyma.GetName(), kyma.GetNamespace(), module.Name, shared.StateError). + Should(Succeed()) + }) + }) +})