From c9444222b78ad41e49a867e8343d39ce2379d1d4 Mon Sep 17 00:00:00 2001 From: Dharmit Shah Date: Tue, 25 Aug 2026 15:54:07 +0530 Subject: [PATCH 1/2] Enables LCM auto-upgrade A separate phase for LCM upgrade is added as the first phase for LCM so that it gets upgraded before anything else. Signed-off-by: Dharmit Shah --- api/v1alpha1/release_types.go | 3 + api/v1alpha1/zz_generated.deepcopy.go | 2 +- cmd/main.go | 1 + docs/monitor-and-troubleshoot.md | 13 ++ internal/upgrade/phase.go | 1 + internal/upgrade/reconcilers/helm.go | 22 ++- internal/upgrade/reconcilers/helm_test.go | 36 +++- internal/upgrade/reconcilers/lcm.go | 109 ++++++++++++ internal/upgrade/reconcilers/lcm_test.go | 207 ++++++++++++++++++++++ 9 files changed, 385 insertions(+), 9 deletions(-) create mode 100644 internal/upgrade/reconcilers/lcm.go create mode 100644 internal/upgrade/reconcilers/lcm_test.go diff --git a/api/v1alpha1/release_types.go b/api/v1alpha1/release_types.go index 8a55336..ea235da 100644 --- a/api/v1alpha1/release_types.go +++ b/api/v1alpha1/release_types.go @@ -39,6 +39,9 @@ const ( ConditionApplied = "Applied" // ConditionManifestResolved indicates whether the release manifest was successfully retrieved. ConditionManifestResolved = "ManifestResolved" + // ConditionLCMUpgraded indicates the states of LCM upgrade. + // Pending -> InProgress -> Succeeded/Failed + ConditionLCMUpgraded = "LCMUpgraded" // ConditionOSUpgraded indicates the status of the OS upgrade. // Pending -> InProgress -> Succeeded/Failed ConditionOSUpgraded = "OSUpgraded" diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index d34bbd2..fef8b43 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -24,7 +24,7 @@ package v1alpha1 import ( "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - runtime "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime" ) // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. diff --git a/cmd/main.go b/cmd/main.go index 47175ef..47787c3 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -206,6 +206,7 @@ func main() { Scheme: mgr.GetScheme(), RetrieveManifest: release.RetrieveManifest, Pipeline: upgrade.NewPipeline( + reconcilers.NewLCMReconciler(k8sClient, helmClient), reconcilers.NewOSReconciler(k8sClient, sucPlanReconciler), reconcilers.NewKubernetesReconciler( k8sClient, diff --git a/docs/monitor-and-troubleshoot.md b/docs/monitor-and-troubleshoot.md index bf0e2df..a78720e 100644 --- a/docs/monitor-and-troubleshoot.md +++ b/docs/monitor-and-troubleshoot.md @@ -17,6 +17,7 @@ The table below summarizes the conditions LCM reports during an upgrade: | Condition Type | Description | |----------------------|-------------| | `ManifestResolved` | Indicates whether LCM retrieved and resolved the [release manifest](https://github.com/SUSE/elemental/blob/main/docs/release-manifest.md) for the requested version. | +| `LCMUpgraded` | Tracks the LCM upgrade phase which upgrades the Helm charts for LCM itself and its CRDs if found in the manifest under upgrade | | `OSUpgraded` | Tracks the operating system upgrade phase, including the related System Upgrade Controller Plans for `control-plane` and `worker` nodes. | | `KubernetesUpgraded` | Tracks the Kubernetes upgrade phase, including related System Upgrade Controller Plans and the availability of packaged Kubernetes components after node upgrades complete. | | `HelmChartsUpgraded` | Tracks the Helm chart upgrade phase for additional chart components defined in the release manifest. | @@ -37,6 +38,18 @@ If this phase fails or stops progressing, inspect the following resources: | Manifest Cache ConfigMap | LCM namespace | `release-manifest-cache` | Use it to confirm whether the release manifest was retrieved and cached. | | LCM Pod | LCM namespace | LCM Pod name | Inspect LCM's logs for errors while retrieving, parsing, or caching the release manifest. | +### Lifecycle Manager Upgrade + +Condition type: `LCMUpgraded` + +If this phase fails or stops progressing, inspect the following resources: + + +| Resource | Namespace | Name | Description | +|-------------------------|---------------------------|-------------|------------- | +| Helm Chart Pod | `kube-system` | `helm-install-` | Inspect the Pod logs for Helm chart errors. | +| LCM Pod | `elemental-system` | LCM Pod name | Inspect LCM's logs for LCM charts upgrade reconciliation errors. | + ### Operating System Upgrade Condition type: `OSUpgraded` diff --git a/internal/upgrade/phase.go b/internal/upgrade/phase.go index 4f67b26..4bfca4d 100644 --- a/internal/upgrade/phase.go +++ b/internal/upgrade/phase.go @@ -29,6 +29,7 @@ type Phase string // Phase constants derived from condition types. var ( + PhaseLCM = Phase(strings.TrimSuffix(lifecyclev1alpha1.ConditionLCMUpgraded, "Upgraded")) PhaseOS = Phase(strings.TrimSuffix(lifecyclev1alpha1.ConditionOSUpgraded, "Upgraded")) PhaseKubernetes = Phase(strings.TrimSuffix(lifecyclev1alpha1.ConditionKubernetesUpgraded, "Upgraded")) PhaseHelmCharts = Phase(strings.TrimSuffix(lifecyclev1alpha1.ConditionHelmChartsUpgraded, "Upgraded")) diff --git a/internal/upgrade/reconcilers/helm.go b/internal/upgrade/reconcilers/helm.go index b88561e..5247198 100644 --- a/internal/upgrade/reconcilers/helm.go +++ b/internal/upgrade/reconcilers/helm.go @@ -97,7 +97,15 @@ func (r *HelmReconciler) reconcileHelmCharts(ctx context.Context, releaseName, r r.releaseName = releaseName r.releaseVersion = releaseVersion - orderedChartConfigs, err := sortChartConfigsByDependencies(chartConfigs) + var workloadCharts []*upgrade.HelmChartConfig + for _, chartCfg := range chartConfigs { + name := chartCfg.Chart.GetName() + if !isLCMChart(name) { + workloadCharts = append(workloadCharts, chartCfg) + } + } + + orderedChartConfigs, err := sortChartConfigsByDependencies(workloadCharts) if err != nil { return &upgrade.PhaseStatus{ State: lifecyclev1alpha1.UpgradeFailed, @@ -130,7 +138,7 @@ func (r *HelmReconciler) reconcileHelmCharts(ctx context.Context, releaseName, r } } - return r.aggregateResults(results, len(orderedChartConfigs)), nil + return aggregateResults(results, len(orderedChartConfigs), "Helm"), nil } // sortChartConfigsByDependencies returns a sorted slice of chart configurations, @@ -405,11 +413,11 @@ func (r *HelmReconciler) evaluateHelmChartJobStatus(ctx context.Context, chart * } // aggregateResults aggregates chart upgrade results into a single PhaseStatus. -func (r *HelmReconciler) aggregateResults(results []chartUpgradeResult, totalCharts int) *upgrade.PhaseStatus { +func aggregateResults(results []chartUpgradeResult, totalCharts int, chartKind string) *upgrade.PhaseStatus { if len(results) == 0 { return &upgrade.PhaseStatus{ State: lifecyclev1alpha1.UpgradeSucceeded, - Message: "No Helm charts to reconcile", + Message: fmt.Sprintf("No %s charts to reconcile", chartKind), } } @@ -442,20 +450,20 @@ func (r *HelmReconciler) aggregateResults(results []chartUpgradeResult, totalCha if inProgress > 0 { return &upgrade.PhaseStatus{ State: lifecyclev1alpha1.UpgradeInProgress, - Message: fmt.Sprintf("Helm charts in progress (%d/%d completed, %d skipped)", succeeded, totalCharts-skipped, skipped), + Message: fmt.Sprintf("%s charts in progress (%d/%d completed, %d skipped)", chartKind, succeeded, totalCharts-skipped, skipped), } } if succeeded == 0 && skipped == totalCharts { return &upgrade.PhaseStatus{ State: lifecyclev1alpha1.UpgradeSucceeded, - Message: "All Helm charts skipped (not installed on cluster)", + Message: fmt.Sprintf("All %s charts skipped (not installed on cluster)", chartKind), } } return &upgrade.PhaseStatus{ State: lifecyclev1alpha1.UpgradeSucceeded, - Message: fmt.Sprintf("All %d Helm charts upgraded successfully (%d skipped)", succeeded, skipped), + Message: fmt.Sprintf("All %d %s charts upgraded successfully (%d skipped)", succeeded, chartKind, skipped), } } diff --git a/internal/upgrade/reconcilers/helm_test.go b/internal/upgrade/reconcilers/helm_test.go index 466ed91..b1e5815 100644 --- a/internal/upgrade/reconcilers/helm_test.go +++ b/internal/upgrade/reconcilers/helm_test.go @@ -101,7 +101,7 @@ var _ = Describe("HelmReconciler", func() { var chart1 *api.HelmChart BeforeEach(func() { - chart1 = testutil.NewTestHelmChart(testChart1Name, "1.0.0") + chart1 = testutil.NewTestHelmChart(testChart1Name, testChartVersion) config = testutil.NewTestConfig(testutil.WithHelmChartConfig([]*upgrade.HelmChartConfig{{Chart: chart1}})) }) @@ -134,6 +134,36 @@ var _ = Describe("HelmReconciler", func() { Expect(status.Message).To(ContainSubstring("skipped")) }) + It("should skip LCM charts as they are handled by LCMReconciler", func() { + lcmChart := testutil.NewTestHelmChart("elemental-lifecycle-manager", "0.2.1") + config = testutil.NewTestConfig(testutil.WithHelmChartConfig([]*upgrade.HelmChartConfig{{Chart: lcmChart}})) + status, err := reconciler.Reconcile(ctx, config) + + Expect(err).ToNot(HaveOccurred()) + Expect(status).ToNot(BeNil()) + Expect(status.Message).To(Equal("No Helm charts to reconcile")) + }) + + It("should skip LCM chart and reconcile other charts successfully", func() { + lcmChart := testutil.NewTestHelmChart("elemental-lifecycle-manager", "0.2.1") + config = testutil.NewTestConfig(testutil.WithHelmChartConfig([]*upgrade.HelmChartConfig{{Chart: lcmChart}, {Chart: chart1}})) + + mockHelm.RetrieveReleaseFn = func(name string) (*helm.ReleaseInfo, error) { + return &helm.ReleaseInfo{ + ChartVersion: testChartVersion, + Namespace: testNamespace, + Config: map[string]any{}, + Revisions: 1, + }, nil + } + + status, err := reconciler.Reconcile(ctx, config) + + Expect(err).NotTo(HaveOccurred()) + Expect(status).NotTo(BeNil()) + Expect(status.Message).To(Equal("All 1 Helm charts upgraded successfully (0 skipped)")) + }) + It("should return error on helm client failure", func() { mockHelm.RetrieveReleaseFn = func(name string) (*helm.ReleaseInfo, error) { return nil, fmt.Errorf("helm client error") @@ -362,6 +392,10 @@ var _ = Describe("HelmReconciler", func() { // Ensure that the chart version was correctly updated. Expect(helmChart.Spec.Version).To(Equal("2.0.0")) + // Ensure the labels are applied + Expect(helmChart.Labels).To(HaveKeyWithValue(lifecyclev1alpha1.ReleaseNameLabel, config.ReleaseNamespacedName.Name)) + Expect(helmChart.Labels).To(HaveKeyWithValue(lifecyclev1alpha1.ReleaseVersionLabel, lifecyclev1alpha1.SanitizeVersion(config.ReleaseVersion))) + // Ensure install time custom values are not corrupted. expectedInstallValues, err := yaml.Marshal(installTimeValues) Expect(err).ToNot(HaveOccurred()) diff --git a/internal/upgrade/reconcilers/lcm.go b/internal/upgrade/reconcilers/lcm.go new file mode 100644 index 0000000..29f71c9 --- /dev/null +++ b/internal/upgrade/reconcilers/lcm.go @@ -0,0 +1,109 @@ +/* +Copyright © 2026 SUSE LLC +SPDX-License-Identifier: Apache-2.0 + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package reconcilers + +import ( + "context" + "fmt" + + lifecyclev1alpha1 "github.com/suse/elemental-lifecycle-manager/api/v1alpha1" + "github.com/suse/elemental-lifecycle-manager/internal/helm" + "github.com/suse/elemental-lifecycle-manager/internal/upgrade" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" +) + +const ( + ElementalLifecycleManagerChart = "elemental-lifecycle-manager" + ElementalLifecycleManagerCRDsChart = "elemental-lifecycle-manager-crds" +) + +type LCMReconciler struct { + helm *HelmReconciler +} + +// NewLCMReconciler creates a new LCM reconciler. +func NewLCMReconciler(c client.Client, h helm.Client) *LCMReconciler { + return &LCMReconciler{ + helm: NewHelmReconciler(c, h), + } +} + +func (r *LCMReconciler) Phase() upgrade.Phase { + return upgrade.PhaseLCM +} + +func (r *LCMReconciler) Reconcile(ctx context.Context, config *upgrade.Config) (*upgrade.PhaseStatus, error) { + if config == nil || config.HelmCharts == nil { + return r.Phase().SkippedStatus(), nil + } + logger := log.FromContext(ctx) + + r.helm.releaseName = config.ReleaseNamespacedName.Name + r.helm.releaseVersion = config.ReleaseVersion + + var lcmCharts []*upgrade.HelmChartConfig + for _, chartConfig := range config.HelmCharts { + name := chartConfig.Chart.GetName() + if isLCMChart(name) { + lcmCharts = append(lcmCharts, chartConfig) + } + } + if len(lcmCharts) == 0 { + return r.Phase().SkippedStatus(), nil + } + + orderedChartConfigs, err := sortChartConfigsByDependencies(lcmCharts) + if err != nil { + return &upgrade.PhaseStatus{ + State: lifecyclev1alpha1.UpgradeFailed, + Message: fmt.Sprintf("Failed to resolve chart dependencies: %v", err), + }, err + } + + logger.Info("Reconciling LCM charts and CRDs", "count", len(orderedChartConfigs)) + + var results []chartUpgradeResult + for _, chartConfig := range orderedChartConfigs { + name := chartConfig.Chart.GetName() + state, err := r.helm.reconcileChart(ctx, chartConfig) + if err != nil { + return &upgrade.PhaseStatus{ + State: lifecyclev1alpha1.UpgradeFailed, + Message: fmt.Sprintf("Failed to reconcile LCM chart %s: %v", name, err), + }, err + } + + results = append(results, chartUpgradeResult{ + chartName: name, + state: state, + }) + + if state == helm.ChartStateInProgress { + logger.Info("LCM chart upgrade in progress, waiting", "chart", name) + break + } + } + + return aggregateResults(results, len(orderedChartConfigs), "LCM"), nil +} + +// isLCMChart reports whether name is one of LCM's own charts. These are upgraded by the LCM phase and skipped by the Helm chart phase +func isLCMChart(name string) bool { + return name == ElementalLifecycleManagerCRDsChart || name == ElementalLifecycleManagerChart +} diff --git a/internal/upgrade/reconcilers/lcm_test.go b/internal/upgrade/reconcilers/lcm_test.go new file mode 100644 index 0000000..d540182 --- /dev/null +++ b/internal/upgrade/reconcilers/lcm_test.go @@ -0,0 +1,207 @@ +/* +Copyright © 2026 SUSE LLC +SPDX-License-Identifier: Apache-2.0 + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package reconcilers_test + +import ( + "context" + + helmv1 "github.com/k3s-io/helm-controller/pkg/apis/helm.cattle.io/v1" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + lifecyclev1alpha1 "github.com/suse/elemental-lifecycle-manager/api/v1alpha1" + "github.com/suse/elemental-lifecycle-manager/internal/helm" + "github.com/suse/elemental-lifecycle-manager/internal/upgrade" + "github.com/suse/elemental-lifecycle-manager/internal/upgrade/reconcilers" + "github.com/suse/elemental-lifecycle-manager/internal/upgrade/reconcilers/testutil" + "github.com/suse/elemental/v3/pkg/manifest/api" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +const ( + lcmChartName = "elemental-lifecycle-manager" + lcmCRDChartName = lcmChartName + "-crds" + lcmChartV1 = "v0.2.0" + lcmChartV2 = "v0.2.1" + lcmCRDChartV1 = "v0.1.0" + lcmCRDChartV2 = "v0.1.1" + testJobCRDS = "test-job-crds" + testJobLCM = "test-job-lcm" +) + +var _ = Describe("LCMReconciler", func() { + var ( + ctx context.Context + reconciler *reconcilers.LCMReconciler + fakeClient client.Client + mockHelm *testutil.MockHelmClient + scheme *runtime.Scheme + config *upgrade.Config + ) + + BeforeEach(func() { + ctx = context.Background() + scheme = testutil.NewTestScheme() + fakeClient = testutil.NewFakeClient(scheme) + mockHelm = testutil.NewMockHelmClient() + reconciler = reconcilers.NewLCMReconciler(fakeClient, mockHelm) + }) + + Describe("Phase", func() { + It("should return PhaseLCM", func() { + Expect(reconciler.Phase()).To(Equal(upgrade.PhaseLCM)) + }) + }) + + Describe("Reconcile", func() { + Context("When no LCM charts are in the list", func() { + It("should skip the upgrade progress", func() { + mockHelm.RetrieveReleaseFn = func(name string) (*helm.ReleaseInfo, error) { + return &helm.ReleaseInfo{ + ChartVersion: testChartVersion, + Namespace: testNamespace, + Config: map[string]any{}, + Revisions: 1, + }, nil + } + + chart1 := testutil.NewTestHelmChart(testChart1Name, testChartVersion) + config = testutil.NewTestConfig(testutil.WithHelmChartConfig([]*upgrade.HelmChartConfig{{Chart: chart1}})) + status, err := reconciler.Reconcile(ctx, config) + + Expect(err).ToNot(HaveOccurred()) + Expect(status).ToNot(BeNil()) + Expect(status.State).To(Equal(lifecyclev1alpha1.UpgradeSkipped)) + Expect(status.Message).To(Equal("Upgrade for phase \"LCM\" skipped")) + }) + + }) + + Context("when LCM charts exist", func() { + It("should skip chart not installed on cluster", func() { + mockHelm.RetrieveReleaseFn = func(name string) (*helm.ReleaseInfo, error) { + return nil, helm.ErrReleaseNotFound + } + + lcmChart := testutil.NewTestHelmChart(lcmChartName, lcmChartV1) + config = testutil.NewTestConfig(testutil.WithHelmChartConfig([]*upgrade.HelmChartConfig{{Chart: lcmChart}})) + status, err := reconciler.Reconcile(ctx, config) + + Expect(err).NotTo(HaveOccurred()) + Expect(status.State).To(Equal(lifecyclev1alpha1.UpgradeSucceeded)) + Expect(status.Message).To(ContainSubstring("All LCM charts skipped")) + }) + + It("should create a HelmChart resource with Release tracking labels", func() { + mockHelm.RetrieveReleaseFn = func(name string) (*helm.ReleaseInfo, error) { + return &helm.ReleaseInfo{ + ChartVersion: lcmChartV1, + Namespace: testNamespace, + Config: map[string]any{}, + Revisions: 1, + }, nil + } + + lcmChart := testutil.NewTestHelmChart(lcmChartName, lcmChartV2) + config := testutil.NewTestConfig(testutil.WithHelmChartConfig([]*upgrade.HelmChartConfig{{Chart: lcmChart}})) + status, err := reconciler.Reconcile(ctx, config) + + Expect(err).ToNot(HaveOccurred()) + Expect(status.State).To(Equal(lifecyclev1alpha1.UpgradeInProgress)) + + helmChart := &helmv1.HelmChart{} + Expect(fakeClient.Get(ctx, types.NamespacedName{ + Name: lcmChartName, + Namespace: reconcilers.HelmChartNamespace, + }, helmChart)).To(Succeed()) + + Expect(helmChart.Labels).To(HaveKeyWithValue(lifecyclev1alpha1.ReleaseNameLabel, config.ReleaseNamespacedName.Name)) + Expect(helmChart.Labels).To(HaveKeyWithValue(lifecyclev1alpha1.ReleaseVersionLabel, lifecyclev1alpha1.SanitizeVersion(config.ReleaseVersion))) + }) + + It("should upgrade LCM CRDs chart before the LCM chart", func() { + lcmChart := testutil.NewTestHelmChart(lcmChartName, lcmChartV2, testutil.WithDependencies([]api.HelmChartDependency{{Name: lcmCRDChartName, Type: api.DependencyTypeHelm}})) + lcmCRDChart := testutil.NewTestHelmChart(lcmCRDChartName, lcmCRDChartV2) + config := testutil.NewTestConfig(testutil.WithHelmChartConfig([]*upgrade.HelmChartConfig{ + { + Chart: lcmChart, + }, + { + Chart: lcmCRDChart, + }, + })) + + mockHelm.RetrieveReleaseFn = func(name string) (*helm.ReleaseInfo, error) { + switch name { + case lcmChartName: + return &helm.ReleaseInfo{ChartVersion: lcmChartV1, Namespace: testNamespace}, nil + case lcmCRDChartName: + return &helm.ReleaseInfo{ChartVersion: lcmCRDChartV1, Namespace: testNamespace}, nil + } + return nil, helm.ErrReleaseNotFound + } + + status, err := reconciler.Reconcile(ctx, config) + Expect(err).ToNot(HaveOccurred()) + Expect(status.State).To(Equal(lifecyclev1alpha1.UpgradeInProgress)) + + // HelmChart CR for LCM CRDs should be created first as it's a dependency of LCM chart. + helmLCMCRDChart := &helmv1.HelmChart{} + Expect(fakeClient.Get(ctx, types.NamespacedName{ + Name: lcmCRDChartName, + Namespace: reconcilers.HelmChartNamespace, + }, helmLCMCRDChart)).To(Succeed()) + + // HelmChart CR for LCM chart shouldn't be created yet + helmLCMChart := &helmv1.HelmChart{} + err = fakeClient.Get(ctx, types.NamespacedName{Name: lcmChartName, Namespace: reconcilers.HelmChartNamespace}, helmLCMChart) + Expect(apierrors.IsNotFound(err)).To(BeTrue()) + + helmLCMCRDChart.Status.JobName = testJobCRDS + Expect(fakeClient.Update(ctx, helmLCMCRDChart)).To(Succeed()) + + job1 := testutil.NewTestJob(testJobCRDS, reconcilers.HelmChartNamespace, true) + Expect(fakeClient.Create(ctx, job1)).To(Succeed()) + + status, err = reconciler.Reconcile(ctx, config) + Expect(err).ToNot(HaveOccurred()) + Expect(status.State).To(Equal(lifecyclev1alpha1.UpgradeInProgress)) + Expect(status.Message).To(Equal("LCM charts in progress (1/2 completed, 0 skipped)")) + + // Now the HelmChart CR for LCM should be created. + Expect(fakeClient.Get(ctx, types.NamespacedName{ + Name: lcmChartName, + Namespace: reconcilers.HelmChartNamespace, + }, helmLCMChart)).To(Succeed()) + + helmLCMChart.Status.JobName = testJobLCM + Expect(fakeClient.Update(ctx, helmLCMChart)).To(Succeed()) + + job2 := testutil.NewTestJob(testJobLCM, reconcilers.HelmChartNamespace, true) + Expect(fakeClient.Create(ctx, job2)).To(Succeed()) + + status, err = reconciler.Reconcile(ctx, config) + Expect(err).ToNot(HaveOccurred()) + Expect(status.State).To(Equal(lifecyclev1alpha1.UpgradeSucceeded)) + Expect(status.Message).To(Equal("All 2 LCM charts upgraded successfully (0 skipped)")) + }) + }) + }) +}) From aac34d9ed9b8e1b152f58817ab84ae237d810131 Mon Sep 17 00:00:00 2001 From: Dharmit Shah Date: Thu, 3 Sep 2026 14:27:51 +0530 Subject: [PATCH 2/2] Fail upgrade process if any of the LCM charts fails to upgrade Signed-off-by: Dharmit Shah --- internal/upgrade/reconcilers/helm.go | 12 ++--- internal/upgrade/reconcilers/lcm.go | 53 ++++++++++++++++++++- internal/upgrade/reconcilers/lcm_test.go | 60 +++++++++++++++++++++++- 3 files changed, 116 insertions(+), 9 deletions(-) diff --git a/internal/upgrade/reconcilers/helm.go b/internal/upgrade/reconcilers/helm.go index 5247198..43d7d99 100644 --- a/internal/upgrade/reconcilers/helm.go +++ b/internal/upgrade/reconcilers/helm.go @@ -138,7 +138,7 @@ func (r *HelmReconciler) reconcileHelmCharts(ctx context.Context, releaseName, r } } - return aggregateResults(results, len(orderedChartConfigs), "Helm"), nil + return aggregateResults(results, len(orderedChartConfigs)), nil } // sortChartConfigsByDependencies returns a sorted slice of chart configurations, @@ -413,11 +413,11 @@ func (r *HelmReconciler) evaluateHelmChartJobStatus(ctx context.Context, chart * } // aggregateResults aggregates chart upgrade results into a single PhaseStatus. -func aggregateResults(results []chartUpgradeResult, totalCharts int, chartKind string) *upgrade.PhaseStatus { +func aggregateResults(results []chartUpgradeResult, totalCharts int) *upgrade.PhaseStatus { if len(results) == 0 { return &upgrade.PhaseStatus{ State: lifecyclev1alpha1.UpgradeSucceeded, - Message: fmt.Sprintf("No %s charts to reconcile", chartKind), + Message: "No Helm charts to reconcile", } } @@ -450,20 +450,20 @@ func aggregateResults(results []chartUpgradeResult, totalCharts int, chartKind s if inProgress > 0 { return &upgrade.PhaseStatus{ State: lifecyclev1alpha1.UpgradeInProgress, - Message: fmt.Sprintf("%s charts in progress (%d/%d completed, %d skipped)", chartKind, succeeded, totalCharts-skipped, skipped), + Message: fmt.Sprintf("Helm charts in progress (%d/%d completed, %d skipped)", succeeded, totalCharts-skipped, skipped), } } if succeeded == 0 && skipped == totalCharts { return &upgrade.PhaseStatus{ State: lifecyclev1alpha1.UpgradeSucceeded, - Message: fmt.Sprintf("All %s charts skipped (not installed on cluster)", chartKind), + Message: "All Helm charts skipped (not installed on cluster)", } } return &upgrade.PhaseStatus{ State: lifecyclev1alpha1.UpgradeSucceeded, - Message: fmt.Sprintf("All %d %s charts upgraded successfully (%d skipped)", succeeded, chartKind, skipped), + Message: fmt.Sprintf("All %d Helm charts upgraded successfully (%d skipped)", succeeded, skipped), } } diff --git a/internal/upgrade/reconcilers/lcm.go b/internal/upgrade/reconcilers/lcm.go index 29f71c9..1f5138c 100644 --- a/internal/upgrade/reconcilers/lcm.go +++ b/internal/upgrade/reconcilers/lcm.go @@ -98,9 +98,60 @@ func (r *LCMReconciler) Reconcile(ctx context.Context, config *upgrade.Config) ( logger.Info("LCM chart upgrade in progress, waiting", "chart", name) break } + + if state == helm.ChartStateFailed { + // Failure in upgrading either LCM CRD or LCM chart should result in overall upgrade failure, + // and prevent moving forward to any other upgrade phase + return &upgrade.PhaseStatus{ + State: lifecyclev1alpha1.UpgradeFailed, + Message: fmt.Sprintf("Failed to upgrade LCM chart %q", name), + }, fmt.Errorf("upgrading LCM chart %q", name) + } + } + + return aggregateLCMResults(results, len(orderedChartConfigs)), nil +} + +// aggregateLCMResults aggregates chart upgrade results into a single PhaseStatus. +func aggregateLCMResults(results []chartUpgradeResult, totalCharts int) *upgrade.PhaseStatus { + if len(results) == 0 { + return &upgrade.PhaseStatus{ + State: lifecyclev1alpha1.UpgradeSucceeded, + Message: "No LCM charts to reconcile", + } } - return aggregateResults(results, len(orderedChartConfigs), "LCM"), nil + var inProgress, succeeded, skipped int + + for _, result := range results { + switch result.state { + case helm.ChartStateInProgress: + inProgress++ + case helm.ChartStateSucceeded, helm.ChartStateVersionAlreadyInstalled: + succeeded++ + case helm.ChartStateNotInstalled: + skipped++ + } + } + + if inProgress > 0 { + return &upgrade.PhaseStatus{ + State: lifecyclev1alpha1.UpgradeInProgress, + Message: fmt.Sprintf("LCM charts in progress (%d/%d completed, %d skipped)", succeeded, totalCharts-skipped, skipped), + } + } + + if succeeded == 0 && skipped == totalCharts { + return &upgrade.PhaseStatus{ + State: lifecyclev1alpha1.UpgradeSucceeded, + Message: "All LCM charts skipped (not installed on cluster)", + } + } + + return &upgrade.PhaseStatus{ + State: lifecyclev1alpha1.UpgradeSucceeded, + Message: fmt.Sprintf("All %d LCM charts upgraded successfully (%d skipped)", succeeded, skipped), + } } // isLCMChart reports whether name is one of LCM's own charts. These are upgraded by the LCM phase and skipped by the Helm chart phase diff --git a/internal/upgrade/reconcilers/lcm_test.go b/internal/upgrade/reconcilers/lcm_test.go index d540182..09c9208 100644 --- a/internal/upgrade/reconcilers/lcm_test.go +++ b/internal/upgrade/reconcilers/lcm_test.go @@ -29,6 +29,8 @@ import ( "github.com/suse/elemental-lifecycle-manager/internal/upgrade/reconcilers" "github.com/suse/elemental-lifecycle-manager/internal/upgrade/reconcilers/testutil" "github.com/suse/elemental/v3/pkg/manifest/api" + batchv1 "k8s.io/api/batch/v1" + corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -120,7 +122,7 @@ var _ = Describe("LCMReconciler", func() { } lcmChart := testutil.NewTestHelmChart(lcmChartName, lcmChartV2) - config := testutil.NewTestConfig(testutil.WithHelmChartConfig([]*upgrade.HelmChartConfig{{Chart: lcmChart}})) + config = testutil.NewTestConfig(testutil.WithHelmChartConfig([]*upgrade.HelmChartConfig{{Chart: lcmChart}})) status, err := reconciler.Reconcile(ctx, config) Expect(err).ToNot(HaveOccurred()) @@ -139,7 +141,7 @@ var _ = Describe("LCMReconciler", func() { It("should upgrade LCM CRDs chart before the LCM chart", func() { lcmChart := testutil.NewTestHelmChart(lcmChartName, lcmChartV2, testutil.WithDependencies([]api.HelmChartDependency{{Name: lcmCRDChartName, Type: api.DependencyTypeHelm}})) lcmCRDChart := testutil.NewTestHelmChart(lcmCRDChartName, lcmCRDChartV2) - config := testutil.NewTestConfig(testutil.WithHelmChartConfig([]*upgrade.HelmChartConfig{ + config = testutil.NewTestConfig(testutil.WithHelmChartConfig([]*upgrade.HelmChartConfig{ { Chart: lcmChart, }, @@ -202,6 +204,60 @@ var _ = Describe("LCMReconciler", func() { Expect(status.State).To(Equal(lifecyclev1alpha1.UpgradeSucceeded)) Expect(status.Message).To(Equal("All 2 LCM charts upgraded successfully (0 skipped)")) }) + + Context("when an LCM chart fails to upgrade", func() { + It("should fail if CRDs chart fails to upgrade", func() { + lcmChart := testutil.NewTestHelmChart(lcmChartName, lcmChartV2, testutil.WithDependencies([]api.HelmChartDependency{{Name: lcmCRDChartName, Type: api.DependencyTypeHelm}})) + lcmCRDChart := testutil.NewTestHelmChart(lcmCRDChartName, lcmCRDChartV2) + config = testutil.NewTestConfig(testutil.WithHelmChartConfig([]*upgrade.HelmChartConfig{ + { + Chart: lcmChart, + }, + { + Chart: lcmCRDChart, + }, + })) + + mockHelm.RetrieveReleaseFn = func(name string) (*helm.ReleaseInfo, error) { + switch name { + case lcmChartName: + return &helm.ReleaseInfo{ChartVersion: lcmChartV1, Namespace: testNamespace}, nil + case lcmCRDChartName: + return &helm.ReleaseInfo{ChartVersion: lcmCRDChartV1, Namespace: testNamespace}, nil + } + return nil, helm.ErrReleaseNotFound + } + + status, err := reconciler.Reconcile(ctx, config) + Expect(err).ToNot(HaveOccurred()) + Expect(status.State).To(Equal(lifecyclev1alpha1.UpgradeInProgress)) + + // HelmChart CR for LCM CRDs should be created first as it's a dependency of LCM chart. + helmLCMCRDChart := &helmv1.HelmChart{} + Expect(fakeClient.Get(ctx, types.NamespacedName{ + Name: lcmCRDChartName, + Namespace: reconcilers.HelmChartNamespace, + }, helmLCMCRDChart)).To(Succeed()) + + // HelmChart CR for LCM chart shouldn't be created yet + helmLCMChart := &helmv1.HelmChart{} + err = fakeClient.Get(ctx, types.NamespacedName{Name: lcmChartName, Namespace: reconcilers.HelmChartNamespace}, helmLCMChart) + Expect(apierrors.IsNotFound(err)).To(BeTrue()) + + helmLCMCRDChart.Status.JobName = testJobCRDS + Expect(fakeClient.Update(ctx, helmLCMCRDChart)).To(Succeed()) + + failedJob := testutil.NewTestJob(testJobCRDS, reconcilers.HelmChartNamespace, false) + failedJob.Status.Conditions = append(failedJob.Status.Conditions, batchv1.JobCondition{Type: batchv1.JobFailed, Status: corev1.ConditionTrue}) + Expect(fakeClient.Create(ctx, failedJob)).To(Succeed()) + + status, err = reconciler.Reconcile(ctx, config) + Expect(err).To(HaveOccurred()) + Expect(status.State).To(Equal(lifecyclev1alpha1.UpgradeFailed)) + Expect(status.Message).To(Equal("Failed to upgrade LCM chart \"elemental-lifecycle-manager-crds\"")) + }) + + }) }) }) })