From 7921bcceadad23f18f57036982dc01330f15abd0 Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Wed, 28 Feb 2024 09:46:08 +1300 Subject: [PATCH] Revert "Forbid multiple machine networks in single-stack clusters" Allow users to specify multiple machine networks of the same address family. This is a documented and supported feature of OpenShift. This reverts commit 873dd81bb499c9db0e868d542b9236bdd6a94079. --- internal/bminventory/inventory_test.go | 46 +++++++++------------ internal/cluster/validations/validations.go | 5 --- 2 files changed, 19 insertions(+), 32 deletions(-) diff --git a/internal/bminventory/inventory_test.go b/internal/bminventory/inventory_test.go index acfd6a40197..983a8ea4cc7 100644 --- a/internal/bminventory/inventory_test.go +++ b/internal/bminventory/inventory_test.go @@ -4829,9 +4829,9 @@ var _ = Describe("cluster", func() { Context("Networks", func() { var ( - clusterNetworks = common.TestIPv4Networking.ClusterNetworks - serviceNetworks = common.TestIPv4Networking.ServiceNetworks - machineNetworks = common.TestIPv4Networking.MachineNetworks + clusterNetworks = []*models.ClusterNetwork{{Cidr: "1.1.0.0/24", HostPrefix: 24}, {Cidr: "2.2.0.0/24", HostPrefix: 24}} + serviceNetworks = []*models.ServiceNetwork{{Cidr: "3.3.0.0/24"}, {Cidr: "4.4.0.0/24"}} + machineNetworks = []*models.MachineNetwork{{Cidr: "5.5.0.0/24"}, {Cidr: "6.6.0.0/24"}} ) setNetworksClusterID := func(clusterID strfmt.UUID, @@ -5020,22 +5020,16 @@ var _ = Describe("cluster", func() { verifyApiErrorString(reply, http.StatusBadRequest, "Machine network CIDR '': Failed to parse CIDR '': invalid CIDR address: ") }) - It("Multiple machine networks illegal for single-stack", func() { - reply := bm.V2UpdateCluster(ctx, installer.V2UpdateClusterParams{ - ClusterID: clusterID, - ClusterUpdateParams: &models.V2ClusterUpdateParams{ - MachineNetworks: []*models.MachineNetwork{{Cidr: "15.15.0.0/21"}, {Cidr: "16.16.0.0/21"}}, - }, - }) - verifyApiErrorString(reply, http.StatusBadRequest, "Single-stack cluster cannot contain multiple Machine Networks") - }) + It("Override networks - additional subnet", func() { + clusterNetworks = []*models.ClusterNetwork{{Cidr: "11.11.0.0/21", HostPrefix: 24}, {Cidr: "12.12.0.0/21", HostPrefix: 24}} + serviceNetworks = []*models.ServiceNetwork{{Cidr: "13.13.0.0/21"}, {Cidr: "14.14.0.0/21"}} + machineNetworks = []*models.MachineNetwork{{Cidr: "15.15.0.0/21"}, {Cidr: "16.16.0.0/21"}} - It("Override networks - additional cluster subnet", func() { mockSuccess(1) reply := bm.V2UpdateCluster(ctx, installer.V2UpdateClusterParams{ ClusterID: clusterID, ClusterUpdateParams: &models.V2ClusterUpdateParams{ - ClusterNetworks: []*models.ClusterNetwork{{Cidr: "11.11.0.0/21", HostPrefix: 24}, {Cidr: "12.12.0.0/21", HostPrefix: 24}}, + ClusterNetworks: clusterNetworks, ServiceNetworks: serviceNetworks, MachineNetworks: machineNetworks, VipDhcpAllocation: swag.Bool(true), @@ -5044,19 +5038,20 @@ var _ = Describe("cluster", func() { Expect(reply).To(BeAssignableToTypeOf(installer.NewV2UpdateClusterCreated())) actual := reply.(*installer.V2UpdateClusterCreated) - validateNetworkConfiguration(actual.Payload, - &[]*models.ClusterNetwork{{Cidr: "11.11.0.0/21", HostPrefix: 24}, {Cidr: "12.12.0.0/21", HostPrefix: 24}}, - &serviceNetworks, - &machineNetworks) + validateNetworkConfiguration(actual.Payload, &clusterNetworks, &serviceNetworks, &machineNetworks) validateHostsRequestedHostname(actual.Payload) }) - It("Override networks - 2 additional cluster subnets", func() { + It("Override networks - 2 additional subnets", func() { + clusterNetworks = []*models.ClusterNetwork{{Cidr: "10.10.0.0/21", HostPrefix: 24}, {Cidr: "11.11.0.0/21", HostPrefix: 24}, {Cidr: "12.12.0.0/21", HostPrefix: 24}} + serviceNetworks = []*models.ServiceNetwork{{Cidr: "13.13.0.0/21"}, {Cidr: "14.14.0.0/21"}} + machineNetworks = []*models.MachineNetwork{{Cidr: "15.15.0.0/21"}, {Cidr: "16.16.0.0/21"}} + mockSuccess(1) reply := bm.V2UpdateCluster(ctx, installer.V2UpdateClusterParams{ ClusterID: clusterID, ClusterUpdateParams: &models.V2ClusterUpdateParams{ - ClusterNetworks: []*models.ClusterNetwork{{Cidr: "10.10.0.0/21", HostPrefix: 24}, {Cidr: "11.11.0.0/21", HostPrefix: 24}, {Cidr: "12.12.0.0/21", HostPrefix: 24}}, + ClusterNetworks: clusterNetworks, ServiceNetworks: serviceNetworks, MachineNetworks: machineNetworks, VipDhcpAllocation: swag.Bool(true), @@ -5065,10 +5060,7 @@ var _ = Describe("cluster", func() { Expect(reply).To(BeAssignableToTypeOf(installer.NewV2UpdateClusterCreated())) actual := reply.(*installer.V2UpdateClusterCreated) - validateNetworkConfiguration(actual.Payload, - &[]*models.ClusterNetwork{{Cidr: "10.10.0.0/21", HostPrefix: 24}, {Cidr: "11.11.0.0/21", HostPrefix: 24}, {Cidr: "12.12.0.0/21", HostPrefix: 24}}, - &serviceNetworks, - &machineNetworks) + validateNetworkConfiguration(actual.Payload, &clusterNetworks, &serviceNetworks, &machineNetworks) validateHostsRequestedHostname(actual.Payload) }) @@ -15216,9 +15208,9 @@ var _ = Describe("RegisterCluster", func() { Context("Networking", func() { var ( - clusterNetworks = common.TestIPv4Networking.ClusterNetworks - serviceNetworks = common.TestIPv4Networking.ServiceNetworks - machineNetworks = common.TestIPv4Networking.MachineNetworks + clusterNetworks = []*models.ClusterNetwork{{Cidr: "1.1.1.0/24", HostPrefix: 24}, {Cidr: "2.2.2.0/24", HostPrefix: 24}} + serviceNetworks = []*models.ServiceNetwork{{Cidr: "3.3.3.0/24"}, {Cidr: "4.4.4.0/24"}} + machineNetworks = []*models.MachineNetwork{{Cidr: "5.5.5.0/24"}, {Cidr: "6.6.6.0/24"}, {Cidr: "7.7.7.0/24"}} ) registerCluster := func() *models.Cluster { diff --git a/internal/cluster/validations/validations.go b/internal/cluster/validations/validations.go index f3bfb6f97ae..b0f53222f14 100644 --- a/internal/cluster/validations/validations.go +++ b/internal/cluster/validations/validations.go @@ -605,11 +605,6 @@ func ValidateDualStackNetworks(clusterParams interface{}, alreadyDualStack bool) return err } } - } else { - if len(machineNetworks) > 1 { - err := errors.Errorf("Single-stack cluster cannot contain multiple Machine Networks") - return err - } } return nil }