Merge pull request openshift#2197 from kyrtapz/roks_net_node_id

OCPBUGS-24690: Disable network-node-identity on ROKS
pperiyasamy · Feb 19, 2024 · c8debd9 · c8debd9
2 parents 7c7c54d + 21a564c
commit c8debd9
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 8 deletions.
diff --git a/pkg/network/multus_admission_controller.go b/pkg/network/multus_admission_controller.go
@@ -50,7 +50,8 @@ func renderMultusAdmissonControllerConfig(manifestDir string, externalControlPla
 	objs := []*uns.Unstructured{}
 	var err error
 
-	replicas := getMultusAdmissionControllerReplicas(bootstrapResult)
+	hsc := hypershift.NewHyperShiftConfig()
+	replicas := getMultusAdmissionControllerReplicas(bootstrapResult, hsc.Enabled)
 	if ignoredNamespaces == "" {
 		ignoredNamespaces, err = getOpenshiftNamespaces(client)
 		if err != nil {
@@ -68,7 +69,6 @@ func renderMultusAdmissonControllerConfig(manifestDir string, externalControlPla
 	data.Data["ExternalControlPlane"] = externalControlPlane
 	data.Data["Replicas"] = replicas
 	// Hypershift
-	hsc := hypershift.NewHyperShiftConfig()
 	data.Data["HyperShiftEnabled"] = hsc.Enabled
 	data.Data["ManagementClusterName"] = names.ManagementClusterName
 	data.Data["AdmissionControllerNamespace"] = "openshift-multus"

diff --git a/pkg/network/node_identity.go b/pkg/network/node_identity.go
@@ -8,6 +8,7 @@ import (
 	"os"
 	"path/filepath"
 
+	configv1 "github.com/openshift/api/config/v1"
 	operv1 "github.com/openshift/api/operator/v1"
 	"github.com/openshift/cluster-network-operator/pkg/bootstrap"
 	cnoclient "github.com/openshift/cluster-network-operator/pkg/client"
@@ -53,6 +54,15 @@ func renderNetworkNodeIdentity(conf *operv1.NetworkSpec, bootstrapResult *bootst
 		klog.Infof("Network node identity is disabled")
 		return nil, nil
 	}
+	if bootstrapResult.Infra.ControlPlaneTopology == configv1.ExternalTopologyMode &&
+		bootstrapResult.Infra.PlatformType == configv1.IBMCloudPlatformType {
+		// In environments with external control plane topology, the API server is deployed out of cluster.
+		// This means that CNO cannot easily predict how to deploy and enforce the node identity webhook.
+		// IBMCloud uses an external control plane topology with Calico as the CNI for both HyperShift based ROKS
+		// deployments and IBM ROKS Toolkit based ROKS deployments.
+		klog.Infof("Network node identity is disabled on %s platorm", configv1.IBMCloudPlatformType)
+		return nil, nil
+	}
 	data := render.MakeRenderData()
 	data.Data["ReleaseVersion"] = os.Getenv("RELEASE_VERSION")
 	data.Data["OVNHybridOverlayEnable"] = false

diff --git a/pkg/network/render.go b/pkg/network/render.go
@@ -743,10 +743,15 @@ func renderAdditionalNetworks(conf *operv1.NetworkSpec, manifestDir string) ([]*
 	return out, nil
 }
 
-func getMultusAdmissionControllerReplicas(bootstrapResult *bootstrap.BootstrapResult) int {
+func getMultusAdmissionControllerReplicas(bootstrapResult *bootstrap.BootstrapResult, hyperShiftEnabled bool) int {
 	replicas := 2
 	if bootstrapResult.Infra.ControlPlaneTopology == configv1.ExternalTopologyMode {
-		if bootstrapResult.Infra.HostedControlPlane.ControllerAvailabilityPolicy == hypershift.SingleReplica {
+		// In HyperShift check HostedControlPlane.ControllerAvailabilityPolicy, otherwise rely on Infra.InfrastructureTopology
+		if hyperShiftEnabled {
+			if bootstrapResult.Infra.HostedControlPlane.ControllerAvailabilityPolicy == hypershift.SingleReplica {
+				replicas = 1
+			}
+		} else if bootstrapResult.Infra.InfrastructureTopology == configv1.SingleReplicaTopologyMode {
 			replicas = 1
 		}
 	} else if bootstrapResult.Infra.ControlPlaneTopology == configv1.SingleReplicaTopologyMode {

diff --git a/pkg/network/render_test.go b/pkg/network/render_test.go
@@ -426,15 +426,16 @@ func TestRenderUnknownNetwork(t *testing.T) {
 
 func Test_getMultusAdmissionControllerReplicas(t *testing.T) {
 	type args struct {
-		bootstrapResult *bootstrap.BootstrapResult
+		bootstrapResult   *bootstrap.BootstrapResult
+		hypershiftEnabled bool
 	}
 	tests := []struct {
 		name string
 		args args
 		want int
 	}{
 		{
-			name: "External control plane, highly available infra",
+			name: "External control plane, HyperShift,  highly available infra",
 			args: args{
 				bootstrapResult: &bootstrap.BootstrapResult{
 					Infra: bootstrap.InfraStatus{
@@ -444,11 +445,12 @@ func Test_getMultusAdmissionControllerReplicas(t *testing.T) {
 						},
 					},
 				},
+				hypershiftEnabled: true,
 			},
 			want: 2,
 		},
 		{
-			name: "External control plane, single-replica infra",
+			name: "External control plane, HyperShift, single-replica infra",
 			args: args{
 				bootstrapResult: &bootstrap.BootstrapResult{
 					Infra: bootstrap.InfraStatus{
@@ -458,6 +460,31 @@ func Test_getMultusAdmissionControllerReplicas(t *testing.T) {
 						},
 					},
 				},
+				hypershiftEnabled: true,
+			},
+			want: 1,
+		},
+		{
+			name: "External control plane, highly available infra",
+			args: args{
+				bootstrapResult: &bootstrap.BootstrapResult{
+					Infra: bootstrap.InfraStatus{
+						ControlPlaneTopology:   configv1.ExternalTopologyMode,
+						InfrastructureTopology: configv1.HighlyAvailableTopologyMode,
+					},
+				},
+			},
+			want: 2,
+		},
+		{
+			name: "External control plane, single-replica infra",
+			args: args{
+				bootstrapResult: &bootstrap.BootstrapResult{
+					Infra: bootstrap.InfraStatus{
+						ControlPlaneTopology:   configv1.ExternalTopologyMode,
+						InfrastructureTopology: configv1.SingleReplicaTopologyMode,
+					},
+				},
 			},
 			want: 1,
 		},
@@ -512,7 +539,7 @@ func Test_getMultusAdmissionControllerReplicas(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if got := getMultusAdmissionControllerReplicas(tt.args.bootstrapResult); got != tt.want {
+			if got := getMultusAdmissionControllerReplicas(tt.args.bootstrapResult, tt.args.hypershiftEnabled); got != tt.want {
 				t.Errorf("getMultusAdmissionControllerReplicas() = %v, want %v", got, tt.want)
 			}
 		})