From 1f1a41f9b380fb2ba05a42e6f449d756a583fa28 Mon Sep 17 00:00:00 2001 From: Lukas Krejci Date: Fri, 2 Jul 2021 00:28:32 +0200 Subject: [PATCH] feat: Don't deploy CheManager - it no longer is a thing. (#868) Remove the CRDs that we don't have to deploy. CheManager is not used at all anymore and DevWorkspaceRouting is deployed at runtime while enabling the devworkspaces. DWCO waits for the CRD to appear. Don't deploy the DWCO metrics service anymore because the metrics endpoint is switched off anyway. Trivial - remove the unneeded serialized storage annotations on checluster objects of different versions. The cluster role must grant the operator the perms to update status of the checluster resource. --- deploy.sh | 2 +- deploy/cluster_role.yaml | 1 + .../che-operator.clusterserviceversion.yaml | 7 +- .../che-operator.clusterserviceversion.yaml | 7 +- local-debug.sh | 1 + pkg/apis/org/conversion.go | 6 ++ pkg/apis/org/conversion_test.go | 16 +++ pkg/deploy/dev-workspace/dev_workspace.go | 99 +------------------ .../dev-workspace/dev_workspace_test.go | 42 -------- 9 files changed, 38 insertions(+), 143 deletions(-) diff --git a/deploy.sh b/deploy.sh index df573c8f1..c0e4c1c58 100755 --- a/deploy.sh +++ b/deploy.sh @@ -37,7 +37,7 @@ oc apply -f ${BASE_DIR}/deploy/crds/org.eclipse.che_chebackupserverconfiguration oc apply -f ${BASE_DIR}/deploy/crds/org.eclipse.che_checlusterbackups_crd.yaml -n $NAMESPACE oc apply -f ${BASE_DIR}/deploy/crds/org.eclipse.che_checlusterrestores_crd.yaml -n $NAMESPACE # sometimes the operator cannot get CRD right away -sleep 2 +sleep 5 cp -f ${BASE_DIR}/deploy/operator.yaml /tmp/operator.yaml yq -riyY "( .spec.template.spec.containers[] | select(.name == \"che-operator\") | .image ) = \"${CHE_OPERATOR_IMAGE}\"" /tmp/operator.yaml diff --git a/deploy/cluster_role.yaml b/deploy/cluster_role.yaml index 4dbe8e814..b688df8dd 100644 --- a/deploy/cluster_role.yaml +++ b/deploy/cluster_role.yaml @@ -132,6 +132,7 @@ rules: - org.eclipse.che resources: - checlusters + - checlusters/status - checlusters/finalizers verbs: - '*' diff --git a/deploy/olm-catalog/nightly/eclipse-che-preview-kubernetes/manifests/che-operator.clusterserviceversion.yaml b/deploy/olm-catalog/nightly/eclipse-che-preview-kubernetes/manifests/che-operator.clusterserviceversion.yaml index 42822d227..308afef7c 100644 --- a/deploy/olm-catalog/nightly/eclipse-che-preview-kubernetes/manifests/che-operator.clusterserviceversion.yaml +++ b/deploy/olm-catalog/nightly/eclipse-che-preview-kubernetes/manifests/che-operator.clusterserviceversion.yaml @@ -86,13 +86,13 @@ metadata: categories: Developer Tools certified: "false" containerImage: quay.io/eclipse/che-operator:next - createdAt: "2021-06-30T12:42:36Z" + createdAt: "2021-07-01T21:02:20Z" description: A Kube-native development solution that delivers portable and collaborative developer workspaces. operatorframework.io/suggested-namespace: eclipse-che repository: https://github.com/eclipse-che/che-operator support: Eclipse Foundation - name: eclipse-che-preview-kubernetes.v7.33.0-244.nightly + name: eclipse-che-preview-kubernetes.v7.33.0-246.nightly namespace: placeholder spec: apiservicedefinitions: {} @@ -318,6 +318,7 @@ spec: - org.eclipse.che resources: - checlusters + - checlusters/status - checlusters/finalizers verbs: - '*' @@ -1252,4 +1253,4 @@ spec: maturity: stable provider: name: Eclipse Foundation - version: 7.33.0-244.nightly + version: 7.33.0-246.nightly diff --git a/deploy/olm-catalog/nightly/eclipse-che-preview-openshift/manifests/che-operator.clusterserviceversion.yaml b/deploy/olm-catalog/nightly/eclipse-che-preview-openshift/manifests/che-operator.clusterserviceversion.yaml index 892262e1a..3657775c6 100644 --- a/deploy/olm-catalog/nightly/eclipse-che-preview-openshift/manifests/che-operator.clusterserviceversion.yaml +++ b/deploy/olm-catalog/nightly/eclipse-che-preview-openshift/manifests/che-operator.clusterserviceversion.yaml @@ -77,13 +77,13 @@ metadata: categories: Developer Tools, OpenShift Optional certified: "false" containerImage: quay.io/eclipse/che-operator:next - createdAt: "2021-06-30T12:42:43Z" + createdAt: "2021-07-01T21:02:26Z" description: A Kube-native development solution that delivers portable and collaborative developer workspaces in OpenShift. operatorframework.io/suggested-namespace: eclipse-che repository: https://github.com/eclipse-che/che-operator support: Eclipse Foundation - name: eclipse-che-preview-openshift.v7.33.0-244.nightly + name: eclipse-che-preview-openshift.v7.33.0-246.nightly namespace: placeholder spec: apiservicedefinitions: {} @@ -365,6 +365,7 @@ spec: - org.eclipse.che resources: - checlusters + - checlusters/status - checlusters/finalizers verbs: - '*' @@ -1329,4 +1330,4 @@ spec: maturity: stable provider: name: Eclipse Foundation - version: 7.33.0-244.nightly + version: 7.33.0-246.nightly diff --git a/local-debug.sh b/local-debug.sh index 3d339cb01..a6df26936 100755 --- a/local-debug.sh +++ b/local-debug.sh @@ -18,6 +18,7 @@ command -v operator-sdk >/dev/null 2>&1 || { echo "operator-sdk is not installed ECLIPSE_CHE_NAMESPACE="eclipse-che" ECLIPSE_CHE_CR="./deploy/crds/org_v1_che_cr.yaml" ECLIPSE_CHE_CRD="./deploy/crds/org_v1_che_crd.yaml" +ECLIPSE_CHE_CRD_V1BETA1="./deploy/crds/org_v1_che_crd-v1beta1.yaml" ECLIPSE_CHE_BACKUP_SERVER_CONFIGURATION_CRD="./deploy/crds/org.eclipse.che_chebackupserverconfigurations_crd.yaml" ECLIPSE_CHE_BACKUP_CRD="./deploy/crds/org.eclipse.che_checlusterbackups_crd.yaml" ECLIPSE_CHE_RESTORE_CRD="./deploy/crds/org.eclipse.che_checlusterrestores_crd.yaml" diff --git a/pkg/apis/org/conversion.go b/pkg/apis/org/conversion.go index fa3e9be23..942b292c8 100644 --- a/pkg/apis/org/conversion.go +++ b/pkg/apis/org/conversion.go @@ -60,6 +60,9 @@ func V1ToV2alpha1(v1 *v1.CheCluster, v2 *v2alpha1.CheCluster) error { v1ToV2alpha1_WorkspaceDomainEndpointsTlsSecretName(v1, v2) v1ToV2alpha1_K8sIngressAnnotations(v1, v2) + // we don't need to store the serialized v2 on a v2 object + delete(v2.Annotations, v2alpha1StorageAnnotation) + return nil } @@ -97,6 +100,9 @@ func V2alpha1ToV1(v2 *v2alpha1.CheCluster, v1Obj *v1.CheCluster) error { v2alpha1ToV1_WorkspaceDomainEndpointsTlsSecretName(v1Obj, v2) v2alpha1ToV1_K8sIngressAnnotations(v1Obj, v2) + // we don't need to store the serialized v1 on a v1 object + delete(v1Obj.Annotations, v1StorageAnnotation) + return nil } diff --git a/pkg/apis/org/conversion_test.go b/pkg/apis/org/conversion_test.go index c051f750a..6c9dbff58 100644 --- a/pkg/apis/org/conversion_test.go +++ b/pkg/apis/org/conversion_test.go @@ -666,6 +666,14 @@ func TestFullCircleV1(t *testing.T) { if !reflect.DeepEqual(&v1Obj, &convertedV1) { t.Errorf("V1 not equal to itself after the conversion through v2alpha1: %v", cmp.Diff(&v1Obj, &convertedV1)) } + + if convertedV1.Annotations[v1StorageAnnotation] != "" { + t.Errorf("The v1 storage annotations should not be present on the v1 object") + } + + if convertedV1.Annotations[v2alpha1StorageAnnotation] == "" { + t.Errorf("The v2alpha1 storage annotation should be present on the v1 object") + } } func TestFullCircleV2(t *testing.T) { @@ -711,6 +719,14 @@ func TestFullCircleV2(t *testing.T) { if !reflect.DeepEqual(&v2Obj, &convertedV2) { t.Errorf("V2alpha1 not equal to itself after the conversion through v1: %v", cmp.Diff(&v2Obj, &convertedV2)) } + + if convertedV2.Annotations[v2alpha1StorageAnnotation] != "" { + t.Errorf("The v2alpha1 storage annotations should not be present on the v2alpha1 object") + } + + if convertedV2.Annotations[v1StorageAnnotation] == "" { + t.Errorf("The v1 storage annotation should be present on the v2alpha1 object") + } } func onFakeOpenShift(f func()) { diff --git a/pkg/deploy/dev-workspace/dev_workspace.go b/pkg/deploy/dev-workspace/dev_workspace.go index ba73429f9..f1791414c 100644 --- a/pkg/deploy/dev-workspace/dev_workspace.go +++ b/pkg/deploy/dev-workspace/dev_workspace.go @@ -30,9 +30,7 @@ import ( apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/yaml" @@ -70,19 +68,6 @@ var ( DevWorkspaceConfigMapFile = DevWorkspaceTemplates + "/devworkspace-controller-configmap.ConfigMap.yaml" DevWorkspaceDeploymentFile = DevWorkspaceTemplates + "/devworkspace-controller-manager.Deployment.yaml" - DevWorkspaceCheServiceAccountFile = DevWorkspaceCheTemplates + "/devworkspace-che-serviceaccount.ServiceAccount.yaml" - DevWorkspaceCheRoleFile = DevWorkspaceCheTemplates + "/devworkspace-che-leader-election-role.Role.yaml" - DevWorkspaceCheClusterRoleFile = DevWorkspaceCheTemplates + "/devworkspace-che-role.ClusterRole.yaml" - DevWorkspaceCheProxyClusterRoleFile = DevWorkspaceCheTemplates + "/devworkspace-che-proxy-role.ClusterRole.yaml" - DevWorkspaceCheMetricsReaderClusterRoleFile = DevWorkspaceCheTemplates + "/devworkspace-che-metrics-reader.ClusterRole.yaml" - DevWorkspaceCheRoleBindingFile = DevWorkspaceCheTemplates + "/devworkspace-che-leader-election-rolebinding.RoleBinding.yaml" - DevWorkspaceCheClusterRoleBindingFile = DevWorkspaceCheTemplates + "/devworkspace-che-rolebinding.ClusterRoleBinding.yaml" - DevWorkspaceCheProxyClusterRoleBindingFile = DevWorkspaceCheTemplates + "/devworkspace-che-proxy-rolebinding.ClusterRoleBinding.yaml" - DevWorkspaceCheManagersCRDFile = DevWorkspaceCheTemplates + "/chemanagers.che.eclipse.org.CustomResourceDefinition.yaml" - DevWorkspaceCheConfigMapFile = DevWorkspaceCheTemplates + "/devworkspace-che-configmap.ConfigMap.yaml" - DevWorkspaceCheDeploymentFile = DevWorkspaceCheTemplates + "/devworkspace-che-manager.Deployment.yaml" - DevWorkspaceCheMetricsServiceFile = DevWorkspaceCheTemplates + "/devworkspace-che-controller-manager-metrics-service.Service.yaml" - WebTerminalOperatorSubscriptionName = "web-terminal" WebTerminalOperatorNamespace = "openshift-operators" ) @@ -112,12 +97,6 @@ var ( syncDwConfigMap, syncDwDeployment, } - - syncDwCheItems = []func(*deploy.DeployContext) (bool, error){ - syncDwCheCRD, - syncDwCheCR, - syncDwCheMetricsService, - } ) func ReconcileDevWorkspace(deployContext *deploy.DeployContext) (bool, error) { @@ -131,6 +110,11 @@ func ReconcileDevWorkspace(deployContext *deploy.DeployContext) (bool, error) { return true, nil } + if !util.IsOpenShift && util.GetServerExposureStrategy(deployContext.CheCluster) == "single-host" { + logrus.Warn(`DevWorkspace Che operator can't be enabled in 'single-host' mode on a Kubernetes cluster. See https://github.com/eclipse/che/issues/19714 for more details. To enable DevWorkspace Che operator set 'spec.server.serverExposureStrategy' to 'multi-host'.`) + return true, nil + } + // check if DW exists on the cluster devWorkspaceWebhookExists, err := deploy.Get( deployContext, @@ -157,20 +141,6 @@ func ReconcileDevWorkspace(deployContext *deploy.DeployContext) (bool, error) { } } - if !util.IsOpenShift && util.GetServerExposureStrategy(deployContext.CheCluster) == "single-host" { - logrus.Warn(`DevWorkspace Che operator can't be enabled in 'single-host' mode on a Kubernetes cluster. See https://github.com/eclipse/che/issues/19714 for more details. To enable DevWorkspace Che operator set 'spec.server.serverExposureStrategy' to 'multi-host'.`) - return true, nil - } - - for _, syncItem := range syncDwCheItems { - done, err := syncItem(deployContext) - if !util.IsTestMode() { - if !done { - return false, err - } - } - } - return true, nil } @@ -292,65 +262,6 @@ func syncDwDeployment(deployContext *deploy.DeployContext) (bool, error) { return syncObject(deployContext, obj2sync, DevWorkspaceNamespace) } -func syncDwCheCRD(deployContext *deploy.DeployContext) (bool, error) { - return readAndSyncObject(deployContext, DevWorkspaceCheManagersCRDFile, &apiextensionsv1.CustomResourceDefinition{}, "") -} - -func syncDwCheMetricsService(deployContext *deploy.DeployContext) (bool, error) { - return readAndSyncObject(deployContext, DevWorkspaceCheMetricsServiceFile, &corev1.Service{}, deployContext.CheCluster.Namespace) -} - -func syncDwCheCR(deployContext *deploy.DeployContext) (bool, error) { - // We want to create a default CheManager instance to be able to configure the che-specific - // parts of the installation, but at the same time we don't want to add a dependency on - // devworkspace-che-operator. Note that this way of initializing will probably see changes - // once we figure out https://github.com/eclipse/che/issues/19220 - - // Wait until CRD for CheManager is created - if !util.HasK8SResourceObject(deployContext.ClusterAPI.DiscoveryClient, CheManagerResourcename) { - return false, nil - } - - obj := &unstructured.Unstructured{} - obj.SetGroupVersionKind(schema.GroupVersionKind{Group: "che.eclipse.org", Version: "v1alpha1", Kind: "CheManager"}) - err := deployContext.ClusterAPI.Client.Get(context.TODO(), client.ObjectKey{Name: "devworkspace-che", Namespace: deployContext.CheCluster.Namespace}, obj) - if err != nil { - if apierrors.IsNotFound(err) { - obj = nil - } else { - return false, err - } - } - - if obj == nil { - obj := &unstructured.Unstructured{} - if !util.IsOpenShift { - obj.SetUnstructuredContent(map[string]interface{}{ - "spec": map[string]interface{}{ - "gatewayHost": deployContext.CheCluster.Spec.K8s.IngressDomain, - }, - }) - } - obj.SetGroupVersionKind(schema.GroupVersionKind{ - Group: "che.eclipse.org", - Version: "v1alpha1", - Kind: "CheManager", - }) - obj.SetName("devworkspace-che") - obj.SetNamespace(deployContext.CheCluster.Namespace) - - err = deployContext.ClusterAPI.Client.Create(context.TODO(), obj) - if err != nil { - if apierrors.IsAlreadyExists(err) { - return false, nil - } - return false, err - } - } - - return true, nil -} - func readAndSyncObject(deployContext *deploy.DeployContext, yamlFile string, obj interface{}, namespace string) (bool, error) { obj2sync, err := readK8SObject(yamlFile, obj) if err != nil { diff --git a/pkg/deploy/dev-workspace/dev_workspace_test.go b/pkg/deploy/dev-workspace/dev_workspace_test.go index 46d835609..05bc31eda 100644 --- a/pkg/deploy/dev-workspace/dev_workspace_test.go +++ b/pkg/deploy/dev-workspace/dev_workspace_test.go @@ -12,21 +12,15 @@ package devworkspace import ( - "context" - orgv1 "github.com/eclipse-che/che-operator/pkg/apis/org/v1" "github.com/eclipse-che/che-operator/pkg/deploy" "github.com/eclipse-che/che-operator/pkg/util" operatorsv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1" admissionregistrationv1 "k8s.io/api/admissionregistration/v1" corev1 "k8s.io/api/core/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" fakeDiscovery "k8s.io/client-go/discovery/fake" - "sigs.k8s.io/controller-runtime/pkg/client" "testing" ) @@ -132,42 +126,6 @@ func TestReconcileDevWorkspace(t *testing.T) { if !done { t.Fatalf("Dev Workspace operator has not been provisioned") } - - t.Run("defaultCheManagerDeployed", func(t *testing.T) { - obj := &unstructured.Unstructured{} - obj.SetGroupVersionKind(schema.GroupVersionKind{Group: "che.eclipse.org", Version: "v1alpha1", Kind: "CheManager"}) - err := deployContext.ClusterAPI.Client.Get(context.TODO(), client.ObjectKey{Name: "devworkspace-che", Namespace: deployContext.CheCluster.Namespace}, obj) - - if testCase.IsOpenShift { - if err != nil { - t.Fatalf("Should have found a CheManager with default config but got an error: %s", err) - } - - if obj.GetName() != "devworkspace-che" { - t.Fatalf("Should have found a CheManager with default config but found: %s", obj.GetName()) - } - } else { - if testCase.cheCluster.Spec.Server.ServerExposureStrategy == "single-host" { - if err == nil || !apierrors.IsNotFound(err) { - t.Fatalf("Should not have found a CheManager") - } - } else { - if err != nil { - t.Fatalf("Should have found a CheManager with default config but got an error: %s", err) - } - - if obj.GetName() != "devworkspace-che" { - t.Fatalf("Should have found a CheManager with default config but found: %s", obj.GetName()) - } - - spec := obj.Object["spec"].(map[string]interface{}) - gatewayHost := spec["gatewayHost"].(string) - if gatewayHost != deployContext.CheCluster.Spec.K8s.IngressDomain { - t.Fatalf("gatewayHost wasn't set correctly, expected: %s, actual: %s", deployContext.CheCluster.Spec.K8s.IngressDomain, gatewayHost) - } - } - } - }) }) } }