Skip to content

Commit

Permalink
refactor(CNV-31248): remove namespace annotation
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
codingben committed Aug 31, 2023
1 parent 0b32d61 commit ca97780
Show file tree
Hide file tree
Showing 9 changed files with 47 additions and 235 deletions.
1 change: 0 additions & 1 deletion automation/e2e-upgrade-functests/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 0 additions & 2 deletions config/samples/ssp_v1beta2_ssp.yaml
Original file line number Diff line number Diff line change
@@ -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:
Expand Down
15 changes: 6 additions & 9 deletions data/olm-catalog/ssp-operator.clusterserviceversion.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
Expand Down Expand Up @@ -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:
Expand Down
4 changes: 0 additions & 4 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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: {}
Expand Down
42 changes: 18 additions & 24 deletions internal/operands/vm-console-proxy/reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ import (
)

const (
VmConsoleProxyNamespaceAnnotation = "ssp.kubevirt.io/vm-console-proxy-namespace"

operandName = "vm-console-proxy"
operandComponent = "vm-console-proxy"

Expand Down Expand Up @@ -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{}},
}
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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()
}
Expand All @@ -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).
Expand All @@ -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).
Expand All @@ -266,38 +271,38 @@ 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()
}
}

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()
}
}

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()
}
}

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).
Expand All @@ -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)
}
Expand Down
145 changes: 1 addition & 144 deletions internal/operands/vm-console-proxy/reconcile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())

Expand All @@ -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) {
Expand Down Expand Up @@ -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{
Expand Down
Loading

0 comments on commit ca97780

Please sign in to comment.