Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(CNV-31248): remove namespace annotation #634

Merged
merged 1 commit into from
Aug 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
codingben marked this conversation as resolved.
Show resolved Hide resolved
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
Loading