Deprecate NoCloudProvider and add CloudProviderEnabled

adilGhaffarDev · adilGhaffarDev · commit 7f7a168de104 · 2024-11-22T03:10:25.000+02:00
Signed-off-by: Muhammad Adil Ghaffar &lt;muhammad.adil.ghaffar@est.tech&gt;
diff --git a/api/v1beta1/metal3cluster_types.go b/api/v1beta1/metal3cluster_types.go
@@ -37,8 +37,16 @@ type Metal3ClusterSpec struct {
 	// Determines if the cluster is not to be deployed with an external cloud provider.
 	// If set to true, CAPM3 will use node labels to set providerID on the kubernetes nodes.
 	// If set to false, providerID is set on nodes by other entities and CAPM3 uses the value of the providerID on the m3m resource.
+	//
+	// Deprecated: This field is deprecated use cloudProviderEnabled instead
+	//
 	// +optional
 	NoCloudProvider bool `json:"noCloudProvider,omitempty"`
+	// Determines if the cluster is to be deployed with an external cloud provider.
+	// If set to false, CAPM3 will use node labels to set providerID on the kubernetes nodes.
+	// If set to true, providerID is set on nodes by other entities and CAPM3 uses the value of the providerID on the m3m resource.
+	// +optional
+	CloudProviderEnabled bool `json:"cloudProviderEnabled,omitempty"`
 }
 
 // IsValid returns an error if the object is not valid, otherwise nil. The
diff --git a/baremetal/metal3machine_manager.go b/baremetal/metal3machine_manager.go
@@ -1295,7 +1295,7 @@ func (m *MachineManager) SetNodeProviderID(ctx context.Context, providerIDOnM3M
 		m.Log.Info(errMessage)
 		return WithTransientError(errors.New(errMessage), requeueAfter)
 	}
