From ad94f417becf0abfdfe19e54dd2d1562e83f7387 Mon Sep 17 00:00:00 2001 From: Patryk Diak Date: Thu, 11 Jan 2024 14:49:21 +0100 Subject: [PATCH 1/2] Disable network-node-identity on ROKS 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. There is no signifficant value added by the network-node-identity in this scenario. Signed-off-by: Patryk Diak --- pkg/network/node_identity.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/pkg/network/node_identity.go b/pkg/network/node_identity.go index 61576d0efb..4d0a2dde4a 100644 --- 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 From 21a564c3449e2334dff33dcd642628afc81aa243 Mon Sep 17 00:00:00 2001 From: Patryk Diak Date: Wed, 7 Feb 2024 11:09:45 +0100 Subject: [PATCH 2/2] getMultusAdmissionControllerReplicas for non-hypershift externalTopology clusters a5edddf194e70a8ab01a44dbc4e3d8214a773f10 introduced a change that doesn't work on non-hypershift clusters with external controllplane. Re-introduce the original logic for this type of clusters. Signed-off-by: Patryk Diak --- pkg/network/multus_admission_controller.go | 4 +-- pkg/network/render.go | 9 ++++-- pkg/network/render_test.go | 35 +++++++++++++++++++--- 3 files changed, 40 insertions(+), 8 deletions(-) diff --git a/pkg/network/multus_admission_controller.go b/pkg/network/multus_admission_controller.go index 700ab0e23e..9e2a5548a4 100644 --- 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/render.go b/pkg/network/render.go index 810efbf89b..2c61f51ee7 100644 --- a/pkg/network/render.go +++ b/pkg/network/render.go @@ -735,10 +735,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 index 5c0f32c6e0..88c9523993 100644 --- a/pkg/network/render_test.go +++ b/pkg/network/render_test.go @@ -410,7 +410,8 @@ 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 @@ -418,7 +419,7 @@ func Test_getMultusAdmissionControllerReplicas(t *testing.T) { 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{ @@ -428,11 +429,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{ @@ -442,6 +444,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, }, @@ -496,7 +523,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) } })