diff --git a/api/v2/checluster_types.go b/api/v2/checluster_types.go index c0d0b327d..72f6c7f33 100644 --- a/api/v2/checluster_types.go +++ b/api/v2/checluster_types.go @@ -873,3 +873,9 @@ func (c *CheCluster) IsEmbeddedOpenVSXRegistryConfigured() bool { } return openVSXURL == "" } + +// IsCheBeingInstalled returns true if the Che version is not set in the status. +// Basically it means that the Che is being installed since the Che version is set only after the installation. +func (c *CheCluster) IsCheBeingInstalled() bool { + return c.Status.CheVersion == "" +} diff --git a/go.mod b/go.mod index 3162c1d3a..13f458f13 100644 --- a/go.mod +++ b/go.mod @@ -15,7 +15,6 @@ require ( github.com/sirupsen/logrus v1.8.1 github.com/stretchr/testify v1.8.0 go.uber.org/zap v1.24.0 - golang.org/x/mod v0.6.0 golang.org/x/net v0.7.0 k8s.io/api v0.26.1 k8s.io/apiextensions-apiserver v0.26.1 @@ -92,6 +91,7 @@ require ( github.com/konsorten/go-windows-terminal-sequences v1.0.2 // indirect go.uber.org/tools v0.0.0-20190618225709-2cfd321de3ee // indirect golang.org/x/lint v0.0.0-20200302205851-738671d3881b // indirect + golang.org/x/mod v0.6.0 // indirect golang.org/x/tools v0.2.0 // indirect golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect honnef.co/go/tools v0.0.1-2020.1.3 // indirect diff --git a/pkg/deploy/migration/checluster-defaults-cleaner.go b/pkg/deploy/migration/checluster-defaults-cleaner.go index 090634bf0..db8d3b767 100644 --- a/pkg/deploy/migration/checluster-defaults-cleaner.go +++ b/pkg/deploy/migration/checluster-defaults-cleaner.go @@ -86,19 +86,19 @@ func (dc *CheClusterDefaultsCleaner) Reconcile(ctx *chetypes.DeployContext) (rec continue } - if done, err := cleanUpTask.cleanUpFunc(ctx); err != nil { - return reconcile.Result{}, false, err - } else { - // set annotation to mark that the field has been processed - dc.setCheClusterDefaultsCleanupAnnotation(ctx, cleanUpTask.fieldsIdentifier) - if err := ctx.ClusterAPI.Client.Update(context.TODO(), ctx.CheCluster); err != nil { + if !ctx.CheCluster.IsCheBeingInstalled() { + if done, err := cleanUpTask.cleanUpFunc(ctx); err != nil { return reconcile.Result{}, false, err - } - - if done { + } else if done { logger.Info("CheCluster CR cleaned up", "field", cleanUpTask.fieldsIdentifier) } } + + // set annotation to mark that the field has been processed + dc.setCheClusterDefaultsCleanupAnnotation(ctx, cleanUpTask.fieldsIdentifier) + if err := ctx.ClusterAPI.Client.Update(context.TODO(), ctx.CheCluster); err != nil { + return reconcile.Result{}, false, err + } } return reconcile.Result{}, true, nil diff --git a/pkg/deploy/migration/checluster-defaults-cleaner_test.go b/pkg/deploy/migration/checluster-defaults-cleaner_test.go index f77d66f2b..e114cb75c 100644 --- a/pkg/deploy/migration/checluster-defaults-cleaner_test.go +++ b/pkg/deploy/migration/checluster-defaults-cleaner_test.go @@ -32,6 +32,98 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +func TestCheClusterDefaultsCleanerShouldNotChangeValuesOnInstallation(t *testing.T) { + type testCase struct { + name string + infra infrastructure.Type + cheCluster *chev2.CheCluster + } + + zeroResource := resource.MustParse("0") + + testCases := []testCase{ + { + name: "Che is being installed, nothing changed", + infra: infrastructure.OpenShiftv4, + cheCluster: &chev2.CheCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "eclipse-che", + Namespace: "eclipse-che", + }, + Spec: chev2.CheClusterSpec{ + Components: chev2.CheClusterComponents{ + CheServer: chev2.CheServer{ + Deployment: &chev2.Deployment{ + Containers: []chev2.Container{ + { + Resources: &chev2.ResourceRequirements{ + Requests: &chev2.ResourceList{ + Memory: &zeroResource, + Cpu: &zeroResource, + }, + Limits: &chev2.ResourceList{ + Memory: &zeroResource, + Cpu: &zeroResource, + }, + }, + }, + }, + }, + }, + PluginRegistry: chev2.PluginRegistry{ + OpenVSXURL: pointer.String("https://open-vsx.org"), + }, + Dashboard: chev2.Dashboard{ + HeaderMessage: &chev2.DashboardHeaderMessage{ + Text: "Some message", + Show: true, + }, + }, + }, + DevEnvironments: chev2.CheClusterDevEnvironments{ + DisableContainerBuildCapabilities: pointer.Bool(false), + DefaultEditor: "che-incubator/che-code/insiders", + DefaultComponents: []devfile.Component{ + { + Name: "universal-developer-image", + ComponentUnion: devfile.ComponentUnion{ + Container: &devfile.ContainerComponent{ + Container: devfile.Container{ + Image: "quay.io/devfile/universal-developer-image:ubi8-38da5c2", + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + infrastructure.InitializeForTesting(testCase.infra) + + cheClusterCopy := testCase.cheCluster.DeepCopy() + + ctx := test.GetDeployContext(testCase.cheCluster, []runtime.Object{}) + cheClusterDefaultsCleanup := NewCheClusterDefaultsCleaner() + + _, done, err := cheClusterDefaultsCleanup.Reconcile(ctx) + assert.NoError(t, err) + assert.True(t, done) + + assert.Equal(t, cheClusterCopy.Spec.DevEnvironments.DefaultEditor, ctx.CheCluster.Spec.DevEnvironments.DefaultEditor) + assert.Equal(t, cheClusterCopy.Spec.DevEnvironments.DefaultComponents, ctx.CheCluster.Spec.DevEnvironments.DefaultComponents) + assert.Equal(t, cheClusterCopy.Spec.DevEnvironments.DisableContainerBuildCapabilities, ctx.CheCluster.Spec.DevEnvironments.DisableContainerBuildCapabilities) + assert.Equal(t, cheClusterCopy.Spec.Components.PluginRegistry.OpenVSXURL, ctx.CheCluster.Spec.Components.PluginRegistry.OpenVSXURL) + assert.Equal(t, cheClusterCopy.Spec.Components.Dashboard.HeaderMessage, ctx.CheCluster.Spec.Components.Dashboard.HeaderMessage) + assert.Equal(t, cheClusterCopy.Spec.Components.CheServer.Deployment.Containers[0].Resources, ctx.CheCluster.Spec.Components.CheServer.Deployment.Containers[0].Resources) + }) + } +} + func TestCheClusterDefaultsCleanerDefaultEditor(t *testing.T) { type testCase struct { name string @@ -49,6 +141,9 @@ func TestCheClusterDefaultsCleanerDefaultEditor(t *testing.T) { Name: "eclipse-che", Namespace: "eclipse-che", }, + Status: chev2.CheClusterStatus{ + CheVersion: "next", + }, }, expectedDefaultEditor: "", }, @@ -65,6 +160,9 @@ func TestCheClusterDefaultsCleanerDefaultEditor(t *testing.T) { DefaultEditor: "che-incubator/che-code/insiders", }, }, + Status: chev2.CheClusterStatus{ + CheVersion: "next", + }, }, expectedDefaultEditor: "", }, @@ -81,6 +179,9 @@ func TestCheClusterDefaultsCleanerDefaultEditor(t *testing.T) { DefaultEditor: "my/editor", }, }, + Status: chev2.CheClusterStatus{ + CheVersion: "next", + }, }, expectedDefaultEditor: "my/editor", }, @@ -122,13 +223,16 @@ func TestCheClusterDefaultsCleanerDefaultComponents(t *testing.T) { testCases := []testCase{ { - name: "Case #2", + name: "Case #1", infra: infrastructure.OpenShiftv4, cheCluster: &chev2.CheCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "eclipse-che", Namespace: "eclipse-che", }, + Status: chev2.CheClusterStatus{ + CheVersion: "next", + }, }, expectedDefaultComponents: nil, }, @@ -156,6 +260,9 @@ func TestCheClusterDefaultsCleanerDefaultComponents(t *testing.T) { }, }, }, + Status: chev2.CheClusterStatus{ + CheVersion: "next", + }, }, expectedDefaultComponents: nil, }, @@ -183,6 +290,9 @@ func TestCheClusterDefaultsCleanerDefaultComponents(t *testing.T) { }, }, }, + Status: chev2.CheClusterStatus{ + CheVersion: "next", + }, }, expectedDefaultComponents: []devfile.Component{ { @@ -234,7 +344,7 @@ func TestCheClusterDefaultsCleanerOpenVSXURL(t *testing.T) { testCases := []testCase{ { - name: "Test upgrade from next", + name: "Case #1", cheCluster: &chev2.CheCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "eclipse-che", @@ -253,68 +363,7 @@ func TestCheClusterDefaultsCleanerOpenVSXURL(t *testing.T) { }, }, { - name: "Test upgrade from v7.52.0", - cheCluster: &chev2.CheCluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "eclipse-che", - Namespace: "eclipse-che", - }, - Status: chev2.CheClusterStatus{ - CheVersion: "7.52.0", - }, - }, - expectedOpenVSXURL: pointer.StringPtr(defaults.GetPluginRegistryOpenVSXURL()), - }, - { - name: "Test upgrade from v7.62.0", - cheCluster: &chev2.CheCluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "eclipse-che", - Namespace: "eclipse-che", - }, - Spec: chev2.CheClusterSpec{ - Components: chev2.CheClusterComponents{ - PluginRegistry: chev2.PluginRegistry{ - OpenVSXURL: pointer.StringPtr("https://open-vsx.org"), - }, - }, - }, - Status: chev2.CheClusterStatus{ - CheVersion: "7.62.0", - }, - }, - }, - { - name: "Test installing a new version", - cheCluster: &chev2.CheCluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "eclipse-che", - Namespace: "eclipse-che", - }, - Spec: chev2.CheClusterSpec{ - Components: chev2.CheClusterComponents{ - PluginRegistry: chev2.PluginRegistry{ - OpenVSXURL: pointer.StringPtr("https://open-vsx.org"), - }, - }, - }, - }, - }, - { - name: "Test use embedded OpenVSXURL after upgrade", - cheCluster: &chev2.CheCluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "eclipse-che", - Namespace: "eclipse-che", - }, - Status: chev2.CheClusterStatus{ - CheVersion: "7.62.0", - }, - }, - expectedOpenVSXURL: pointer.StringPtr(""), - }, - { - name: "Keep existed OpenVSXURL after upgrade", + name: "Case #2", cheCluster: &chev2.CheCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "eclipse-che", @@ -327,9 +376,32 @@ func TestCheClusterDefaultsCleanerOpenVSXURL(t *testing.T) { }, }, }, + Status: chev2.CheClusterStatus{ + CheVersion: "next", + }, }, expectedOpenVSXURL: pointer.StringPtr("https://bla-bla-bla"), }, + { + name: "Case #2", + cheCluster: &chev2.CheCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "eclipse-che", + Namespace: "eclipse-che", + }, + Spec: chev2.CheClusterSpec{ + Components: chev2.CheClusterComponents{ + PluginRegistry: chev2.PluginRegistry{ + OpenVSXURL: pointer.StringPtr(""), + }, + }, + }, + Status: chev2.CheClusterStatus{ + CheVersion: "next", + }, + }, + expectedOpenVSXURL: pointer.StringPtr(""), + }, } for _, testCase := range testCases { @@ -384,6 +456,9 @@ func TestCheClusterDefaultsCleanerDashboardHeaderMessage(t *testing.T) { Name: "eclipse-che", Namespace: "eclipse-che", }, + Status: chev2.CheClusterStatus{ + CheVersion: "next", + }, }, expectedHeaderMessage: nil, }, @@ -405,6 +480,9 @@ func TestCheClusterDefaultsCleanerDashboardHeaderMessage(t *testing.T) { }, }, }, + Status: chev2.CheClusterStatus{ + CheVersion: "next", + }, }, expectedHeaderMessage: &chev2.DashboardHeaderMessage{ Text: "Some message", @@ -429,6 +507,9 @@ func TestCheClusterDefaultsCleanerDashboardHeaderMessage(t *testing.T) { }, }, }, + Status: chev2.CheClusterStatus{ + CheVersion: "next", + }, }, expectedHeaderMessage: nil, }, @@ -477,6 +558,9 @@ func TestCheClusterDefaultsCleanerDisableContainerBuildCapabilities(t *testing.T Name: "eclipse-che", Namespace: "eclipse-che", }, + Status: chev2.CheClusterStatus{ + CheVersion: "next", + }, }, expectedDisableContainerBuildCapabilities: pointer.BoolPtr(true), }, @@ -493,6 +577,9 @@ func TestCheClusterDefaultsCleanerDisableContainerBuildCapabilities(t *testing.T DisableContainerBuildCapabilities: pointer.BoolPtr(false), }, }, + Status: chev2.CheClusterStatus{ + CheVersion: "next", + }, }, expectedDisableContainerBuildCapabilities: pointer.BoolPtr(true), }, @@ -504,6 +591,9 @@ func TestCheClusterDefaultsCleanerDisableContainerBuildCapabilities(t *testing.T Name: "eclipse-che", Namespace: "eclipse-che", }, + Status: chev2.CheClusterStatus{ + CheVersion: "next", + }, }, expectedDisableContainerBuildCapabilities: nil, }, @@ -520,25 +610,12 @@ func TestCheClusterDefaultsCleanerDisableContainerBuildCapabilities(t *testing.T DisableContainerBuildCapabilities: pointer.BoolPtr(true), }, }, + Status: chev2.CheClusterStatus{ + CheVersion: "next", + }, }, expectedDisableContainerBuildCapabilities: pointer.BoolPtr(true), }, - { - name: "OpenShift case #3", - infra: infrastructure.OpenShiftv4, - cheCluster: &chev2.CheCluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "eclipse-che", - Namespace: "eclipse-che", - }, - Spec: chev2.CheClusterSpec{ - DevEnvironments: chev2.CheClusterDevEnvironments{ - DisableContainerBuildCapabilities: pointer.BoolPtr(false), - }, - }, - }, - expectedDisableContainerBuildCapabilities: nil, - }, } for _, testCase := range testCases { @@ -586,6 +663,9 @@ func TestCheClusterDefaultsCleanerContainerResources(t *testing.T) { Name: "eclipse-che", Namespace: "eclipse-che", }, + Status: chev2.CheClusterStatus{ + CheVersion: "next", + }, }, expectedDeployment: nil, }, @@ -617,6 +697,9 @@ func TestCheClusterDefaultsCleanerContainerResources(t *testing.T) { }, }, }, + Status: chev2.CheClusterStatus{ + CheVersion: "next", + }, }, expectedDeployment: &chev2.Deployment{ Containers: []chev2.Container{ @@ -662,6 +745,9 @@ func TestCheClusterDefaultsCleanerContainerResources(t *testing.T) { }, }, }, + Status: chev2.CheClusterStatus{ + CheVersion: "next", + }, }, expectedDeployment: &chev2.Deployment{ Containers: []chev2.Container{ diff --git a/pkg/deploy/migration/checluster-defaults-cleanupfunc.go b/pkg/deploy/migration/checluster-defaults-cleanupfunc.go index ae2d47cf3..ce83a7c3b 100644 --- a/pkg/deploy/migration/checluster-defaults-cleanupfunc.go +++ b/pkg/deploy/migration/checluster-defaults-cleanupfunc.go @@ -14,7 +14,6 @@ package migration import ( "encoding/json" - "fmt" "strconv" chev2 "github.com/eclipse-che/che-operator/api/v2" @@ -25,8 +24,6 @@ import ( "github.com/google/go-cmp/cmp/cmpopts" devfile "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" - "golang.org/x/mod/semver" - "github.com/eclipse-che/che-operator/pkg/common/chetypes" "github.com/google/go-cmp/cmp" "k8s.io/utils/pointer" @@ -97,16 +94,7 @@ func cleanUpDashboardHeaderMessage(ctx *chetypes.DeployContext) (bool, error) { return false, nil } -// cleanUpPluginRegistryOpenVSXURL cleans up CheCluster CR `Spec.Components.PluginRegistry.OpenVSXURL` field: -// (complies with requirements https://github.com/eclipse/che/issues/21637): -// 1. if value equals to the default one, then set it to nil -// 2. if Eclipse Che is being installed, then use the default openVSXURL -// 3. if Eclipse Che is being upgraded -// * if value is and Eclipse Che v7.52 or earlier, then set the default -// * if value is and Eclipse Che v7.53 or later, then set it to empty string (starts embedded registry) -// * if value is , then do nothing (starts embedded registry) -// * if value is and not equals to the default value, then do nothing (use external registry from the value) -// See also [v2.CheCluster]. +// cleanUpPluginRegistryOpenVSXURL cleans up CheCluster CR `Spec.Components.PluginRegistry.OpenVSXURL` field. func cleanUpPluginRegistryOpenVSXURL(ctx *chetypes.DeployContext) (bool, error) { pluginRegistryOpenVSXURL := []string{ "https://openvsx.org", // redirects to "https://open-vsx.org" @@ -123,31 +111,6 @@ func cleanUpPluginRegistryOpenVSXURL(ctx *chetypes.DeployContext) (bool, error) } } - // Eclipse Che is being installed - if ctx.CheCluster.Status.CheVersion == "" { - if ctx.CheCluster.IsAirGapMode() { - ctx.CheCluster.Spec.Components.PluginRegistry.OpenVSXURL = pointer.StringPtr("") - return true, nil - } - - return false, nil - } - - // Eclipse Che is being upgraded - if ctx.CheCluster.Spec.Components.PluginRegistry.OpenVSXURL == nil { - if ctx.CheCluster.IsCheFlavor() && - ctx.CheCluster.Status.CheVersion != "" && - ctx.CheCluster.Status.CheVersion != "next" && - semver.Compare(fmt.Sprintf("v%s", ctx.CheCluster.Status.CheVersion), "v7.53.0") == -1 { - // Eclipse Che is being updated from version v7.52 or earlier - ctx.CheCluster.Spec.Components.PluginRegistry.OpenVSXURL = pointer.StringPtr(defaults.GetPluginRegistryOpenVSXURL()) - return true, nil - } - - ctx.CheCluster.Spec.Components.PluginRegistry.OpenVSXURL = pointer.StringPtr("") - return true, nil - } - return false, nil }