-	if !m.Metal3Cluster.Spec.NoCloudProvider && matchingNodesCount == 0 {
+	if (!m.Metal3Cluster.Spec.NoCloudProvider || m.Metal3Cluster.Spec.CloudProviderEnabled) && matchingNodesCount == 0 {
 		// The node could either be still running cloud-init or
 		// kubernetes has not set the node.spec.ProviderID field yet.
 		errMessage := "Some target nodes do not have spec.providerID field set yet, requeuing"
diff --git a/baremetal/metal3machine_manager_test.go b/baremetal/metal3machine_manager_test.go
@@ -2627,14 +2627,14 @@ var _ = Describe("Metal3Machine manager", func() {
 
 				machineMgr, err := NewMachineManager(fakeClient, newCluster(clusterName),
 					newMetal3Cluster(metal3ClusterName, bmcOwnerRef,
-						&infrav1.Metal3ClusterSpec{NoCloudProvider: true}, nil,
+						&infrav1.Metal3ClusterSpec{NoCloudProvider: true, CloudProviderEnabled: false}, nil,
 					),
 					&clusterv1.Machine{}, &infrav1.Metal3Machine{}, logr.Discard(),
 				)
 				if tc.M3MHasHostAnnotation {
 					machineMgr, err = NewMachineManager(fakeClient, newCluster(clusterName),
 						newMetal3Cluster(metal3ClusterName, bmcOwnerRef,
-							&infrav1.Metal3ClusterSpec{NoCloudProvider: true}, nil,
+							&infrav1.Metal3ClusterSpec{NoCloudProvider: true, CloudProviderEnabled: false}, nil,
 						),
 						&clusterv1.Machine{}, &infrav1.Metal3Machine{
 							ObjectMeta: metav1.ObjectMeta{
@@ -2731,7 +2731,7 @@ var _ = Describe("Metal3Machine manager", func() {
 				M3MHasHostAnnotation: true,
 			}),
 		)
-		DescribeTable("Test SetNodeProviderID with noCloudProvider set to false",
+		DescribeTable("Test SetNodeProviderID with CloudProviderEnabled set to true",
 			func(tc testCaseSetNodePoviderID) {
 				BMHHost := newBareMetalHost(baremetalhostName, nil, bmov1alpha1.StateNone, nil, false, "metadata", false, tc.HostID)
 				fakeClient := fake.NewClientBuilder().WithScheme(s).WithObjects(BMHHost).Build()
@@ -2744,7 +2744,7 @@ var _ = Describe("Metal3Machine manager", func() {
 
 				machineMgr, err := NewMachineManager(fakeClient, newCluster(clusterName),
 					newMetal3Cluster(metal3ClusterName, bmcOwnerRef,
-						&infrav1.Metal3ClusterSpec{NoCloudProvider: false}, nil,
+						&infrav1.Metal3ClusterSpec{NoCloudProvider: false, CloudProviderEnabled: true}, nil,
 					),
 					&clusterv1.Machine{}, &infrav1.Metal3Machine{
 						ObjectMeta: metav1.ObjectMeta{
diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_metal3clusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_metal3clusters.yaml
@@ -68,6 +68,12 @@ spec:
           spec:
             description: Metal3ClusterSpec defines the desired state of Metal3Cluster.
             properties:
+              cloudProviderEnabled:
+                description: |-
+                  Determines if the cluster is to be deployed with an external cloud provider.
+                  If set to false, CAPM3 will use node labels to set providerID on the kubernetes nodes.
+                  If set to true, providerID is set on nodes by other entities and CAPM3 uses the value of the providerID on the m3m resource.
+                type: boolean
               controlPlaneEndpoint:
                 description: ControlPlaneEndpoint represents the endpoint used to
                   communicate with the control plane.
@@ -87,6 +93,8 @@ spec:
                   Determines if the cluster is not to be deployed with an external cloud provider.
                   If set to true, CAPM3 will use node labels to set providerID on the kubernetes nodes.
                   If set to false, providerID is set on nodes by other entities and CAPM3 uses the value of the providerID on the m3m resource.
+
+                  Deprecated: This field is deprecated use cloudProviderEnabled instead
                 type: boolean
             type: object
           status:
diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_metal3clustertemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_metal3clustertemplates.yaml
@@ -51,6 +51,12 @@ spec:
                   spec:
                     description: Metal3ClusterSpec defines the desired state of Metal3Cluster.
                     properties:
+                      cloudProviderEnabled:
+                        description: |-
+                          Determines if the cluster is to be deployed with an external cloud provider.
+                          If set to false, CAPM3 will use node labels to set providerID on the kubernetes nodes.
+                          If set to true, providerID is set on nodes by other entities and CAPM3 uses the value of the providerID on the m3m resource.
+                        type: boolean
                       controlPlaneEndpoint:
                         description: ControlPlaneEndpoint represents the endpoint
                           used to communicate with the control plane.
@@ -72,6 +78,8 @@ spec:
                           Determines if the cluster is not to be deployed with an external cloud provider.
                           If set to true, CAPM3 will use node labels to set providerID on the kubernetes nodes.
                           If set to false, providerID is set on nodes by other entities and CAPM3 uses the value of the providerID on the m3m resource.
+
+                          Deprecated: This field is deprecated use cloudProviderEnabled instead
                         type: boolean
                     type: object
                 required:
diff --git a/controllers/suite_test.go b/controllers/suite_test.go
@@ -169,6 +169,7 @@ func bmcSpec() *infrav1.Metal3ClusterSpec {
 			Port: 6443,
 		},
 		NoCloudProvider: true,
+		CloudProviderEnabled: false,
 	}
 }
 
diff --git a/docs/api.md b/docs/api.md
@@ -80,10 +80,14 @@ cluster on Baremetal. It currently has two specification fields :
 
 - **controlPlaneEndpoint**: contains the target cluster API server address and
   port
-- **noCloudProvider**: (true/false) Whether the cluster will not be deployed
+- **noCloudProvider(Deprecated use CloudProviderEnabled)**: (true/false) Whether the cluster will not be deployed
   with an external cloud provider. If set to true, CAPM3 will patch the target
   cluster node objects to add a providerID. This will allow the CAPI process to
   continue even if the cluster is deployed without cloud provider.
+- **CloudProviderEnabled**: (true/false) Whether the cluster will be deployed
+  with an external cloud provider. If set to false, CAPM3 will patch the target
+  cluster node objects to add a providerID. This will allow the CAPI process to
+  continue even if the cluster is deployed without cloud provider.
 
 Example metal3cluster :
 
@@ -97,7 +101,8 @@ spec:
   controlPlaneEndpoint:
     host: 192.168.111.249
     port: 6443
-  noCloudProvider: true
+  noCloudProvider: true  # This field is deprecated use cloudProviderEnabled instead
+  cloudProviderEnabled: false
 ```
 
 ## KubeadmControlPlane
diff --git a/docs/architecture.md b/docs/architecture.md
@@ -121,7 +121,8 @@ metadata:
   name: test1
 spec:
   apiEndpoint: https://192.168.111.249:6443
-  noCloudProvider: true
+  noCloudProvider: true  # This field is deprecated use cloudProviderEnabled instead
+  cloudProviderEnabled: false
 ```
 
 Metal3Cluster, after reconciliation
@@ -142,7 +143,8 @@ metadata:
   |----------------------------------------------------------------------------|
 spec:
   apiEndpoint: https://192.168.111.249:6443
-  noCloudProvider: true
+  noCloudProvider: true  # This field is deprecated use cloudProviderEnabled instead
+  cloudProviderEnabled: false
 status:
   apiEndpoints:
   - host: 192.168.111.249
diff --git a/examples/cluster/cluster.yaml b/examples/cluster/cluster.yaml
@@ -30,7 +30,8 @@ metadata:
   name: ${CLUSTER_NAME}
   namespace: ${NAMESPACE}
 spec:
+noCloudProvider: true # This field is deprecated use cloudProviderEnabled instead
+cloudProviderEnabled: false
   controlPlaneEndpoint:
     host: ${CLUSTER_APIENDPOINT_HOST}
     port: ${CLUSTER_APIENDPOINT_PORT}
-  noCloudProvider: true
diff --git a/examples/clusterctl-templates/clusterctl-cluster.yaml b/examples/clusterctl-templates/clusterctl-cluster.yaml
@@ -30,7 +30,8 @@ spec:
   controlPlaneEndpoint:
     host: ${CLUSTER_APIENDPOINT_HOST}
     port: ${CLUSTER_APIENDPOINT_PORT}
-  noCloudProvider: true
+  noCloudProvider: true # This field is deprecated use cloudProviderEnabled instead
+  cloudProviderEnabled: false
 ---
 kind: KubeadmControlPlane
 apiVersion: controlplane.cluster.x-k8s.io/v1beta1
diff --git a/examples/templates/clusterclass.yaml b/examples/templates/clusterclass.yaml
@@ -9,7 +9,8 @@ spec:
       controlPlaneEndpoint:
         host: 127.0.0.1
         port: 6443
-      noCloudProvider: true
+        noCloudProvider: true # This field is deprecated use cloudProviderEnabled instead
+        cloudProviderEnabled: false
 ---
 apiVersion: controlplane.cluster.x-k8s.io/v1beta1
 kind: KubeadmControlPlaneTemplate
diff --git a/test/e2e/data/infrastructure-metal3/bases/cluster/cluster-with-kcp.yaml b/test/e2e/data/infrastructure-metal3/bases/cluster/cluster-with-kcp.yaml
@@ -29,7 +29,8 @@ spec:
   controlPlaneEndpoint:
     host: ${CLUSTER_APIENDPOINT_HOST}
     port: ${CLUSTER_APIENDPOINT_PORT}
-  noCloudProvider: true
+    noCloudProvider: true # This field is deprecated use cloudProviderEnabled instead
+    cloudProviderEnabled: false
 ---
 apiVersion: controlplane.cluster.x-k8s.io/${CAPI_VERSION}
 kind: KubeadmControlPlane
diff --git a/test/e2e/data/infrastructure-metal3/bases/clusterclass-cluster/cluster-with-kcp.yaml b/test/e2e/data/infrastructure-metal3/bases/clusterclass-cluster/cluster-with-kcp.yaml
@@ -15,7 +15,8 @@ spec:
       controlPlaneEndpoint:
         host: ${CLUSTER_APIENDPOINT_HOST}
         port: ${CLUSTER_APIENDPOINT_PORT}
-      noCloudProvider: true
+        noCloudProvider: true # This field is deprecated use cloudProviderEnabled instead
+        cloudProviderEnabled: false
 ---
 apiVersion: infrastructure.cluster.x-k8s.io/${CAPM3_VERSION}
 kind: Metal3MachineTemplate

Original file line number	Diff line number	Diff line change
`@@ -1295,7 +1295,7 @@ func (m *MachineManager) SetNodeProviderID(ctx context.Context, providerIDOnM3M`
`1295`	`1295`	`m.Log.Info(errMessage)`
`1296`	`1296`	`return WithTransientError(errors.New(errMessage), requeueAfter)`
`1297`	`1297`	`}`
`1298`		`- if !m.Metal3Cluster.Spec.NoCloudProvider && matchingNodesCount == 0 {`
	`1298`	`+ if (!m.Metal3Cluster.Spec.NoCloudProvider \|\| m.Metal3Cluster.Spec.CloudProviderEnabled) && matchingNodesCount == 0 {`
`1299`	`1299`	`// The node could either be still running cloud-init or`
`1300`	`1300`	`// kubernetes has not set the node.spec.ProviderID field yet.`
`1301`	`1301`	`errMessage := "Some target nodes do not have spec.providerID field set yet, requeuing"`
Original file line number	Diff line number	Diff line change
`@@ -169,6 +169,7 @@ func bmcSpec() *infrav1.Metal3ClusterSpec {`
`169`	`169`	`Port: 6443,`
`170`	`170`	`},`
`171`	`171`	`NoCloudProvider: true,`
	`172`	`+ CloudProviderEnabled: false,`
`172`	`173`	`}`
`173`	`174`	`}`
`174`	`175`