From ca977800c9a65109706a828e6edbcabbbc60d6d0 Mon Sep 17 00:00:00 2001 From: Ben Oukhanov Date: Thu, 31 Aug 2023 18:14:51 +0300 Subject: [PATCH] refactor(CNV-31248): remove namespace annotation Remove vm-console-proxy-namespace annotation that is no longer relevant since a new released VM console proxy is no longer require namespace to operate, by default it was kubevirt as it required an access to virt-handler (and also virt-handler certificates). Jira-Url: https://issues.redhat.com/browse/CNV-31248 Signed-off-by: Ben Oukhanov --- automation/e2e-upgrade-functests/run.sh | 1 - config/samples/ssp_v1beta2_ssp.yaml | 2 - .../ssp-operator.clusterserviceversion.yaml | 15 +- docs/configuration.md | 4 - .../operands/vm-console-proxy/reconcile.go | 42 +++-- .../vm-console-proxy/reconcile_test.go | 145 +----------------- tests/env/env.go | 27 ++-- tests/tests_suite_test.go | 25 --- tests/vm_console_proxy_test.go | 21 +-- 9 files changed, 47 insertions(+), 235 deletions(-) diff --git a/automation/e2e-upgrade-functests/run.sh b/automation/e2e-upgrade-functests/run.sh index b6bcad6c6..7a40d18f4 100755 --- a/automation/e2e-upgrade-functests/run.sh +++ b/automation/e2e-upgrade-functests/run.sh @@ -70,6 +70,5 @@ export SKIP_CLEANUP_AFTER_TESTS="true" export TEST_EXISTING_CR_NAME="${SSP_NAME}" export TEST_EXISTING_CR_NAMESPACE="${SSP_NAMESPACE}" export IS_UPGRADE_LANE="true" -export VM_CONSOLE_PROXY_NAMESPACE="kubevirt" make deploy functest diff --git a/config/samples/ssp_v1beta2_ssp.yaml b/config/samples/ssp_v1beta2_ssp.yaml index d1e2a594a..7c18bb271 100644 --- a/config/samples/ssp_v1beta2_ssp.yaml +++ b/config/samples/ssp_v1beta2_ssp.yaml @@ -1,8 +1,6 @@ apiVersion: ssp.kubevirt.io/v1beta2 kind: SSP metadata: - annotations: - ssp.kubevirt.io/vm-console-proxy-namespace: "kubevirt" name: ssp-sample namespace: kubevirt spec: diff --git a/data/olm-catalog/ssp-operator.clusterserviceversion.yaml b/data/olm-catalog/ssp-operator.clusterserviceversion.yaml index 0d97a9dd0..533b27720 100644 --- a/data/olm-catalog/ssp-operator.clusterserviceversion.yaml +++ b/data/olm-catalog/ssp-operator.clusterserviceversion.yaml @@ -9,9 +9,6 @@ metadata: "apiVersion": "ssp.kubevirt.io/v1beta2", "kind": "SSP", "metadata": { - "annotations": { - "ssp.kubevirt.io/vm-console-proxy-namespace": "kubevirt" - }, "name": "ssp-sample", "namespace": "kubevirt" }, @@ -259,19 +256,19 @@ spec: - apiGroups: - "" resources: - - pods + - persistentvolumes verbs: - get - list - watch - apiGroups: - - "" + - "" resources: - - persistentvolumes + - pods verbs: - - get - - list - - watch + - get + - list + - watch - apiGroups: - "" resources: diff --git a/docs/configuration.md b/docs/configuration.md index 603e731d8..50dc4a174 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -21,8 +21,6 @@ To activate the operator, create the SSP Custom Resource (CR): apiVersion: ssp.kubevirt.io/v1beta2 kind: SSP metadata: - annotations: - ssp.kubevirt.io/vm-console-proxy-namespace: "kubevirt" name: ssp-sample namespace: kubevirt spec: @@ -49,8 +47,6 @@ This annotation is used by VM console proxy operand. apiVersion: ssp.kubevirt.io/v1beta1 kind: SSP metadata: - annotations: - ssp.kubevirt.io/vm-console-proxy-namespace: "kubevirt" # If not set, then default namespace is "kubevirt" name: ssp-sample namespace: kubevirt spec: {} diff --git a/internal/operands/vm-console-proxy/reconcile.go b/internal/operands/vm-console-proxy/reconcile.go index 6e380ac1c..8d89304fd 100644 --- a/internal/operands/vm-console-proxy/reconcile.go +++ b/internal/operands/vm-console-proxy/reconcile.go @@ -18,8 +18,6 @@ import ( ) const ( - VmConsoleProxyNamespaceAnnotation = "ssp.kubevirt.io/vm-console-proxy-namespace" - operandName = "vm-console-proxy" operandComponent = "vm-console-proxy" @@ -55,11 +53,16 @@ func WatchClusterTypes() []operands.WatchType { return []operands.WatchType{ {Object: &rbac.ClusterRole{}}, {Object: &rbac.ClusterRoleBinding{}}, + {Object: &apiregv1.APIService{}}, + } +} + +func WatchTypes() []operands.WatchType { + return []operands.WatchType{ {Object: &core.ServiceAccount{}}, {Object: &core.Service{}}, {Object: &apps.Deployment{}, WatchFullObject: true}, {Object: &core.ConfigMap{}}, - {Object: &apiregv1.APIService{}}, {Object: &routev1.Route{}}, } } @@ -95,7 +98,7 @@ func (v *vmConsoleProxy) Name() string { } func (v *vmConsoleProxy) WatchTypes() []operands.WatchType { - return nil + return WatchTypes() } func (v *vmConsoleProxy) WatchClusterTypes() []operands.WatchType { @@ -229,9 +232,9 @@ func (v *vmConsoleProxy) deleteRoute(request *common.Request) ([]common.CleanupR func reconcileServiceAccount(serviceAccount core.ServiceAccount) common.ReconcileFunc { return func(request *common.Request) (common.ReconcileResult, error) { - serviceAccount.Namespace = getVmConsoleProxyNamespace(request) + serviceAccount.Namespace = request.Instance.Namespace return common.CreateOrUpdate(request). - ClusterResource(&serviceAccount). + NamespacedResource(&serviceAccount). WithAppLabels(operandName, operandComponent). Reconcile() } @@ -248,6 +251,7 @@ func reconcileClusterRole(clusterRole rbac.ClusterRole) common.ReconcileFunc { func reconcileClusterRoleBinding(clusterRoleBinding rbac.ClusterRoleBinding) common.ReconcileFunc { return func(request *common.Request) (common.ReconcileResult, error) { + clusterRoleBinding.Subjects[0].Namespace = request.Instance.Namespace return common.CreateOrUpdate(request). ClusterResource(&clusterRoleBinding). WithAppLabels(operandName, operandComponent). @@ -257,6 +261,7 @@ func reconcileClusterRoleBinding(clusterRoleBinding rbac.ClusterRoleBinding) com func reconcileRoleBinding(roleBinding *rbac.RoleBinding) common.ReconcileFunc { return func(request *common.Request) (common.ReconcileResult, error) { + roleBinding.Subjects[0].Namespace = request.Instance.Namespace return common.CreateOrUpdate(request). ClusterResource(roleBinding). WithAppLabels(operandName, operandComponent). @@ -266,9 +271,9 @@ func reconcileRoleBinding(roleBinding *rbac.RoleBinding) common.ReconcileFunc { func reconcileConfigMap(configMap core.ConfigMap) common.ReconcileFunc { return func(request *common.Request) (common.ReconcileResult, error) { - configMap.Namespace = getVmConsoleProxyNamespace(request) + configMap.Namespace = request.Instance.Namespace return common.CreateOrUpdate(request). - ClusterResource(&configMap). + NamespacedResource(&configMap). WithAppLabels(operandName, operandComponent). Reconcile() } @@ -276,9 +281,9 @@ func reconcileConfigMap(configMap core.ConfigMap) common.ReconcileFunc { func reconcileService(service core.Service) common.ReconcileFunc { return func(request *common.Request) (common.ReconcileResult, error) { - service.Namespace = getVmConsoleProxyNamespace(request) + service.Namespace = request.Instance.Namespace return common.CreateOrUpdate(request). - ClusterResource(&service). + NamespacedResource(&service). WithAppLabels(operandName, operandComponent). Reconcile() } @@ -286,10 +291,10 @@ func reconcileService(service core.Service) common.ReconcileFunc { func reconcileDeployment(deployment apps.Deployment) common.ReconcileFunc { return func(request *common.Request) (common.ReconcileResult, error) { - deployment.Namespace = getVmConsoleProxyNamespace(request) + deployment.Namespace = request.Instance.Namespace deployment.Spec.Template.Spec.Containers[0].Image = getVmConsoleProxyImage() return common.CreateOrUpdate(request). - ClusterResource(&deployment). + NamespacedResource(&deployment). WithAppLabels(operandName, operandComponent). Reconcile() } @@ -297,7 +302,7 @@ func reconcileDeployment(deployment apps.Deployment) common.ReconcileFunc { func reconcileApiService(apiService *apiregv1.APIService) common.ReconcileFunc { return func(request *common.Request) (common.ReconcileResult, error) { - apiService.Spec.Service.Namespace = getVmConsoleProxyNamespace(request) + apiService.Spec.Service.Namespace = request.Instance.Namespace return common.CreateOrUpdate(request). ClusterResource(apiService). WithAppLabels(operandName, operandComponent). @@ -314,17 +319,6 @@ func reconcileApiService(apiService *apiregv1.APIService) common.ReconcileFunc { } } -func getVmConsoleProxyNamespace(request *common.Request) string { - const defaultNamespace = "kubevirt" - if request.Instance.GetAnnotations() == nil { - return defaultNamespace - } - if namespace, isFound := request.Instance.GetAnnotations()[VmConsoleProxyNamespaceAnnotation]; isFound { - return namespace - } - return defaultNamespace -} - func getVmConsoleProxyImage() string { return common.EnvOrDefault(common.VmConsoleProxyImageKey, defaultVmConsoleProxyImage) } diff --git a/internal/operands/vm-console-proxy/reconcile_test.go b/internal/operands/vm-console-proxy/reconcile_test.go index 47f70c1f2..6eabca598 100644 --- a/internal/operands/vm-console-proxy/reconcile_test.go +++ b/internal/operands/vm-console-proxy/reconcile_test.go @@ -241,7 +241,7 @@ var _ = Describe("VM Console Proxy Operand", func() { } }) - It("should delete resources when enabled annotation is removed", func() { + It("should delete resources when feature gate was disabled", func() { _, err := operand.Reconcile(&request) Expect(err).ToNot(HaveOccurred()) @@ -268,146 +268,6 @@ var _ = Describe("VM Console Proxy Operand", func() { ExpectResourceNotExists(bundle.Deployment, request) ExpectResourceNotExists(bundle.ApiService, request) }) - - Context("with namespace annotation", func() { - const otherNamespace = "some-namespace" - - var ( - serviceAccount *core.ServiceAccount - configMap *core.ConfigMap - service *core.Service - deployment *apps.Deployment - ) - - BeforeEach(func() { - serviceAccount = bundle.ServiceAccount.DeepCopy() - serviceAccount.Namespace = otherNamespace - - configMap = bundle.ConfigMap.DeepCopy() - configMap.Namespace = otherNamespace - - service = bundle.Service.DeepCopy() - service.Namespace = otherNamespace - - deployment = bundle.Deployment.DeepCopy() - deployment.Namespace = otherNamespace - - request.Instance.GetAnnotations()[VmConsoleProxyNamespaceAnnotation] = otherNamespace - }) - - It("should deploy resources in namespace provided by annotation", func() { - _, err := operand.Reconcile(&request) - Expect(err).ToNot(HaveOccurred()) - - ExpectResourceExists(serviceAccount, request) - ExpectResourceExists(bundle.ClusterRole, request) - ExpectResourceExists(bundle.ClusterRoleBinding, request) - ExpectResourceExists(bundle.RoleBinding, request) - ExpectResourceExists(configMap, request) - ExpectResourceExists(service, request) - ExpectResourceExists(deployment, request) - ExpectResourceExists(bundle.ApiService, request) - }) - - It("should cleanup resources from namespace provided by annotation", func() { - _, err := operand.Reconcile(&request) - Expect(err).ToNot(HaveOccurred()) - - ExpectResourceExists(serviceAccount, request) - ExpectResourceExists(bundle.ClusterRole, request) - ExpectResourceExists(bundle.ClusterRoleBinding, request) - ExpectResourceExists(bundle.RoleBinding, request) - ExpectResourceExists(configMap, request) - ExpectResourceExists(service, request) - ExpectResourceExists(deployment, request) - ExpectResourceExists(bundle.ApiService, request) - - _, err = operand.Cleanup(&request) - Expect(err).ToNot(HaveOccurred()) - - ExpectResourceNotExists(serviceAccount, request) - ExpectResourceNotExists(bundle.ClusterRole, request) - ExpectResourceNotExists(bundle.ClusterRoleBinding, request) - ExpectResourceNotExists(bundle.RoleBinding, request) - ExpectResourceNotExists(configMap, request) - ExpectResourceNotExists(service, request) - ExpectResourceNotExists(deployment, request) - ExpectResourceNotExists(bundle.ApiService, request) - }) - - It("should deploy APIService with ServiceReference pointing to the right namespace", func() { - _, err := operand.Reconcile(&request) - Expect(err).ToNot(HaveOccurred()) - - apiService := &apiregv1.APIService{} - key := client.ObjectKeyFromObject(bundle.ApiService) - Expect(request.Client.Get(request.Context, key, apiService)).To(Succeed()) - - Expect(apiService.Spec.Service.Namespace).To(Equal(otherNamespace)) - }) - - It("should remove resources from the namespace", func() { - _, err := operand.Reconcile(&request) - Expect(err).ToNot(HaveOccurred()) - - ExpectResourceExists(serviceAccount, request) - ExpectResourceExists(bundle.ClusterRole, request) - ExpectResourceExists(bundle.ClusterRoleBinding, request) - ExpectResourceExists(bundle.RoleBinding, request) - ExpectResourceExists(configMap, request) - ExpectResourceExists(service, request) - ExpectResourceExists(deployment, request) - ExpectResourceExists(bundle.ApiService, request) - - request.Instance.Spec.FeatureGates.DeployVmConsoleProxy = false - - delete(request.Instance.Annotations, VmConsoleProxyNamespaceAnnotation) - - _, err = operand.Reconcile(&request) - Expect(err).ToNot(HaveOccurred()) - - ExpectResourceNotExists(serviceAccount, request) - ExpectResourceNotExists(bundle.ClusterRole, request) - ExpectResourceNotExists(bundle.ClusterRoleBinding, request) - ExpectResourceNotExists(bundle.RoleBinding, request) - ExpectResourceNotExists(configMap, request) - ExpectResourceNotExists(service, request) - ExpectResourceNotExists(deployment, request) - ExpectResourceNotExists(bundle.ApiService, request) - }) - - DescribeTable("should delete Route leftover from previous version", func(op func() error) { - route := &routev1.Route{ - ObjectMeta: metav1.ObjectMeta{ - Name: routeName, - Namespace: otherNamespace, - Annotations: map[string]string{ - libhandler.TypeAnnotation: "SSP.ssp.kubevirt.io", - libhandler.NamespacedNameAnnotation: namespace + "/" + name, - }, - Labels: map[string]string{ - common.AppKubernetesNameLabel: operandName, - common.AppKubernetesComponentLabel: operandComponent, - common.AppKubernetesManagedByLabel: common.AppKubernetesManagedByValue, - }, - }, - Spec: routev1.RouteSpec{}, - } - - Expect(request.Client.Create(request.Context, route)).To(Succeed()) - Expect(op()).To(Succeed()) - ExpectResourceNotExists(route, request) - }, - Entry("on reconcile", func() error { - _, err := operand.Reconcile(&request) - return err - }), - Entry("on cleanup", func() error { - _, err := operand.Cleanup(&request) - return err - }), - ) - }) }) func TestVmConsoleProxyBundle(t *testing.T) { @@ -436,9 +296,6 @@ func getMockedRequest() common.Request { ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: namespace, - Annotations: map[string]string{ - VmConsoleProxyNamespaceAnnotation: namespace, - }, }, Spec: ssp.SSPSpec{ FeatureGates: &ssp.FeatureGates{ diff --git a/tests/env/env.go b/tests/env/env.go index 7513dc3c1..061c6318c 100644 --- a/tests/env/env.go +++ b/tests/env/env.go @@ -9,18 +9,17 @@ import ( ) const ( - envExistingCrName = "TEST_EXISTING_CR_NAME" - envExistingCrNamespace = "TEST_EXISTING_CR_NAMESPACE" - envSkipUpdateSspTests = "SKIP_UPDATE_SSP_TESTS" - envSkipCleanupAfterTests = "SKIP_CLEANUP_AFTER_TESTS" - envTimeout = "TIMEOUT_MINUTES" - envShortTimeout = "SHORT_TIMEOUT_MINUTES" - envTopologyMode = "TOPOLOGY_MODE" - envIsUpgradeLane = "IS_UPGRADE_LANE" - envSspDeploymentName = "SSP_DEPLOYMENT_NAME" - envSspDeploymentNamespace = "SSP_DEPLOYMENT_NAMESPACE" - envSspWebhookServiceName = "SSP_WEBHOOK_SERVICE_NAME" - envVmConsoleProxyNamespace = "VM_CONSOLE_PROXY_NAMESPACE" + envExistingCrName = "TEST_EXISTING_CR_NAME" + envExistingCrNamespace = "TEST_EXISTING_CR_NAMESPACE" + envSkipUpdateSspTests = "SKIP_UPDATE_SSP_TESTS" + envSkipCleanupAfterTests = "SKIP_CLEANUP_AFTER_TESTS" + envTimeout = "TIMEOUT_MINUTES" + envShortTimeout = "SHORT_TIMEOUT_MINUTES" + envTopologyMode = "TOPOLOGY_MODE" + envIsUpgradeLane = "IS_UPGRADE_LANE" + envSspDeploymentName = "SSP_DEPLOYMENT_NAME" + envSspDeploymentNamespace = "SSP_DEPLOYMENT_NAMESPACE" + envSspWebhookServiceName = "SSP_WEBHOOK_SERVICE_NAME" ) const ( @@ -97,10 +96,6 @@ func SspWebhookServiceName() string { return os.Getenv(envSspWebhookServiceName) } -func VmConsoleProxyNamespace() string { - return os.Getenv(envVmConsoleProxyNamespace) -} - func getBoolEnv(envName string) bool { envVal := os.Getenv(envName) if envVal == "" { diff --git a/tests/tests_suite_test.go b/tests/tests_suite_test.go index a65db3688..e5bf79aea 100644 --- a/tests/tests_suite_test.go +++ b/tests/tests_suite_test.go @@ -45,7 +45,6 @@ import ( sspv1beta1 "kubevirt.io/ssp-operator/api/v1beta1" sspv1beta2 "kubevirt.io/ssp-operator/api/v1beta2" "kubevirt.io/ssp-operator/internal/common" - vm_console_proxy "kubevirt.io/ssp-operator/internal/operands/vm-console-proxy" ) var ( @@ -63,7 +62,6 @@ type TestSuiteStrategy interface { GetName() string GetNamespace() string GetTemplatesNamespace() string - GetVmConsoleProxyNamespace() string GetValidatorReplicas() int GetSSPDeploymentName() string GetSSPDeploymentNameSpace() string @@ -115,9 +113,6 @@ func (s *newSspStrategy) Init() { common.AppKubernetesVersionLabel: "v0.0.0-test", common.AppKubernetesComponentLabel: common.AppComponentSchedule.String(), }, - Annotations: map[string]string{ - vm_console_proxy.VmConsoleProxyNamespaceAnnotation: s.GetVmConsoleProxyNamespace(), - }, }, Spec: sspv1beta2.SSPSpec{ TemplateValidator: &sspv1beta2.TemplateValidator{ @@ -182,11 +177,6 @@ func (s *newSspStrategy) GetTemplatesNamespace() string { return commonTemplatesTestNS } -func (s *newSspStrategy) GetVmConsoleProxyNamespace() string { - const vmConsoleProxyNamespace = "kubevirt" - return vmConsoleProxyNamespace -} - func (s *newSspStrategy) GetValidatorReplicas() int { const templateValidatorReplicas = 2 return templateValidatorReplicas @@ -316,21 +306,6 @@ func (s *existingSspStrategy) GetTemplatesNamespace() string { return s.ssp.Spec.CommonTemplates.Namespace } -func (s *existingSspStrategy) GetVmConsoleProxyNamespace() string { - if s.ssp != nil && s.ssp.ObjectMeta.GetAnnotations() != nil { - namespace, isFound := s.ssp.ObjectMeta.GetAnnotations()[vm_console_proxy.VmConsoleProxyNamespaceAnnotation] - if isFound { - return namespace - } - } - - namespace := env.VmConsoleProxyNamespace() - if namespace == "" { - panic("VM_CONSOLE_PROXY_NAMESPACE is not set") - } - return namespace -} - func (s *existingSspStrategy) GetValidatorReplicas() int { if s.ssp == nil || s.ssp.Spec.TemplateValidator == nil || s.ssp.Spec.TemplateValidator.Replicas == nil { panic("Strategy is not initialized") diff --git a/tests/vm_console_proxy_test.go b/tests/vm_console_proxy_test.go index 9a79107f9..1ffadb507 100644 --- a/tests/vm_console_proxy_test.go +++ b/tests/vm_console_proxy_test.go @@ -22,7 +22,6 @@ import ( kubevirtcorev1 "kubevirt.io/api/core/v1" ssp "kubevirt.io/ssp-operator/api/v1beta2" - vm_console_proxy "kubevirt.io/ssp-operator/internal/operands/vm-console-proxy" "kubevirt.io/ssp-operator/tests/env" ) @@ -40,17 +39,12 @@ var _ = Describe("VM Console Proxy Operand", func() { BeforeEach(OncePerOrdered, func() { strategy.SkipSspUpdateTestsIfNeeded() - namespace := strategy.GetVmConsoleProxyNamespace() updateSsp(func(foundSsp *ssp.SSP) { if foundSsp.Spec.FeatureGates == nil { foundSsp.Spec.FeatureGates = &ssp.FeatureGates{} } - if foundSsp.GetAnnotations() == nil { - foundSsp.Annotations = make(map[string]string) - } foundSsp.Spec.FeatureGates.DeployVmConsoleProxy = true - foundSsp.Annotations[vm_console_proxy.VmConsoleProxyNamespaceAnnotation] = namespace }) expectedLabels := expectedLabelsFor("vm-console-proxy", "vm-console-proxy") @@ -92,13 +86,13 @@ var _ = Describe("VM Console Proxy Operand", func() { } serviceAccountResource = testResource{ Name: "vm-console-proxy", - Namespace: strategy.GetVmConsoleProxyNamespace(), + Namespace: strategy.GetNamespace(), Resource: &core.ServiceAccount{}, ExpectedLabels: expectedLabels, } serviceResource = testResource{ Name: "vm-console-proxy", - Namespace: strategy.GetVmConsoleProxyNamespace(), + Namespace: strategy.GetNamespace(), Resource: &core.Service{}, ExpectedLabels: expectedLabels, UpdateFunc: func(service *core.Service) { @@ -111,7 +105,7 @@ var _ = Describe("VM Console Proxy Operand", func() { } deploymentResource = testResource{ Name: "vm-console-proxy", - Namespace: strategy.GetVmConsoleProxyNamespace(), + Namespace: strategy.GetNamespace(), Resource: &apps.Deployment{}, ExpectedLabels: expectedLabels, UpdateFunc: func(deployment *apps.Deployment) { @@ -123,7 +117,7 @@ var _ = Describe("VM Console Proxy Operand", func() { } configMapResource = testResource{ Name: "vm-console-proxy", - Namespace: strategy.GetVmConsoleProxyNamespace(), + Namespace: strategy.GetNamespace(), Resource: &core.ConfigMap{}, ExpectedLabels: expectedLabels, UpdateFunc: func(configMap *core.ConfigMap) { @@ -164,6 +158,13 @@ var _ = Describe("VM Console Proxy Operand", func() { Entry("[test_id:9888] cluster role", &clusterRoleResource), Entry("[test_id:9847] cluster role binding", &clusterRoleBindingResource), Entry("[test_id:TODO] role binding", &roleBindingResource), + ) + + DescribeTable("created resource", func(res *testResource) { + resource := res.NewResource() + err := apiClient.Get(ctx, res.GetKey(), resource) + Expect(err).ToNot(HaveOccurred()) + }, Entry("[test_id:9848] service account", &serviceAccountResource), Entry("[test_id:9849] service", &serviceResource), Entry("[test_id:9850] deployment", &deploymentResource),