From c7423211b706bb4064e30bed2fe25a0ab2a5c8c7 Mon Sep 17 00:00:00 2001 From: mengyipeng <413183498@qq.com> Date: Tue, 24 Sep 2024 14:25:17 +0800 Subject: [PATCH] improve fakeActionConfig by singleton mode, modify condition to only support upgrade from 'deployed' --- pkg/cmd/kubeblocks/upgrade.go | 9 +- pkg/cmd/kubeblocks/upgrade_test.go | 261 +++++++++++++++++++---------- pkg/util/helm/helm.go | 59 ++++--- pkg/util/helm/helm_test.go | 11 +- 4 files changed, 223 insertions(+), 117 deletions(-) diff --git a/pkg/cmd/kubeblocks/upgrade.go b/pkg/cmd/kubeblocks/upgrade.go index e20a01513..a189b05d9 100644 --- a/pkg/cmd/kubeblocks/upgrade.go +++ b/pkg/cmd/kubeblocks/upgrade.go @@ -23,12 +23,12 @@ import ( "context" "encoding/json" "fmt" - "helm.sh/helm/v3/pkg/release" "time" "github.com/hashicorp/go-version" "github.com/pkg/errors" "github.com/spf13/cobra" + "helm.sh/helm/v3/pkg/release" appsv1 "k8s.io/api/apps/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -94,12 +94,13 @@ func newUpgradeCmd(f cmdutil.Factory, streams genericiooptions.IOStreams) *cobra func (o *InstallOptions) Upgrade() error { klog.V(1).Info("##### Start to upgrade KubeBlocks #####") // check helm release status - status, err := helm.GetHelmReleaseStatus(o.HelmCfg, nil, types.KubeBlocksChartName) + status, err := helm.GetHelmReleaseStatus(o.HelmCfg, types.KubeBlocksChartName) if err != nil { return fmt.Errorf("failed to get Helm release status: %v", err) } - if status == release.StatusFailed { - return fmt.Errorf("helm release status is 'failed'. Please fix the release before upgrading KubeBlocks") + // intercept status except from 'deployed' + if status != release.StatusDeployed { + return fmt.Errorf("helm release status is %s instead of 'deployed'. Please fix the release before upgrading KubeBlocks", status) } if o.HelmCfg.Namespace() == "" { diff --git a/pkg/cmd/kubeblocks/upgrade_test.go b/pkg/cmd/kubeblocks/upgrade_test.go index d35a4d52f..a23fa61d5 100644 --- a/pkg/cmd/kubeblocks/upgrade_test.go +++ b/pkg/cmd/kubeblocks/upgrade_test.go @@ -45,110 +45,187 @@ var _ = Describe("kubeblocks upgrade", func() { var actionCfg *action.Configuration var cfg *helm.Config - BeforeEach(func() { - streams, _, _, _ = genericiooptions.NewTestIOStreams() - tf = cmdtesting.NewTestFactory().WithNamespace(namespace) - tf.Client = &clientfake.RESTClient{} - }) + Context("Upgrade", func() { + BeforeEach(func() { + streams, _, _, _ = genericiooptions.NewTestIOStreams() + tf = cmdtesting.NewTestFactory().WithNamespace(namespace) + tf.Client = &clientfake.RESTClient{} + cfg = helm.NewFakeConfig(namespace) + actionCfg, _ = helm.NewActionConfig(cfg) + err := actionCfg.Releases.Create(&release.Release{ + Name: testing.KubeBlocksChartName, + Namespace: namespace, + Version: 1, + Info: &release.Info{ + Status: release.StatusDeployed, + }, + Chart: &chart.Chart{}, + }) + Expect(err).Should(BeNil()) - AfterEach(func() { - tf.Cleanup() - }) + }) - mockKubeBlocksDeploy := func() *appsv1.Deployment { - deploy := &appsv1.Deployment{} - deploy.SetLabels(map[string]string{ - "app.kubernetes.io/component": "apps", - "app.kubernetes.io/name": types.KubeBlocksChartName, - "app.kubernetes.io/version": "0.3.0", + AfterEach(func() { + helm.ResetFakeActionConfig() + tf.Cleanup() }) - return deploy - } - - It("check upgrade", func() { - var cfg string - cmd = newUpgradeCmd(tf, streams) - Expect(cmd).ShouldNot(BeNil()) - Expect(cmd.HasSubCommands()).Should(BeFalse()) - - o := &InstallOptions{ - Options: Options{ - IOStreams: streams, - }, + + mockKubeBlocksDeploy := func() *appsv1.Deployment { + deploy := &appsv1.Deployment{} + deploy.SetLabels(map[string]string{ + "app.kubernetes.io/component": "apps", + "app.kubernetes.io/name": types.KubeBlocksChartName, + "app.kubernetes.io/version": "0.3.0", + }) + return deploy } - By("command without kubeconfig flag") - Expect(o.Complete(tf, cmd)).Should(HaveOccurred()) + It("check upgrade", func() { + var cfg string + cmd = newUpgradeCmd(tf, streams) + Expect(cmd).ShouldNot(BeNil()) + Expect(cmd.HasSubCommands()).Should(BeFalse()) + + o := &InstallOptions{ + Options: Options{ + IOStreams: streams, + }, + } + + By("command without kubeconfig flag") + Expect(o.Complete(tf, cmd)).Should(HaveOccurred()) + + cmd.Flags().StringVar(&cfg, "kubeconfig", "", "Path to the kubeconfig file to use for CLI requests.") + cmd.Flags().StringVar(&cfg, "context", "", "The name of the kubeconfig context to use.") + Expect(o.Complete(tf, cmd)).To(Succeed()) + Expect(o.HelmCfg).ShouldNot(BeNil()) + Expect(o.Namespace).To(Equal("test")) + }) - cmd.Flags().StringVar(&cfg, "kubeconfig", "", "Path to the kubeconfig file to use for CLI requests.") - cmd.Flags().StringVar(&cfg, "context", "", "The name of the kubeconfig context to use.") - Expect(o.Complete(tf, cmd)).To(Succeed()) - Expect(o.HelmCfg).ShouldNot(BeNil()) - Expect(o.Namespace).To(Equal("test")) - }) + It("double-check when version change", func() { + o := &InstallOptions{ + Options: Options{ + IOStreams: streams, + HelmCfg: helm.NewFakeConfig(namespace), + Namespace: "default", + Client: testing.FakeClientSet(mockKubeBlocksDeploy()), + Dynamic: testing.FakeDynamicClient(), + }, + Version: "0.5.0-fake", + Check: false, + } + Expect(o.Upgrade()).Should(HaveOccurred()) + // o.In = bytes.NewBufferString("fake-version") mock input error + // Expect(o.Upgrade()).Should(Succeed()) + o.autoApprove = true + Expect(o.Upgrade()).Should(Succeed()) - It("double-check when version change", func() { - o := &InstallOptions{ - Options: Options{ - IOStreams: streams, - HelmCfg: helm.NewFakeConfig(namespace), - Namespace: "default", - Client: testing.FakeClientSet(mockKubeBlocksDeploy()), - Dynamic: testing.FakeDynamicClient(), - }, - Version: "0.5.0-fake", - Check: false, - } - Expect(o.Upgrade()).Should(HaveOccurred()) - // o.In = bytes.NewBufferString("fake-version") mock input error - // Expect(o.Upgrade()).Should(Succeed()) - o.autoApprove = true - Expect(o.Upgrade()).Should(Succeed()) + }) - }) + It("helm ValueOpts upgrade", func() { + o := &InstallOptions{ + Options: Options{ + IOStreams: streams, + HelmCfg: helm.NewFakeConfig(namespace), + Namespace: "default", + Client: testing.FakeClientSet(mockKubeBlocksDeploy()), + Dynamic: testing.FakeDynamicClient(), + }, + Version: "", + } + o.ValueOpts.Values = []string{"replicaCount=2"} + Expect(o.Upgrade()).Should(Succeed()) + }) - It("helm ValueOpts upgrade", func() { - o := &InstallOptions{ - Options: Options{ - IOStreams: streams, - HelmCfg: helm.NewFakeConfig(namespace), - Namespace: "default", - Client: testing.FakeClientSet(mockKubeBlocksDeploy()), - Dynamic: testing.FakeDynamicClient(), - }, - Version: "", - } - o.ValueOpts.Values = []string{"replicaCount=2"} - Expect(o.Upgrade()).Should(Succeed()) + It("run upgrade", func() { + o := &InstallOptions{ + Options: Options{ + IOStreams: streams, + HelmCfg: cfg, + Namespace: "default", + Client: testing.FakeClientSet(mockKubeBlocksDeploy()), + Dynamic: testing.FakeDynamicClient(), + }, + Version: version.DefaultKubeBlocksVersion, + Check: false, + } + Expect(o.Upgrade()).Should(Succeed()) + Expect(len(o.ValueOpts.Values)).To(Equal(0)) + Expect(o.upgradeChart()).Should(Succeed()) + }) }) - It("run upgrade", func() { - cfg = helm.NewFakeConfig(namespace) - actionCfg, _ = helm.NewActionConfig(cfg) - err := actionCfg.Releases.Create(&release.Release{ - Name: testing.KubeBlocksChartName, - Namespace: namespace, - Version: 1, - Info: &release.Info{ - Status: release.StatusDeployed, - }, - Chart: &chart.Chart{}, + Context("upgrade from different status", func() { + BeforeEach(func() { + streams, _, _, _ = genericiooptions.NewTestIOStreams() + tf = cmdtesting.NewTestFactory().WithNamespace(namespace) + tf.Client = &clientfake.RESTClient{} + cfg = helm.NewFakeConfig(namespace) + actionCfg, _ = helm.NewActionConfig(cfg) + }) + + AfterEach(func() { + helm.ResetFakeActionConfig() + tf.Cleanup() }) - Expect(err).Should(BeNil()) - - o := &InstallOptions{ - Options: Options{ - IOStreams: streams, - HelmCfg: cfg, - Namespace: "default", - Client: testing.FakeClientSet(mockKubeBlocksDeploy()), - Dynamic: testing.FakeDynamicClient(), - }, - Version: version.DefaultKubeBlocksVersion, - Check: false, + + mockKubeBlocksDeploy := func() *appsv1.Deployment { + deploy := &appsv1.Deployment{} + deploy.SetLabels(map[string]string{ + "app.kubernetes.io/component": "apps", + "app.kubernetes.io/name": types.KubeBlocksChartName, + "app.kubernetes.io/version": "0.3.0", + }) + return deploy } - Expect(o.Upgrade()).Should(Succeed()) - Expect(len(o.ValueOpts.Values)).To(Equal(0)) - Expect(o.upgradeChart()).Should(Succeed()) + It("run upgrade", func() { + testCase := []struct { + status release.Status + checkResult bool + }{ + {release.StatusDeployed, true}, + {release.StatusSuperseded, false}, + {release.StatusUnknown, false}, + {release.StatusUninstalled, false}, + {release.StatusFailed, false}, + {release.StatusUninstalling, false}, + {release.StatusPendingInstall, false}, + {release.StatusPendingUpgrade, false}, + {release.StatusPendingRollback, false}, + } + + for i := range testCase { + actionCfg, _ = helm.NewActionConfig(cfg) + err := actionCfg.Releases.Create(&release.Release{ + Name: testing.KubeBlocksChartName, + Namespace: namespace, + Version: 1, + Info: &release.Info{ + Status: testCase[i].status, + }, + Chart: &chart.Chart{}, + }) + Expect(err).Should(BeNil()) + o := &InstallOptions{ + Options: Options{ + IOStreams: streams, + HelmCfg: cfg, + Namespace: "default", + Client: testing.FakeClientSet(mockKubeBlocksDeploy()), + Dynamic: testing.FakeDynamicClient(), + }, + Version: version.DefaultKubeBlocksVersion, + Check: false, + } + if testCase[i].checkResult { + Expect(o.Upgrade()).Should(Succeed()) + } else { + Expect(o.Upgrade()).Should(HaveOccurred()) + } + helm.ResetFakeActionConfig() + } + + }) }) + }) diff --git a/pkg/util/helm/helm.go b/pkg/util/helm/helm.go index b813b8b1b..50859aaec 100644 --- a/pkg/util/helm/helm.go +++ b/pkg/util/helm/helm.go @@ -28,6 +28,7 @@ import ( "os/signal" "path/filepath" "strings" + "sync" "syscall" "time" @@ -440,23 +441,44 @@ func NewActionConfig(cfg *Config) (*action.Configuration, error) { return actionCfg, nil } +var ( + singletonFakeCfg *action.Configuration + singletonFakeMu sync.Mutex +) + +// fakeActionConfig returns a singleton instance of action.Configuration func fakeActionConfig() *action.Configuration { - registryClient, err := registry.NewClient() - if err != nil { - return nil + if singletonFakeCfg != nil { + return singletonFakeCfg } - res := &action.Configuration{ - Releases: storage.Init(driver.NewMemory()), - KubeClient: &kubefake.FailingKubeClient{PrintingKubeClient: kubefake.PrintingKubeClient{Out: io.Discard}}, - Capabilities: chartutil.DefaultCapabilities, - RegistryClient: registryClient, - Log: func(format string, v ...interface{}) {}, + singletonFakeMu.Lock() + defer singletonFakeMu.Unlock() + + if singletonFakeCfg == nil { + registryClient, err := registry.NewClient() + if err != nil { + return nil + } + + singletonFakeCfg = &action.Configuration{ + Releases: storage.Init(driver.NewMemory()), + KubeClient: &kubefake.FailingKubeClient{PrintingKubeClient: kubefake.PrintingKubeClient{Out: io.Discard}}, + Capabilities: chartutil.DefaultCapabilities, + RegistryClient: registryClient, + Log: func(format string, v ...interface{}) {}, + } + singletonFakeCfg.Capabilities.KubeVersion.Version = "v99.99.0" } - // to template the kubeblocks manifest, dry-run install will check and valida the KubeVersion in Capabilities is bigger than - // the KubeVersion in Chart.yaml. Set a max KubeVersion to avoid the check fail. - res.Capabilities.KubeVersion.Version = "v99.99.0" - return res + + return singletonFakeCfg +} + +// ResetFakeActionConfig resets the singleton action.Configuration instance +func ResetFakeActionConfig() { + singletonFakeMu.Lock() + defer singletonFakeMu.Unlock() + singletonFakeCfg = nil } // Upgrade will upgrade a Chart @@ -692,13 +714,10 @@ func GetTemplateInstallOps(name, chart, version, namespace string) *InstallOpts } // GetHelmReleaseStatus retrieves the status of a Helm release within a specified namespace. -func GetHelmReleaseStatus(cfg *Config, actionCfg *action.Configuration, releaseName string) (release.Status, error) { - var err error - if actionCfg == nil { - actionCfg, err = NewActionConfig(cfg) - if err != nil { - return "", err - } +func GetHelmReleaseStatus(cfg *Config, releaseName string) (release.Status, error) { + actionCfg, err := NewActionConfig(cfg) + if err != nil { + return "", err } client := action.NewGet(actionCfg) rel, err := client.Run(releaseName) diff --git a/pkg/util/helm/helm_test.go b/pkg/util/helm/helm_test.go index d49be98ae..e610c3067 100644 --- a/pkg/util/helm/helm_test.go +++ b/pkg/util/helm/helm_test.go @@ -68,6 +68,10 @@ var _ = Describe("helm util", func() { Expect(actionCfg).ShouldNot(BeNil()) }) + AfterEach(func() { + ResetFakeActionConfig() + }) + It("Install", func() { _, err := o.Install(cfg) Expect(err).Should(HaveOccurred()) @@ -119,6 +123,10 @@ var _ = Describe("helm util", func() { Expect(actionCfg).ShouldNot(BeNil()) }) + AfterEach(func() { + ResetFakeActionConfig() + }) + It("should fail when release is not found", func() { Expect(ReleaseNotFound(o.Upgrade(cfg))).Should(BeTrue()) Expect(o.Uninstall(cfg)).Should(HaveOccurred()) // release not found @@ -185,9 +193,10 @@ var _ = Describe("helm util", func() { }) Expect(err).Should(BeNil()) _, _ = GetValues("", cfg) - status, err := GetHelmReleaseStatus(cfg, actionCfg, o.Name) + status, err := GetHelmReleaseStatus(cfg, o.Name) Expect(status).Should(Equal(release.StatusFailed)) Expect(err).Should(BeNil()) + ResetFakeActionConfig() }) })