fix: Don't clean up `openVSXURL` on installation (#1716)

* fix: `CheClusterDefaultsCleaner` does not clean up CheCluster CR values on a fresh installation

Signed-off-by: Anatolii Bazko <abazko@redhat.com>
pull/1719/head
Anatolii Bazko 2023-07-04 13:49:08 +02:00 committed by GitHub
parent b5cdfc4108
commit 4db565cb8e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 183 additions and 128 deletions

View File

@ -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 == ""
}

2
go.mod
View File

@ -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

View File

@ -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

View File

@ -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{

View File

@ -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 <not set> and Eclipse Che v7.52 or earlier, then set the default
// * if value is <not set> and Eclipse Che v7.53 or later, then set it to empty string (starts embedded registry)
// * if value is <empty>, then do nothing (starts embedded registry)
// * if value is <non-empty> 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
}