From d1cf00bd68451cae351c9fb9220596f9ada73878 Mon Sep 17 00:00:00 2001 From: Sandhya Dasu Date: Mon, 22 Feb 2021 15:35:04 -0500 Subject: [PATCH 1/2] Provide a ProvisioningSpec config to enable multi-namespace functionality --- api/v1alpha1/provisioning_types.go | 8 +++ config/crd/bases/metal3.io_provisionings.yaml | 3 + ...al-operator_02_metal3provisioning.crd.yaml | 3 + provisioning/baremetal_config_test.go | 66 +++++++++++++++++++ 4 files changed, 80 insertions(+) diff --git a/api/v1alpha1/provisioning_types.go b/api/v1alpha1/provisioning_types.go index b7ce81e62..d870b5fa8 100644 --- a/api/v1alpha1/provisioning_types.go +++ b/api/v1alpha1/provisioning_types.go @@ -99,6 +99,14 @@ type ProvisioningSpec struct { // accessible from the machine networks. User should provide two IPs on // the external network that would be used for provisioning services. ProvisioningNetwork ProvisioningNetwork `json:"provisioningNetwork,omitempty"` + + // WatchAllNamespaces provides a way to explicitly allow use of this + // Provisioning configuration across all Namespaces. It is an + // optional configuration which defaults to false and in that state + // will be used to provision baremetal hosts in only the + // openshift-machine-api namespace. When set to true, this provisioning + // configuration would be used for baremetal hosts across all namespaces. + WatchAllNamespaces bool `json:"watchAllNamespaces,omitempty"` } // ProvisioningStatus defines the observed state of Provisioning diff --git a/config/crd/bases/metal3.io_provisionings.yaml b/config/crd/bases/metal3.io_provisionings.yaml index 6292625b4..99d201f24 100644 --- a/config/crd/bases/metal3.io_provisionings.yaml +++ b/config/crd/bases/metal3.io_provisionings.yaml @@ -56,6 +56,9 @@ spec: provisioningOSDownloadURL: description: ProvisioningOSDownloadURL is the location from which the OS Image used to boot baremetal host machines can be downloaded by the metal3 cluster. type: string + watchAllNamespaces: + description: WatchAllNamespaces provides a way to explicitly allow use of this Provisioning configuration across all Namespaces. It is an optional configuration which defaults to false and in that state will be used to provision baremetal hosts in only the openshift-machine-api namespace. When set to true, this provisioning configuration would be used for baremetal hosts across all namespaces. + type: boolean type: object status: description: ProvisioningStatus defines the observed state of Provisioning diff --git a/manifests/0000_31_cluster-baremetal-operator_02_metal3provisioning.crd.yaml b/manifests/0000_31_cluster-baremetal-operator_02_metal3provisioning.crd.yaml index 71b5d61ce..d706e330a 100644 --- a/manifests/0000_31_cluster-baremetal-operator_02_metal3provisioning.crd.yaml +++ b/manifests/0000_31_cluster-baremetal-operator_02_metal3provisioning.crd.yaml @@ -56,6 +56,9 @@ spec: provisioningOSDownloadURL: description: ProvisioningOSDownloadURL is the location from which the OS Image used to boot baremetal host machines can be downloaded by the metal3 cluster. type: string + watchAllNamespaces: + description: WatchAllNamespaces provides a way to explicitly allow use of this Provisioning configuration across all Namespaces. It is an optional configuration which defaults to false and in that state will be used to provision baremetal hosts in only the openshift-machine-api namespace. When set to true, this provisioning configuration would be used for baremetal hosts across all namespaces. + type: boolean type: object status: description: ProvisioningStatus defines the observed state of Provisioning diff --git a/provisioning/baremetal_config_test.go b/provisioning/baremetal_config_test.go index 1d8712866..0c9e6727a 100644 --- a/provisioning/baremetal_config_test.go +++ b/provisioning/baremetal_config_test.go @@ -17,6 +17,7 @@ package provisioning import ( "fmt" + "strconv" "strings" "testing" @@ -408,3 +409,68 @@ func (pb *provisioningBuilder) ProvisioningOSDownloadURL(value string) *provisio pb.ProvisioningSpec.ProvisioningOSDownloadURL = value return pb } + +func enableMultiNamespace() *provisioningBuilder { + return &provisioningBuilder{ + metal3iov1alpha1.ProvisioningSpec{ + ProvisioningInterface: "", + ProvisioningIP: "172.30.20.3", + ProvisioningNetworkCIDR: "172.30.20.0/24", + ProvisioningOSDownloadURL: "http://172.22.0.1/images/rhcos-44.81.202001171431.0-openstack.x86_64.qcow2.gz?sha256=e98f83a2b9d4043719664a2be75fe8134dc6ca1fdbde807996622f8cc7ecd234", + ProvisioningNetwork: "Disabled", + EnableMultiNamespaces: true, + }, + } +} + +func disableMultiNamespace() *provisioningBuilder { + return &provisioningBuilder{ + metal3iov1alpha1.ProvisioningSpec{ + ProvisioningInterface: "", + ProvisioningIP: "172.30.20.3", + ProvisioningNetworkCIDR: "172.30.20.0/24", + ProvisioningOSDownloadURL: "http://172.22.0.1/images/rhcos-44.81.202001171431.0-openstack.x86_64.qcow2.gz?sha256=e98f83a2b9d4043719664a2be75fe8134dc6ca1fdbde807996622f8cc7ecd234", + ProvisioningNetwork: "Disabled", + EnableMultiNamespaces: false, + }, + } +} + +func (pb *provisioningBuilder) EnableMultiNamespaces(value bool) *provisioningBuilder { + pb.ProvisioningSpec.EnableMultiNamespaces = value + return pb +} + +func TestEnableMultiNamespacesConfig(t *testing.T) { + + tCases := []struct { + name string + spec *metal3iov1alpha1.ProvisioningSpec + expectedValue bool + }{ + { + name: "Default", + spec: managedProvisioning().build(), + expectedValue: false, + }, + { + name: "Single Namespace", + spec: disableMultiNamespace().build(), + expectedValue: false, + }, + { + name: "Multiple Namespaces", + spec: enableMultiNamespace().build(), + expectedValue: true, + }, + } + for _, tc := range tCases { + t.Run(tc.name, func(t *testing.T) { + t.Logf("Testing tc : %s", tc.name) + actualValue := tc.spec.EnableMultiNamespaces + assert.NotNil(t, actualValue) + assert.Equal(t, tc.expectedValue, tc.spec.EnableMultiNamespaces, fmt.Sprintf("EnableMultiNamespaces : Expected : %s Actual : %s", strconv.FormatBool(tc.expectedValue), strconv.FormatBool(tc.spec.EnableMultiNamespaces))) + return + }) + } +} From 4f004f4830ea48fd27e0e917bc7ddd1934cb5520 Mon Sep 17 00:00:00 2001 From: Sandhya Dasu Date: Fri, 12 Feb 2021 16:50:55 -0500 Subject: [PATCH 2/2] Allow BMO to watch all Namespaces When the metal3 deployment is created, the BMO is not provided with the WATCH_NAMESPACE env var which allows it to watch all Namespaces. Also, change the rbacs to reflect the fact that BMH objects now can exist in multiple namespaces and not just the openshift-machine-api namespace. --- config/rbac/role.yaml | 34 +++++++++---------- controllers/provisioning_controller.go | 4 +-- ...31_cluster-baremetal-operator_05_rbac.yaml | 34 +++++++++---------- provisioning/baremetal_config_test.go | 16 ++++----- provisioning/baremetal_pod.go | 24 ++++++++----- 5 files changed, 59 insertions(+), 53 deletions(-) diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 17e5a569e..65f861689 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -91,6 +91,23 @@ rules: - infrastructures/status verbs: - get +- apiGroups: + - metal3.io + resources: + - baremetalhosts + verbs: + - get + - list + - patch + - update + - watch +- apiGroups: + - metal3.io + resources: + - baremetalhosts/finalizers + - baremetalhosts/status + verbs: + - update - apiGroups: - metal3.io resources: @@ -198,23 +215,6 @@ rules: - patch - update - watch -- apiGroups: - - metal3.io - resources: - - baremetalhosts - verbs: - - get - - list - - patch - - update - - watch -- apiGroups: - - metal3.io - resources: - - baremetalhosts/finalizers - - baremetalhosts/status - verbs: - - update - apiGroups: - monitoring.coreos.com resources: diff --git a/controllers/provisioning_controller.go b/controllers/provisioning_controller.go index 2821eca8f..0cb37a305 100644 --- a/controllers/provisioning_controller.go +++ b/controllers/provisioning_controller.go @@ -67,8 +67,6 @@ type ensureFunc func(*provisioning.ProvisioningInfo) (bool, error) // +kubebuilder:rbac:namespace=openshift-machine-api,groups="",resources=configmaps,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:namespace=openshift-machine-api,groups="",resources=secrets,verbs=get;list;watch;create;update;patch;delete -// +kubebuilder:rbac:namespace=openshift-machine-api,groups=metal3.io,resources=baremetalhosts,verbs=get;list;watch;update;patch -// +kubebuilder:rbac:namespace=openshift-machine-api,groups=metal3.io,resources=baremetalhosts/status;baremetalhosts/finalizers,verbs=update // +kubebuilder:rbac:namespace=openshift-machine-api,groups=security.openshift.io,resources=securitycontextconstraints,verbs=use // +kubebuilder:rbac:namespace=openshift-machine-api,groups=apps,resources=deployments,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:namespace=openshift-machine-api,groups=apps,resources=daemonsets,verbs=get;list;watch;create;update;patch;delete @@ -83,6 +81,8 @@ type ensureFunc func(*provisioning.ProvisioningInfo) (bool, error) // +kubebuilder:rbac:groups=metal3.io,resources=provisionings,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=metal3.io,resources=provisionings/status,verbs=get;update;patch // +kubebuilder:rbac:groups=metal3.io,resources=provisionings/finalizers,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=metal3.io,resources=baremetalhosts,verbs=get;list;watch;update;patch +// +kubebuilder:rbac:groups=metal3.io,resources=baremetalhosts/status;baremetalhosts/finalizers,verbs=update // +kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=apps,resources=daemonsets,verbs=get;list;watch;create;update;patch;delete diff --git a/manifests/0000_31_cluster-baremetal-operator_05_rbac.yaml b/manifests/0000_31_cluster-baremetal-operator_05_rbac.yaml index a9efe9079..9a0e5968c 100644 --- a/manifests/0000_31_cluster-baremetal-operator_05_rbac.yaml +++ b/manifests/0000_31_cluster-baremetal-operator_05_rbac.yaml @@ -68,23 +68,6 @@ rules: - patch - update - watch -- apiGroups: - - metal3.io - resources: - - baremetalhosts - verbs: - - get - - list - - patch - - update - - watch -- apiGroups: - - metal3.io - resources: - - baremetalhosts/finalizers - - baremetalhosts/status - verbs: - - update - apiGroups: - monitoring.coreos.com resources: @@ -196,6 +179,23 @@ rules: - infrastructures/status verbs: - get +- apiGroups: + - metal3.io + resources: + - baremetalhosts + verbs: + - get + - list + - patch + - update + - watch +- apiGroups: + - metal3.io + resources: + - baremetalhosts/finalizers + - baremetalhosts/status + verbs: + - update - apiGroups: - metal3.io resources: diff --git a/provisioning/baremetal_config_test.go b/provisioning/baremetal_config_test.go index 0c9e6727a..56a1f0be5 100644 --- a/provisioning/baremetal_config_test.go +++ b/provisioning/baremetal_config_test.go @@ -418,7 +418,7 @@ func enableMultiNamespace() *provisioningBuilder { ProvisioningNetworkCIDR: "172.30.20.0/24", ProvisioningOSDownloadURL: "http://172.22.0.1/images/rhcos-44.81.202001171431.0-openstack.x86_64.qcow2.gz?sha256=e98f83a2b9d4043719664a2be75fe8134dc6ca1fdbde807996622f8cc7ecd234", ProvisioningNetwork: "Disabled", - EnableMultiNamespaces: true, + WatchAllNamespaces: true, }, } } @@ -431,18 +431,17 @@ func disableMultiNamespace() *provisioningBuilder { ProvisioningNetworkCIDR: "172.30.20.0/24", ProvisioningOSDownloadURL: "http://172.22.0.1/images/rhcos-44.81.202001171431.0-openstack.x86_64.qcow2.gz?sha256=e98f83a2b9d4043719664a2be75fe8134dc6ca1fdbde807996622f8cc7ecd234", ProvisioningNetwork: "Disabled", - EnableMultiNamespaces: false, + WatchAllNamespaces: false, }, } } -func (pb *provisioningBuilder) EnableMultiNamespaces(value bool) *provisioningBuilder { - pb.ProvisioningSpec.EnableMultiNamespaces = value +func (pb *provisioningBuilder) WatchAllNamespaces(value bool) *provisioningBuilder { + pb.ProvisioningSpec.WatchAllNamespaces = value return pb } -func TestEnableMultiNamespacesConfig(t *testing.T) { - +func TestWatchAllNamespaces(t *testing.T) { tCases := []struct { name string spec *metal3iov1alpha1.ProvisioningSpec @@ -467,9 +466,8 @@ func TestEnableMultiNamespacesConfig(t *testing.T) { for _, tc := range tCases { t.Run(tc.name, func(t *testing.T) { t.Logf("Testing tc : %s", tc.name) - actualValue := tc.spec.EnableMultiNamespaces - assert.NotNil(t, actualValue) - assert.Equal(t, tc.expectedValue, tc.spec.EnableMultiNamespaces, fmt.Sprintf("EnableMultiNamespaces : Expected : %s Actual : %s", strconv.FormatBool(tc.expectedValue), strconv.FormatBool(tc.spec.EnableMultiNamespaces))) + assert.NotNil(t, tc.spec.WatchAllNamespaces) + assert.Equal(t, tc.expectedValue, tc.spec.WatchAllNamespaces, fmt.Sprintf("WatchAllNamespaces : Expected : %s Actual : %s", strconv.FormatBool(tc.expectedValue), strconv.FormatBool(tc.spec.WatchAllNamespaces))) return }) } diff --git a/provisioning/baremetal_pod.go b/provisioning/baremetal_pod.go index 273abb83c..bd9dd3f3f 100644 --- a/provisioning/baremetal_pod.go +++ b/provisioning/baremetal_pod.go @@ -262,6 +262,21 @@ func newMetal3Containers(images *Images, config *metal3iov1alpha1.ProvisioningSp return containers } +func getWatchNamespace(config *metal3iov1alpha1.ProvisioningSpec) corev1.EnvVar { + if config.WatchAllNamespaces { + return corev1.EnvVar{} + } else { + return corev1.EnvVar{ + Name: "WATCH_NAMESPACE", + ValueFrom: &corev1.EnvVarSource{ + FieldRef: &corev1.ObjectFieldSelector{ + FieldPath: "metadata.namespace", + }, + }, + } + } +} + func createContainerMetal3BaremetalOperator(images *Images, config *metal3iov1alpha1.ProvisioningSpec) corev1.Container { container := corev1.Container{ Name: "metal3-baremetal-operator", @@ -280,14 +295,7 @@ func createContainerMetal3BaremetalOperator(images *Images, config *metal3iov1al inspectorCredentialsMount, }, Env: []corev1.EnvVar{ - { - Name: "WATCH_NAMESPACE", - ValueFrom: &corev1.EnvVarSource{ - FieldRef: &corev1.ObjectFieldSelector{ - FieldPath: "metadata.namespace", - }, - }, - }, + getWatchNamespace(config), { Name: "POD_NAMESPACE", ValueFrom: &corev1.EnvVarSource{