From 8fe718260d97b682d2e5f2922911c2d70e50da8c Mon Sep 17 00:00:00 2001 From: Dan Manor Date: Wed, 15 Jan 2025 22:19:26 +0200 Subject: [PATCH] MGMT-19590: Enable user managed load balancer with hosts in the same subnet but load balancer outside this subnet on baremetal (#7187) --- internal/bminventory/inventory.go | 202 ++++-- internal/bminventory/inventory_test.go | 663 ++++++++--------- internal/cluster/cluster.go | 7 + internal/cluster/validations/validations.go | 39 +- internal/cluster/validator.go | 63 +- internal/cluster/validator_test.go | 266 ++++++- .../featuresupport/features_networking.go | 14 +- internal/host/validations_test.go | 670 ++++++++++++++++++ internal/host/validator.go | 35 +- internal/network/cidr_validations.go | 7 +- internal/network/cidr_validations_test.go | 20 +- internal/network/machine_network_cidr.go | 117 ++- internal/network/machine_network_cidr_test.go | 548 +++++++++++--- internal/network/utils.go | 9 + 14 files changed, 2104 insertions(+), 556 deletions(-) diff --git a/internal/bminventory/inventory.go b/internal/bminventory/inventory.go index 7a6bd41dfc2..6edf681778a 100644 --- a/internal/bminventory/inventory.go +++ b/internal/bminventory/inventory.go @@ -553,7 +553,7 @@ func (b *bareMetalInventory) RegisterClusterInternal(ctx context.Context, kubeKe b.log.WithError(err).Error("Cannot register cluster. Failed VIP validations") return nil, common.NewApiError(http.StatusBadRequest, err) } - if err = validations.ValidateDualStackNetworks(params.NewClusterParams, false); err != nil { + if err = validations.ValidateDualStackNetworks(params.NewClusterParams, false, false); err != nil { return nil, common.NewApiError(http.StatusBadRequest, err) } if err = validations.ValidatePlatformCapability(params.NewClusterParams.Platform, ctx, b.authzHandler); err != nil { @@ -2014,7 +2014,8 @@ func (b *bareMetalInventory) validateUpdateCluster( return params, common.NewApiError(http.StatusBadRequest, err) } alreadyDualStack := network.CheckIfClusterIsDualStack(cluster) - if err = validations.ValidateDualStackNetworks(params.ClusterUpdateParams, alreadyDualStack); err != nil { + alreadyUserManagedLoadBalancer := network.IsLoadBalancerUserManaged(cluster) + if err = validations.ValidateDualStackNetworks(params.ClusterUpdateParams, alreadyDualStack, alreadyUserManagedLoadBalancer); err != nil { return params, common.NewApiError(http.StatusBadRequest, err) } @@ -2215,17 +2216,18 @@ func (b *bareMetalInventory) updateNonDhcpNetworkParams(cluster *common.Cluster, if params.ClusterUpdateParams.IngressVips != nil { targetConfiguration.IngressVips = params.ClusterUpdateParams.IngressVips } + if params.ClusterUpdateParams.LoadBalancer != nil { + targetConfiguration.LoadBalancer = params.ClusterUpdateParams.LoadBalancer + } if params.ClusterUpdateParams.MachineNetworks != nil && common.IsSliceNonEmpty(params.ClusterUpdateParams.MachineNetworks) && interactivity == Interactive && - !reqDualStack { - err := errors.New("Setting Machine network CIDR is forbidden when cluster is not in vip-dhcp-allocation mode") + !reqDualStack && + !network.IsLoadBalancerUserManaged(&targetConfiguration) { + err := errors.New("Setting the Machine Network CIDR is forbidden when the cluster is neither in VIP DHCP allocation mode nor using a user-managed load balance") log.WithError(err).Warnf("Set Machine Network CIDR") return common.NewApiError(http.StatusBadRequest, err) } - if params.ClusterUpdateParams.LoadBalancer != nil { - targetConfiguration.LoadBalancer = params.ClusterUpdateParams.LoadBalancer - } var err error err = validations.VerifyParsableVIPs(targetConfiguration.APIVips, targetConfiguration.IngressVips) @@ -2242,80 +2244,139 @@ func (b *bareMetalInventory) updateNonDhcpNetworkParams(cluster *common.Cluster, return common.NewApiError(http.StatusBadRequest, err) } if interactivity == Interactive && (params.ClusterUpdateParams.APIVips != nil || params.ClusterUpdateParams.IngressVips != nil) { - var primaryMachineNetworkCidr string - var secondaryMachineNetworkCidr string - // We want to calculate Machine Network based on the API/Ingress VIPs only in case of the // single-stack cluster. Auto calculation is not supported for dual-stack in which we // require that user explicitly provides all the Machine Networks. if reqDualStack { - if params.ClusterUpdateParams.MachineNetworks != nil { - primaryMachineNetworkCidr = string(params.ClusterUpdateParams.MachineNetworks[0].Cidr) - } else { - primaryMachineNetworkCidr = network.GetPrimaryMachineCidrForUserManagedNetwork(cluster, log) - } + return b.updateNonDhcpDualStackNetworkParams( + cluster, + params, + &targetConfiguration, + log, + ) + } - err = network.VerifyMachineNetworksDualStack(targetConfiguration.MachineNetworks, reqDualStack) - if err != nil { - log.WithError(err).Warnf("Verify dual-stack machine networks") - return common.NewApiError(http.StatusBadRequest, err) - } - secondaryMachineNetworkCidr, err = network.GetSecondaryMachineCidr(&targetConfiguration) - if err != nil { - return common.NewApiError(http.StatusBadRequest, err) - } + return b.updateNonDhcpSingleStackNetworkParams( + cluster, + params, + &targetConfiguration, + log, + ) + } - if err = network.VerifyVips( - cluster.Hosts, - primaryMachineNetworkCidr, - network.GetApiVipById(&targetConfiguration, 0), - network.GetIngressVipById(&targetConfiguration, 0), - network.IsLoadBalancerUserManaged(&targetConfiguration), - log, - ); err != nil { - log.WithError(err).Warnf("Verify VIPs") - return common.NewApiError(http.StatusBadRequest, err) - } + return nil +} - if len(targetConfiguration.IngressVips) == 2 && len(targetConfiguration.APIVips) == 2 { // in case there's a second set of VIPs - if err = network.VerifyVips(cluster.Hosts, secondaryMachineNetworkCidr, network.GetApiVipById(&targetConfiguration, 1), network.GetIngressVipById(&targetConfiguration, 1), network.IsLoadBalancerUserManaged(&targetConfiguration), log); err != nil { - log.WithError(err).Warnf("Verify VIPs") - return common.NewApiError(http.StatusBadRequest, err) - } - } +func (b *bareMetalInventory) updateNonDhcpDualStackNetworkParams( + cluster *common.Cluster, + params installer.V2UpdateClusterParams, + targetConfiguration *common.Cluster, + log logrus.FieldLogger, +) error { + var primaryMachineNetworkCidr string + var secondaryMachineNetworkCidr string - } else { - matchRequired := (network.GetApiVipById(&targetConfiguration, 0) != "" || network.GetIngressVipById(&targetConfiguration, 0) != "") - primaryMachineNetworkCidr, err = network.CalculateMachineNetworkCIDR(network.GetApiVipById(&targetConfiguration, 0), network.GetIngressVipById(&targetConfiguration, 0), cluster.Hosts, matchRequired) - if err != nil { - return common.NewApiError(http.StatusBadRequest, errors.Wrap(err, "Calculate machine network CIDR")) - } - if primaryMachineNetworkCidr != "" { - // We set the machine networks in the ClusterUpdateParams, so they will be viewed as part of the request - // to update the cluster - - // Earlier in this function, if reqDualStack was false and the MachineNetworks was non-empty, the function - // returned with an error. Therefore, params.ClusterUpdateParams.MachineNetworks is empty here before - // the assignment below. - params.ClusterUpdateParams.MachineNetworks = []*models.MachineNetwork{{Cidr: models.Subnet(primaryMachineNetworkCidr)}} - } - if err = network.VerifyVips( - cluster.Hosts, - primaryMachineNetworkCidr, - network.GetApiVipById(&targetConfiguration, 0), - network.GetIngressVipById(&targetConfiguration, 0), - network.IsLoadBalancerUserManaged(&targetConfiguration), - log, - ); err != nil { - log.WithError(err).Warnf("Verify VIPs") - return common.NewApiError(http.StatusBadRequest, err) - } + if params.ClusterUpdateParams.MachineNetworks != nil { + primaryMachineNetworkCidr = string(params.ClusterUpdateParams.MachineNetworks[0].Cidr) + } else { + primaryMachineNetworkCidr = network.GetPrimaryMachineCidrForUserManagedNetwork(cluster, log) + } + + err := network.VerifyMachineNetworksDualStack(targetConfiguration.MachineNetworks, true) + if err != nil { + log.WithError(err).Warnf("Verify dual-stack machine networks") + return common.NewApiError(http.StatusBadRequest, err) + } + + secondaryMachineNetworkCidr, err = network.GetSecondaryMachineCidr(targetConfiguration) + if err != nil { + return common.NewApiError(http.StatusBadRequest, err) + } + + if err = network.VerifyVipsForClusterManagedLoadBalancer( + cluster.Hosts, + primaryMachineNetworkCidr, + network.GetApiVipById(targetConfiguration, 0), + network.GetIngressVipById(targetConfiguration, 0), + log, + ); err != nil { + log.WithError(err).Warnf("Verify VIPs") + return common.NewApiError(http.StatusBadRequest, err) + } + + if len(targetConfiguration.IngressVips) == 2 && len(targetConfiguration.APIVips) == 2 { // in case there's a second set of VIPs + if err = network.VerifyVipsForClusterManagedLoadBalancer( + cluster.Hosts, + secondaryMachineNetworkCidr, + network.GetApiVipById(targetConfiguration, 1), + network.GetIngressVipById(targetConfiguration, 1), + log, + ); err != nil { + log.WithError(err).Warnf("Verify VIPs") + return common.NewApiError(http.StatusBadRequest, err) } } return nil } +func (b *bareMetalInventory) updateNonDhcpSingleStackNetworkParams( + cluster *common.Cluster, + params installer.V2UpdateClusterParams, + targetConfiguration *common.Cluster, + log logrus.FieldLogger, +) error { + if !network.IsLoadBalancerUserManaged(targetConfiguration) { + matchRequired := (network.GetApiVipById(targetConfiguration, 0) != "" || network.GetIngressVipById(targetConfiguration, 0) != "") + primaryMachineNetworkCidr, err := network.CalculateMachineNetworkCIDR( + network.GetApiVipById(targetConfiguration, 0), + network.GetIngressVipById(targetConfiguration, 0), + cluster.Hosts, + matchRequired, + ) + + if err != nil { + return common.NewApiError(http.StatusBadRequest, errors.Wrap(err, "Calculate machine network CIDR")) + } + + if primaryMachineNetworkCidr != "" { + // We set the machine networks in the ClusterUpdateParams, so they will be viewed as part of the request + // to update the cluster + + // Earlier in this function, if reqDualStack was false and the MachineNetworks was non-empty, the function + // returned with an error. Therefore, params.ClusterUpdateParams.MachineNetworks is empty here before + // the assignment below. + params.ClusterUpdateParams.MachineNetworks = []*models.MachineNetwork{{Cidr: models.Subnet(primaryMachineNetworkCidr)}} + } + + if err = network.VerifyVipsForClusterManagedLoadBalancer( + cluster.Hosts, + primaryMachineNetworkCidr, + network.GetApiVipById(targetConfiguration, 0), + network.GetIngressVipById(targetConfiguration, 0), + log, + ); err != nil { + log.WithError(err).Warnf("Verify VIPs") + return common.NewApiError(http.StatusBadRequest, err) + } + + return nil + } + + if err := network.VerifyVipsForUserManangedLoadBalancer( + cluster.Hosts, + targetConfiguration.MachineNetworks, + network.GetApiVipById(targetConfiguration, 0), + network.GetIngressVipById(targetConfiguration, 0), + log, + ); err != nil { + log.WithError(err).Warnf("Verify VIPs") + return common.NewApiError(http.StatusBadRequest, err) + } + + return nil +} + func (b *bareMetalInventory) updateDhcpNetworkParams(db *gorm.DB, id *strfmt.UUID, params installer.V2UpdateClusterParams, primaryMachineCIDR string) error { if err := validations.ValidateVIPsWereNotSetDhcpMode(params.ClusterUpdateParams.APIVips, params.ClusterUpdateParams.IngressVips); err != nil { return common.NewApiError(http.StatusBadRequest, err) @@ -2546,7 +2607,11 @@ func (b *bareMetalInventory) updateNetworks(db *gorm.DB, params installer.V2Upda if params.ClusterUpdateParams.MachineNetworks != nil && !network.AreMachineNetworksIdentical(params.ClusterUpdateParams.MachineNetworks, cluster.MachineNetworks) { for _, machineNetwork := range params.ClusterUpdateParams.MachineNetworks { - if err = network.VerifyMachineCIDR(string(machineNetwork.Cidr), common.IsSingleNodeCluster(cluster)); err != nil { + if err = network.VerifyMachineCIDR( + string(machineNetwork.Cidr), + common.IsSingleNodeCluster(cluster), + network.IsLoadBalancerUserManaged(cluster), + ); err != nil { return common.NewApiError(http.StatusBadRequest, errors.Wrapf(err, "Machine network CIDR '%s'", string(machineNetwork.Cidr))) } } @@ -2730,6 +2795,7 @@ func (b *bareMetalInventory) updateNetworkParams(params installer.V2UpdateCluste (cluster.LoadBalancer == nil || (cluster.LoadBalancer != nil && params.ClusterUpdateParams.LoadBalancer.Type != cluster.LoadBalancer.Type)) { updates["load_balancer_type"] = params.ClusterUpdateParams.LoadBalancer.Type + cluster.LoadBalancer = &models.LoadBalancer{Type: params.ClusterUpdateParams.LoadBalancer.Type} } if userManagedNetworking { diff --git a/internal/bminventory/inventory_test.go b/internal/bminventory/inventory_test.go index 799d71b4208..63deb7b081c 100644 --- a/internal/bminventory/inventory_test.go +++ b/internal/bminventory/inventory_test.go @@ -4340,7 +4340,7 @@ var _ = Describe("cluster", func() { }, }) verifyApiErrorString(reply, http.StatusBadRequest, - "Setting Machine network CIDR is forbidden when cluster is not in vip-dhcp-allocation mode") + "Setting the Machine Network CIDR is forbidden when the cluster is neither in VIP DHCP allocation mode nor using a user-managed load balance") }) It("Machine network CIDR in non dhcp non interactive", func() { mockClusterUpdatability(2) @@ -8117,85 +8117,21 @@ var _ = Describe("V2UpdateCluster", func() { }) Context("load_balancer", func() { - BeforeEach(func() { - mockSetConnectivityMajorityGroupsForCluster(mockClusterApi) - mockDetectAndStoreCollidingIPsForCluster(mockClusterApi, 1) - mockClusterApi.EXPECT().RefreshStatus(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil).Times(1) - mockHostApi.EXPECT().RefreshInventory(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).Times(1) - mockHostApi.EXPECT().RefreshStatus(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).Times(1) - mockHostApi.EXPECT().GetStagesByRole(gomock.Any(), gomock.Any()).Return(nil).Times(1) - }) - - Context("should succeed", func() { - It("when updating to user managed load balancer with common VIP", func() { - apiVIPs := []*models.APIVip{ - {IP: "192.168.127.10"}, - } - - ingressVIPs := []*models.IngressVip{ - {IP: "192.168.127.10"}, - } - - cluster := &common.Cluster{ - Cluster: models.Cluster{ - ID: &clusterID, - OpenshiftVersion: common.MinimumVersionForUserManagedLoadBalancerFeature, - Platform: &models.Platform{ - Type: models.PlatformTypeBaremetal.Pointer(), - }, - CPUArchitecture: common.X86CPUArchitecture, - MachineNetworks: []*models.MachineNetwork{ - {ClusterID: clusterID, Cidr: "192.168.127.0/24"}, - }, - Hosts: []*models.Host{ - { - ID: &masterHostId1, - Inventory: common.GenerateTestInventoryWithNetwork( - common.NetAddress{ - IPv4Address: []string{"192.168.127.12/24"}, - }, - )}, - }, - LoadBalancer: &models.LoadBalancer{Type: models.LoadBalancerTypeClusterManaged}, - }, - } - err := db.Create(cluster).Error - Expect(err).ShouldNot(HaveOccurred()) - - mockClusterApi.EXPECT().VerifyClusterUpdatability(createClusterIdMatcher(cluster)).Return(nil).Times(1) - - reply := bm.V2UpdateCluster(ctx, installer.V2UpdateClusterParams{ - ClusterID: clusterID, - ClusterUpdateParams: &models.V2ClusterUpdateParams{ - APIVips: apiVIPs, - IngressVips: ingressVIPs, - LoadBalancer: &models.LoadBalancer{Type: models.LoadBalancerTypeUserManaged}, - }, - }) - - Expect(reply).To(BeAssignableToTypeOf(installer.NewV2UpdateClusterCreated())) - actual := reply.(*installer.V2UpdateClusterCreated) - responseCluster := actual.Payload - - Expect(responseCluster.LoadBalancer).ToNot(BeNil()) - Expect(responseCluster.LoadBalancer.Type).To(BeEquivalentTo(models.LoadBalancerTypeUserManaged)) - - Expect(responseCluster.APIVips).To(ContainElement(apiVIPs[0])) - Expect(responseCluster.IngressVips).To(ContainElement(ingressVIPs[0])) - }) }) - It("when updating to common VIP, while cluster is already with user managed load balancer", func() { - apiVIPs := []*models.APIVip{ - {IP: "192.168.127.10"}, - } - - ingressVIPs := []*models.IngressVip{ - {IP: "192.168.127.10"}, - } - + DescribeTable("different update scenarios for focused on load balancer type with machine networks and vips", func( + clusterLoadBalancer *models.LoadBalancer, + newLoadBalancer *models.LoadBalancer, + clusterAPIVIPs []*models.APIVip, + newAPIVIPs []*models.APIVip, + clusterIngressVIPs []*models.IngressVip, + newIngressVIPs []*models.IngressVip, + clusterMachineNetworks []*models.MachineNetwork, + newMachineNetworks []*models.MachineNetwork, + errString *string, + ) { cluster := &common.Cluster{ Cluster: models.Cluster{ ID: &clusterID, @@ -8204,9 +8140,6 @@ var _ = Describe("V2UpdateCluster", func() { Type: models.PlatformTypeBaremetal.Pointer(), }, CPUArchitecture: common.X86CPUArchitecture, - MachineNetworks: []*models.MachineNetwork{ - {ClusterID: clusterID, Cidr: "192.168.127.0/24"}, - }, Hosts: []*models.Host{ { ID: &masterHostId1, @@ -8216,269 +8149,247 @@ var _ = Describe("V2UpdateCluster", func() { }, )}, }, - LoadBalancer: &models.LoadBalancer{Type: models.LoadBalancerTypeUserManaged}, + LoadBalancer: clusterLoadBalancer, + MachineNetworks: clusterMachineNetworks, + APIVips: clusterAPIVIPs, + IngressVips: clusterIngressVIPs, }, } - err := db.Create(cluster).Error Expect(err).ShouldNot(HaveOccurred()) - mockClusterApi.EXPECT().VerifyClusterUpdatability(createClusterIdMatcher(cluster)).Return(nil).Times(1) - - reply := bm.V2UpdateCluster(ctx, installer.V2UpdateClusterParams{ - ClusterID: clusterID, - ClusterUpdateParams: &models.V2ClusterUpdateParams{ - APIVips: apiVIPs, - IngressVips: ingressVIPs, - }, - }) - - Expect(reply).To(BeAssignableToTypeOf(installer.NewV2UpdateClusterCreated())) - actual := reply.(*installer.V2UpdateClusterCreated) - responseCluster := actual.Payload - - Expect(responseCluster.LoadBalancer).ToNot(BeNil()) - Expect(responseCluster.LoadBalancer.Type).To(BeEquivalentTo(models.LoadBalancerTypeUserManaged)) - - Expect(responseCluster.APIVips).To(ContainElement(apiVIPs[0])) - Expect(responseCluster.IngressVips).To(ContainElement(ingressVIPs[0])) - }) - - It("when updating to distinct VIPs, while cluster is with user managed load balancer", func() { - apiVIPs := []*models.APIVip{ - {IP: "192.168.127.10"}, - } - - ingressVIPs := []*models.IngressVip{ - {IP: "192.168.127.11"}, - } - - cluster := &common.Cluster{ - Cluster: models.Cluster{ - ID: &clusterID, - OpenshiftVersion: common.MinimumVersionForUserManagedLoadBalancerFeature, - Platform: &models.Platform{ - Type: models.PlatformTypeBaremetal.Pointer(), - }, - CPUArchitecture: common.X86CPUArchitecture, - MachineNetworks: []*models.MachineNetwork{ - {ClusterID: clusterID, Cidr: "192.168.127.0/24"}, - }, - Hosts: []*models.Host{ - { - ID: &masterHostId1, - Inventory: common.GenerateTestInventoryWithNetwork( - common.NetAddress{ - IPv4Address: []string{"192.168.127.12/24"}, - }, - )}, - }, - LoadBalancer: &models.LoadBalancer{Type: models.LoadBalancerTypeUserManaged}, - }, + mockClusterApi.EXPECT().VerifyClusterUpdatability(createClusterIdMatcher(cluster)).Return(nil).AnyTimes() + if errString == nil { + mockDetectAndStoreCollidingIPsForCluster(mockClusterApi, 1) + mockClusterApi.EXPECT().RefreshStatus(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil).Times(1) + mockSetConnectivityMajorityGroupsForCluster(mockClusterApi) + mockHostApi.EXPECT().RefreshInventory(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).Times(1) + mockHostApi.EXPECT().RefreshStatus(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).Times(1) + mockHostApi.EXPECT().GetStagesByRole(gomock.Any(), gomock.Any()).Return(nil).Times(1) } - err := db.Create(cluster).Error - Expect(err).ShouldNot(HaveOccurred()) - - mockClusterApi.EXPECT().VerifyClusterUpdatability(createClusterIdMatcher(cluster)).Return(nil).Times(1) - reply := bm.V2UpdateCluster(ctx, installer.V2UpdateClusterParams{ ClusterID: clusterID, ClusterUpdateParams: &models.V2ClusterUpdateParams{ - APIVips: apiVIPs, - IngressVips: ingressVIPs, + LoadBalancer: newLoadBalancer, + APIVips: newAPIVIPs, + IngressVips: newIngressVIPs, + MachineNetworks: newMachineNetworks, }, }) - Expect(reply).To(BeAssignableToTypeOf(installer.NewV2UpdateClusterCreated())) - actual := reply.(*installer.V2UpdateClusterCreated) - responseCluster := actual.Payload - - Expect(responseCluster.LoadBalancer).ToNot(BeNil()) - Expect(responseCluster.LoadBalancer.Type).To(BeEquivalentTo(models.LoadBalancerTypeUserManaged)) - - Expect(responseCluster.APIVips).To(ContainElement(apiVIPs[0])) - Expect(responseCluster.IngressVips).To(ContainElement(ingressVIPs[0])) - }) - - It("when updating to distinct VIPs, while cluster is with cluster managed load balancer", func() { - apiVIPs := []*models.APIVip{ - {IP: "192.168.127.10"}, - } - - ingressVIPs := []*models.IngressVip{ - {IP: "192.168.127.11"}, - } - - cluster := &common.Cluster{ - Cluster: models.Cluster{ - ID: &clusterID, - OpenshiftVersion: common.MinimumVersionForUserManagedLoadBalancerFeature, - Platform: &models.Platform{ - Type: models.PlatformTypeBaremetal.Pointer(), - }, - CPUArchitecture: common.X86CPUArchitecture, - MachineNetworks: []*models.MachineNetwork{ - {ClusterID: clusterID, Cidr: "192.168.127.0/24"}, - }, - Hosts: []*models.Host{ - { - ID: &masterHostId1, - Inventory: common.GenerateTestInventoryWithNetwork( - common.NetAddress{ - IPv4Address: []string{"192.168.127.12/24"}, - }, - )}, - }, - LoadBalancer: &models.LoadBalancer{Type: models.LoadBalancerTypeUserManaged}, - }, + if errString != nil { + verifyApiErrorString( + reply, + http.StatusBadRequest, + *errString, + ) + return } - err := db.Create(cluster).Error - Expect(err).ShouldNot(HaveOccurred()) - - mockClusterApi.EXPECT().VerifyClusterUpdatability(createClusterIdMatcher(cluster)).Return(nil).Times(1) - - reply := bm.V2UpdateCluster(ctx, installer.V2UpdateClusterParams{ - ClusterID: clusterID, - ClusterUpdateParams: &models.V2ClusterUpdateParams{ - APIVips: apiVIPs, - IngressVips: ingressVIPs, - }, - }) - Expect(reply).To(BeAssignableToTypeOf(installer.NewV2UpdateClusterCreated())) actual := reply.(*installer.V2UpdateClusterCreated) responseCluster := actual.Payload - Expect(responseCluster.LoadBalancer).ToNot(BeNil()) - Expect(responseCluster.LoadBalancer.Type).To(BeEquivalentTo(models.LoadBalancerTypeUserManaged)) - - Expect(responseCluster.APIVips).To(ContainElement(apiVIPs[0])) - Expect(responseCluster.IngressVips).To(ContainElement(ingressVIPs[0])) - }) - }) - - Context("should fail", func() { - It("when updating to cluster managed load balancer with common VIP", func() { - cluster := &common.Cluster{ - Cluster: models.Cluster{ - ID: &clusterID, - OpenshiftVersion: common.MinimumVersionForUserManagedLoadBalancerFeature, - Platform: &models.Platform{ - Type: models.PlatformTypeBaremetal.Pointer(), - }, - CPUArchitecture: common.X86CPUArchitecture, - LoadBalancer: &models.LoadBalancer{Type: models.LoadBalancerTypeUserManaged}, - }, - } - - err := db.Create(cluster).Error - Expect(err).ShouldNot(HaveOccurred()) - - reply := bm.V2UpdateCluster(ctx, installer.V2UpdateClusterParams{ - ClusterID: clusterID, - ClusterUpdateParams: &models.V2ClusterUpdateParams{ - APIVips: []*models.APIVip{ - {IP: "192.168.127.10"}, - }, - IngressVips: []*models.IngressVip{ - {IP: "192.168.127.10"}, - }, - LoadBalancer: &models.LoadBalancer{Type: models.LoadBalancerTypeClusterManaged}, - }, - }) - - verifyApiErrorString( - reply, - http.StatusBadRequest, - "The IP address \"192.168.127.10\" appears both in apiVIPs and ingressVIPs", - ) - }) - - It("when updating to VIPs outside of the machine network, while cluster is with cluster managed load balancer", func() { - cluster := &common.Cluster{ - Cluster: models.Cluster{ - ID: &clusterID, - OpenshiftVersion: common.MinimumVersionForUserManagedLoadBalancerFeature, - Platform: &models.Platform{ - Type: models.PlatformTypeBaremetal.Pointer(), - }, - CPUArchitecture: common.X86CPUArchitecture, - LoadBalancer: &models.LoadBalancer{Type: models.LoadBalancerTypeClusterManaged}, - MachineNetworks: []*models.MachineNetwork{ - {Cidr: "192.168.127.0/24"}, - }, - }, + if newLoadBalancer != nil { + Expect(responseCluster.LoadBalancer).ToNot(BeNil()) + Expect(responseCluster.LoadBalancer.Type).To(BeEquivalentTo(newLoadBalancer.Type)) + } else { + Expect(responseCluster.LoadBalancer).To(Equal(clusterLoadBalancer)) } - err := db.Create(cluster).Error - Expect(err).ShouldNot(HaveOccurred()) - - mockClusterApi.EXPECT().VerifyClusterUpdatability(createClusterIdMatcher(cluster)).Return(nil).Times(1) - - reply := bm.V2UpdateCluster(ctx, installer.V2UpdateClusterParams{ - ClusterID: clusterID, - ClusterUpdateParams: &models.V2ClusterUpdateParams{ - APIVips: []*models.APIVip{ - {IP: "192.168.128.10"}, - }, - IngressVips: []*models.IngressVip{ - {IP: "192.168.128.11"}, - }, - }, - }) - - verifyApiErrorString( - reply, - http.StatusBadRequest, - "Calculate machine network CIDR: No suitable matching CIDR found for VIP 192.168.128.10", - ) - }) - - It("when updating to VIPs outside of the machine network, while cluster is with user managed load balancer", func() { - apiVIPs := []*models.APIVip{ - {IP: "192.168.128.10"}, + if newAPIVIPs != nil { + Expect(len(responseCluster.APIVips)).To(Equal(len(newAPIVIPs))) + Expect(*responseCluster.APIVips[0]).To(Equal(*newAPIVIPs[0])) + } else { + if clusterAPIVIPs == nil { + Expect(responseCluster.APIVips).To(Equal([]*models.APIVip{})) + } else { + Expect(responseCluster.APIVips).To(Equal(clusterAPIVIPs)) + } } - ingressVIPs := []*models.IngressVip{ - {IP: "192.168.128.11"}, + if newIngressVIPs != nil { + Expect(len(responseCluster.IngressVips)).To(Equal(len(newIngressVIPs))) + Expect(*responseCluster.IngressVips[0]).To(Equal(*newIngressVIPs[0])) + } else { + if clusterIngressVIPs == nil { + Expect(responseCluster.IngressVips).To(Equal([]*models.IngressVip{})) + } else { + Expect(responseCluster.IngressVips).To(Equal(clusterIngressVIPs)) + } } - cluster := &common.Cluster{ - Cluster: models.Cluster{ - ID: &clusterID, - OpenshiftVersion: common.MinimumVersionForUserManagedLoadBalancerFeature, - Platform: &models.Platform{ - Type: models.PlatformTypeBaremetal.Pointer(), - }, - CPUArchitecture: common.X86CPUArchitecture, - LoadBalancer: &models.LoadBalancer{Type: models.LoadBalancerTypeUserManaged}, - MachineNetworks: []*models.MachineNetwork{ - {Cidr: "192.168.127.0/24"}, - }, - }, + if newMachineNetworks != nil { + Expect(len(responseCluster.MachineNetworks)).To(Equal(len(newMachineNetworks))) + for k := range newMachineNetworks { + Expect(*responseCluster.MachineNetworks[k]).To(Equal(*newMachineNetworks[k])) + } + } else { + if clusterMachineNetworks == nil { + Expect(responseCluster.MachineNetworks).To(Equal([]*models.MachineNetwork{})) + } else { + Expect(responseCluster.MachineNetworks).To(Equal(cluster.MachineNetworks)) + } } - - err := db.Create(cluster).Error - Expect(err).ShouldNot(HaveOccurred()) - - mockClusterApi.EXPECT().VerifyClusterUpdatability(createClusterIdMatcher(cluster)).Return(nil).Times(1) - - reply := bm.V2UpdateCluster(ctx, installer.V2UpdateClusterParams{ - ClusterID: clusterID, - ClusterUpdateParams: &models.V2ClusterUpdateParams{ - APIVips: apiVIPs, - IngressVips: ingressVIPs, - }, - }) - - verifyApiErrorString( - reply, - http.StatusBadRequest, - "Calculate machine network CIDR: No suitable matching CIDR found for VIP 192.168.128.10", - ) - }) + }, + Entry("update load_balancer type to user-managed when it is nil should succeed", + nil, + &models.LoadBalancer{Type: models.LoadBalancerTypeUserManaged}, + nil, + nil, + nil, + nil, + nil, + nil, + nil, + ), + + Entry("update load_balancer type to user-managed when it is cluster-managed should succeed", + &models.LoadBalancer{Type: models.LoadBalancerTypeClusterManaged}, + &models.LoadBalancer{Type: models.LoadBalancerTypeUserManaged}, + nil, + nil, + nil, + nil, + nil, + nil, + nil, + ), + + Entry("update load_balancer type to cluster-managed when it is nil should succeed", + nil, + &models.LoadBalancer{Type: models.LoadBalancerTypeClusterManaged}, + nil, + nil, + nil, + nil, + nil, + nil, + nil, + ), + + Entry("update load_balancer type to cluster-managed when it is user-managed should succeed", + &models.LoadBalancer{Type: models.LoadBalancerTypeUserManaged}, + &models.LoadBalancer{Type: models.LoadBalancerTypeClusterManaged}, + nil, + nil, + nil, + nil, + nil, + nil, + nil, + ), + + Entry("update load_balancer type to user-managed when it is user-managed should succeed", + &models.LoadBalancer{Type: models.LoadBalancerTypeUserManaged}, + &models.LoadBalancer{Type: models.LoadBalancerTypeUserManaged}, + nil, + nil, + nil, + nil, + nil, + nil, + nil, + ), + + Entry("update load_balancer type to cluster-managed when it is cluster-managed should succeed", + &models.LoadBalancer{Type: models.LoadBalancerTypeClusterManaged}, + &models.LoadBalancer{Type: models.LoadBalancerTypeClusterManaged}, + nil, + nil, + nil, + nil, + nil, + nil, + nil, + ), + + Entry("adding machine networks with cluster-managed load balancer should fail", + &models.LoadBalancer{Type: models.LoadBalancerTypeClusterManaged}, + nil, + nil, + nil, + nil, + nil, + nil, + []*models.MachineNetwork{{ClusterID: clusterID, Cidr: "192.168.127.0/24"}}, + swag.String("Setting the Machine Network CIDR is forbidden when the cluster is neither"+ + " in VIP DHCP allocation mode nor using a user-managed load balance"), + ), + + Entry("adding machine networks with user-managed load balancer should succeed", + &models.LoadBalancer{Type: models.LoadBalancerTypeUserManaged}, + nil, + nil, + nil, + nil, + nil, + nil, + []*models.MachineNetwork{{ClusterID: clusterID, Cidr: "192.168.127.0/24"}}, + nil, + ), + + Entry("adding common VIPs with cluster-managed load balancer should fail", + &models.LoadBalancer{Type: models.LoadBalancerTypeClusterManaged}, + nil, + nil, + []*models.APIVip{{ClusterID: clusterID, IP: "192.168.127.1"}}, + nil, + []*models.IngressVip{{ClusterID: clusterID, IP: "192.168.127.1"}}, + []*models.MachineNetwork{{ClusterID: clusterID, Cidr: "192.168.127.0/24"}}, + nil, + swag.String("appears both in apiVIPs and ingressVIPs"), + ), + + Entry("adding common VIPs with user-managed load balancer should succeed", + &models.LoadBalancer{Type: models.LoadBalancerTypeUserManaged}, + nil, + nil, + []*models.APIVip{{ClusterID: clusterID, IP: "192.168.127.1"}}, + nil, + []*models.IngressVip{{ClusterID: clusterID, IP: "192.168.127.1"}}, + nil, + []*models.MachineNetwork{{ClusterID: clusterID, Cidr: "192.168.127.0/24"}}, + nil, + ), + + Entry("update to multiple machine networks with user-managed load balancer should succeed", + &models.LoadBalancer{Type: models.LoadBalancerTypeUserManaged}, + nil, + nil, + []*models.APIVip{{ClusterID: clusterID, IP: "192.168.127.1"}}, + nil, + []*models.IngressVip{{ClusterID: clusterID, IP: "192.168.127.1"}}, + nil, + []*models.MachineNetwork{{ClusterID: clusterID, Cidr: "192.168.127.0/24"}, {ClusterID: clusterID, Cidr: "192.168.128.0/24"}}, + nil, + ), + + Entry("update to multiple machine networks such that only one satisfies the requirements with user-managed load balancer should succeed", + &models.LoadBalancer{Type: models.LoadBalancerTypeUserManaged}, + nil, + nil, + []*models.APIVip{{ClusterID: clusterID, IP: "192.168.127.1"}}, + nil, + []*models.IngressVip{{ClusterID: clusterID, IP: "192.168.127.1"}}, + nil, + []*models.MachineNetwork{{ClusterID: clusterID, Cidr: "192.168.127.0/24"}, {ClusterID: clusterID, Cidr: "192.168.128.0/24"}}, + nil, + ), + + Entry("update to multiple machine networks such that none satisfies the requirements with user-managed load balancer should fail", + &models.LoadBalancer{Type: models.LoadBalancerTypeUserManaged}, + nil, + nil, + []*models.APIVip{{ClusterID: clusterID, IP: "192.168.127.1"}}, + nil, + []*models.IngressVip{{ClusterID: clusterID, IP: "192.168.127.1"}}, + nil, + []*models.MachineNetwork{{ClusterID: clusterID, Cidr: "192.168.126.0/24"}, {ClusterID: clusterID, Cidr: "192.168.128.0/24"}}, + swag.String("failed verification: none of the machine networks is satisfying the requirements"), + ), + ) }) }) @@ -16472,12 +16383,49 @@ location = "%s" Expect(responseCluster.APIVips).To(ContainElement(apiVips[0])) Expect(responseCluster.IngressVips).To(ContainElement(ingressVIPs[0])) }) + + It("when registering cluster with user managed load balancer and VIPs inside second machine network", func() { + apiVips := []*models.APIVip{ + {IP: "192.168.128.1"}, + } + ingressVIPs := []*models.IngressVip{ + {IP: "192.168.128.2"}, + } + reply := bm.V2RegisterCluster(ctx, installer.V2RegisterClusterParams{ + NewClusterParams: &models.ClusterCreateParams{ + OpenshiftVersion: swag.String(common.MinimumVersionForUserManagedLoadBalancerFeature), + CPUArchitecture: common.X86CPUArchitecture, + Platform: &models.Platform{ + Type: models.PlatformTypeBaremetal.Pointer(), + }, + LoadBalancer: &models.LoadBalancer{ + Type: models.LoadBalancerTypeUserManaged, + }, + APIVips: apiVips, + IngressVips: ingressVIPs, + MachineNetworks: []*models.MachineNetwork{ + {Cidr: "192.168.127.0/24"}, + {Cidr: "192.168.128.0/30"}, + }, + }, + }) + + Expect(reply).To(BeAssignableToTypeOf(installer.NewV2RegisterClusterCreated())) + actual := reply.(*installer.V2RegisterClusterCreated) + responseCluster := actual.Payload + + Expect(responseCluster.LoadBalancer).ToNot(BeNil()) + Expect(responseCluster.LoadBalancer.Type).To(BeEquivalentTo(models.LoadBalancerTypeUserManaged)) + + Expect(responseCluster.APIVips).To(ContainElement(apiVips[0])) + Expect(responseCluster.IngressVips).To(ContainElement(ingressVIPs[0])) + }) }) Context("should fail", func() { - It("when registering cluster with user managed load balancer and VIPs outside machine network", func() { + It("when registering cluster with user managed load balancer and API VIP outside all machine networks", func() { apiVips := []*models.APIVip{ - {IP: "192.168.128.10"}, + {IP: "192.168.129.10"}, } ingressVIPs := []*models.IngressVip{ {IP: "192.168.128.11"}, @@ -16496,6 +16444,7 @@ location = "%s" IngressVips: ingressVIPs, MachineNetworks: []*models.MachineNetwork{ {Cidr: "192.168.127.0/24"}, + {Cidr: "192.168.128.0/24"}, }, }, }) @@ -16503,7 +16452,50 @@ location = "%s" verifyApiErrorString( reply, http.StatusBadRequest, - "api-vip <192.168.128.10> does not belong to machine-network-cidr <192.168.127.0/24>", + "api-vip '192.168.129.10' failed verification: "+ + "none of the machine networks is satisfying the requirements, description: "+ + "machine network CIDR <192.168.127.0/24> verification failed: "+ + "api-vip <192.168.129.10> does not belong to machine-network-cidr <192.168.127.0/24>; "+ + "machine network CIDR <192.168.128.0/24> verification failed: "+ + "api-vip <192.168.129.10> does not belong to machine-network-cidr <192.168.128.0/24>", + ) + }) + + It("when registering cluster with user managed load balancer and Ingress VIP outside all machine networks", func() { + apiVips := []*models.APIVip{ + {IP: "192.168.129.10"}, + } + ingressVIPs := []*models.IngressVip{ + {IP: "192.168.127.11"}, + } + reply := bm.V2RegisterCluster(ctx, installer.V2RegisterClusterParams{ + NewClusterParams: &models.ClusterCreateParams{ + OpenshiftVersion: swag.String(common.MinimumVersionForUserManagedLoadBalancerFeature), + CPUArchitecture: common.X86CPUArchitecture, + Platform: &models.Platform{ + Type: models.PlatformTypeBaremetal.Pointer(), + }, + LoadBalancer: &models.LoadBalancer{ + Type: models.LoadBalancerTypeUserManaged, + }, + APIVips: apiVips, + IngressVips: ingressVIPs, + MachineNetworks: []*models.MachineNetwork{ + {Cidr: "192.168.128.0/24"}, + {Cidr: "192.168.129.0/24"}, + }, + }, + }) + + verifyApiErrorString( + reply, + http.StatusBadRequest, + "ingress-vip '192.168.127.11' failed verification: "+ + "none of the machine networks is satisfying the requirements, description: "+ + "machine network CIDR <192.168.128.0/24> verification failed: "+ + "ingress-vip <192.168.127.11> does not belong to machine-network-cidr <192.168.128.0/24>; "+ + "machine network CIDR <192.168.129.0/24> verification failed: "+ + "ingress-vip <192.168.127.11> does not belong to machine-network-cidr <192.168.129.0/24>", ) }) @@ -16539,6 +16531,39 @@ location = "%s" ) }) + It("when cluster managed load balancer with multiple machine networks provided", func() { + apiVips := []*models.APIVip{ + {IP: "192.168.127.10"}, + } + ingressVIPs := []*models.IngressVip{ + {IP: "192.168.127.11"}, + } + reply := bm.V2RegisterCluster(ctx, installer.V2RegisterClusterParams{ + NewClusterParams: &models.ClusterCreateParams{ + OpenshiftVersion: swag.String(common.MinimumVersionForUserManagedLoadBalancerFeature), + CPUArchitecture: common.X86CPUArchitecture, + Platform: &models.Platform{ + Type: models.PlatformTypeBaremetal.Pointer(), + }, + LoadBalancer: &models.LoadBalancer{ + Type: models.LoadBalancerTypeClusterManaged, + }, + APIVips: apiVips, + IngressVips: ingressVIPs, + MachineNetworks: []*models.MachineNetwork{ + {Cidr: "192.168.127.0/24"}, + {Cidr: "192.168.128.0/24"}, + }, + }, + }) + + verifyApiErrorString( + reply, + http.StatusBadRequest, + "Single-stack cluster cannot contain multiple Machine Networks", + ) + }) + It("when cluster managed load balancer with common VIP", func() { apiVips := []*models.APIVip{ {IP: "192.168.127.10"}, diff --git a/internal/cluster/cluster.go b/internal/cluster/cluster.go index 01740a5a9d6..af7fd45b4e1 100644 --- a/internal/cluster/cluster.go +++ b/internal/cluster/cluster.go @@ -426,6 +426,13 @@ func (m *Manager) tryAssignMachineCidrDHCPMode(cluster *common.Cluster) error { } func (m *Manager) tryAssignMachineCidrNonDHCPMode(cluster *common.Cluster) error { + // With user-managed load balancer we can't calculate the + // machine network as the vips might be outside the hosts subnets. + // It should be set by the user manually. + if network.IsLoadBalancerUserManaged(cluster) { + return nil + } + primaryMachineNetwork, err := network.CalculateMachineNetworkCIDR( network.GetApiVipById(cluster, 0), network.GetIngressVipById(cluster, 0), cluster.Hosts, false) if err != nil { diff --git a/internal/cluster/validations/validations.go b/internal/cluster/validations/validations.go index 19a748b757e..33c5ebdb1e8 100644 --- a/internal/cluster/validations/validations.go +++ b/internal/cluster/validations/validations.go @@ -481,7 +481,7 @@ func validateVIPAddresses(ipV6Supported bool, targetConfiguration common.Cluster if err != nil { return err } - err = ValidateDualStackNetworks(targetConfiguration, false) + err = ValidateDualStackNetworks(targetConfiguration, false, false) if err != nil { return err } @@ -504,11 +504,16 @@ func validateVIPAddresses(ipV6Supported bool, targetConfiguration common.Cluster targetConfiguration.APIVips, targetConfiguration.IngressVips); err != nil { return common.NewApiError(http.StatusBadRequest, err) } - } else { + } else if !network.IsLoadBalancerUserManaged(&targetConfiguration) { if len(targetConfiguration.MachineNetworks) > 0 { for i := range targetConfiguration.APIVips { // len of APIVips and IngressVips should be the same. asserted above. - err = network.VerifyVips(nil, string(targetConfiguration.MachineNetworks[i].Cidr), - string(targetConfiguration.APIVips[i].IP), string(targetConfiguration.IngressVips[i].IP), network.IsLoadBalancerUserManaged(&targetConfiguration), nil) + err = network.VerifyVipsForClusterManagedLoadBalancer( + nil, + string(targetConfiguration.MachineNetworks[i].Cidr), + string(targetConfiguration.APIVips[i].IP), + string(targetConfiguration.IngressVips[i].IP), + nil, + ) if err != nil { multiErr = multierror.Append(multiErr, err) } @@ -519,6 +524,18 @@ func validateVIPAddresses(ipV6Supported bool, targetConfiguration common.Cluster } else if reqDualStack { return errors.New("Dual-stack cluster cannot be created with empty Machine Networks") } + } else { + if len(targetConfiguration.MachineNetworks) == 0 || len(targetConfiguration.APIVips) == 0 || len(targetConfiguration.IngressVips) == 0 { + return nil + } + + return network.VerifyVipsForUserManangedLoadBalancer( + nil, + targetConfiguration.MachineNetworks, + string(targetConfiguration.APIVips[0].IP), + string(targetConfiguration.IngressVips[0].IP), + nil, + ) } return nil @@ -552,10 +569,11 @@ func ValidateVIPsWereNotSetDhcpMode(apiVips []*models.APIVip, ingressVips []*mod return nil } -func ValidateDualStackNetworks(clusterParams interface{}, alreadyDualStack bool) error { +func ValidateDualStackNetworks(clusterParams interface{}, alreadyDualStack bool, alreadyUserManagedLoadBalancer bool) error { var machineNetworks []*models.MachineNetwork var serviceNetworks []*models.ServiceNetwork var clusterNetworks []*models.ClusterNetwork + var clusterLoadBalancer *models.LoadBalancer var err error var ipv4, ipv6 bool reqDualStack := false @@ -563,6 +581,15 @@ func ValidateDualStackNetworks(clusterParams interface{}, alreadyDualStack bool) machineNetworks = network.DerefMachineNetworks(funk.Get(clusterParams, "MachineNetworks")) serviceNetworks = network.DerefServiceNetworks(funk.Get(clusterParams, "ServiceNetworks")) clusterNetworks = network.DerefClusterNetworks(funk.Get(clusterParams, "ClusterNetworks")) + clusterLoadBalancer = network.DerefClusterLoadBalancer(funk.Get(clusterParams, "LoadBalancer")) + + var targetLoadBalancerType string = models.LoadBalancerTypeClusterManaged + if alreadyUserManagedLoadBalancer { + targetLoadBalancerType = models.LoadBalancerTypeUserManaged + } + if clusterLoadBalancer != nil { + targetLoadBalancerType = clusterLoadBalancer.Type + } ipv4, ipv6, err = network.GetAddressFamilies(machineNetworks) if err != nil { @@ -612,7 +639,7 @@ func ValidateDualStackNetworks(clusterParams interface{}, alreadyDualStack bool) } } } else { - if len(machineNetworks) > 1 { + if len(machineNetworks) > 1 && targetLoadBalancerType == models.LoadBalancerTypeClusterManaged { err := errors.Errorf("Single-stack cluster cannot contain multiple Machine Networks") return err } diff --git a/internal/cluster/validator.go b/internal/cluster/validator.go index 3682995f8cf..382c7c9853d 100644 --- a/internal/cluster/validator.go +++ b/internal/cluster/validator.go @@ -121,6 +121,9 @@ func (v *clusterValidator) isMachineCidrEqualsToCalculatedCidr(c *clusterPreproc if swag.BoolValue(c.cluster.UserManagedNetworking) { return ValidationSuccess, "The Cluster Machine CIDR is not required: User Managed Networking" } + if network.IsLoadBalancerUserManaged(c.cluster) { + return ValidationSuccess, "Calculating machine network CIDR is not enabled: User Managed Load Balancer" + } if len(c.cluster.APIVips) == 0 && len(c.cluster.IngressVips) == 0 { return ValidationPending, "The Machine Network CIDR, API virtual IPs, or Ingress virtual IPs are undefined." } @@ -214,12 +217,6 @@ func (v *clusterValidator) areApiVipsDefined(c *clusterPreprocessContext) (Valid } func (v *clusterValidator) areVipsValid(c *clusterPreprocessContext, vipsWrapper VipsWrapper) (ValidationStatus, string) { - var ( - multiErr *multierror.Error - msg string - ) - - name := strings.ToLower(vipsWrapper.Name()) + " vips" if swag.BoolValue(c.cluster.UserManagedNetworking) { return ValidationSuccess, fmt.Sprintf("%s virtual IPs are not required: User Managed Networking", vipsWrapper.Name()) } @@ -236,21 +233,35 @@ func (v *clusterValidator) areVipsValid(c *clusterPreprocessContext, vipsWrapper return ValidationPending, "Hosts have not been discovered yet" } - // When using user managed load balancer, the VIPs are the load balancer IP (not virtual), - // therefore will not be free - shouldVerifyVIPIsFree := !network.IsLoadBalancerUserManaged(c.cluster) + if !network.IsLoadBalancerUserManaged(c.cluster) { + return validateVipsClusterManagedLoadBalancer(c, vipsWrapper, v.log) + } + + return validateVipsUserManagedLoadBalancer(c, vipsWrapper, v.log) +} + +func validateVipsClusterManagedLoadBalancer( + c *clusterPreprocessContext, vipsWrapper VipsWrapper, log logrus.FieldLogger, +) (ValidationStatus, string) { + var ( + multiErr *multierror.Error + msg string + ) + + name := strings.ToLower(vipsWrapper.Name()) + " vips" failed := false for i := 0; i != vipsWrapper.Len(); i++ { - verification, err := network.VerifyVip( + verification, err := network.VerifyVipWithSingleMachineNetwork( c.cluster.Hosts, network.GetMachineCidrById(c.cluster, i), vipsWrapper.IP(i), name, vipsWrapper.Verification(i), - shouldVerifyVIPIsFree, - v.log, + true, + log, ) + failed = failed || verification != models.VipVerificationSucceeded if err != nil { multiErr = multierror.Append(multiErr, err) @@ -267,6 +278,34 @@ func (v *clusterValidator) areVipsValid(c *clusterPreprocessContext, vipsWrapper return ValidationSuccess, fmt.Sprintf("%s %s belongs to the Machine CIDR and is not in use.", name, strings.Join(vipsWrapper.GetVips(), `, `)) } +func validateVipsUserManagedLoadBalancer( + c *clusterPreprocessContext, vipsWrapper VipsWrapper, log logrus.FieldLogger, +) (ValidationStatus, string) { + if vipsWrapper.Len() == 0 { + return ValidationPending, fmt.Sprintf("%s virtual IPs are undefined.", vipsWrapper.Name()) + } + + name := strings.ToLower(vipsWrapper.Name()) + " vip" + verification, err := network.VerifyVipWithMachineNetworkList( + c.cluster.Hosts, + c.cluster.MachineNetworks, + vipsWrapper.IP(0), + name, + vipsWrapper.Verification(0), + false, + log, + ) + + if verification != models.VipVerificationSucceeded { + if err != nil { + return ValidationFailure, err.Error() + } + return ValidationFailure, fmt.Sprintf("%s %s is invalid", name, vipsWrapper.IP(0)) + } + + return ValidationSuccess, fmt.Sprintf("%s %s is valid", name, vipsWrapper.IP(0)) +} + func (v *clusterValidator) areApiVipsValid(c *clusterPreprocessContext) (ValidationStatus, string) { return v.areVipsValid(c, &ApiVipsWrapper{c: c}) } diff --git a/internal/cluster/validator_test.go b/internal/cluster/validator_test.go index 885fbc0f29b..b95d9d35e96 100644 --- a/internal/cluster/validator_test.go +++ b/internal/cluster/validator_test.go @@ -332,8 +332,30 @@ var _ = Describe("areVipsValid", func() { ingressContext := loopContext{name: "Ingress", function: validator.areIngressVipsValid} for _, lc := range []loopContext{apiContext, ingressContext} { lcontext := lc - Context(fmt.Sprintf("%s vips validation", lcontext.name), func() { - It("Vips undefined", func() { + Context(fmt.Sprintf("- %s vips validation:", lcontext.name), func() { + It("user-managed networking", func() { + preprocessContext.cluster = &common.Cluster{Cluster: models.Cluster{ + ID: &clusterID, + UserManagedNetworking: swag.Bool(true), + }} + + status, message := lcontext.function(preprocessContext) + Expect(status).Should(Equal(ValidationSuccess)) + Expect(message).Should(Equal(fmt.Sprintf("%s virtual IPs are not required: User Managed Networking", lcontext.name))) + }) + + It("SNO", func() { + preprocessContext.cluster = &common.Cluster{Cluster: models.Cluster{ + ID: &clusterID, + HighAvailabilityMode: swag.String(models.ClusterCreateParamsHighAvailabilityModeNone), + }} + + status, message := lcontext.function(preprocessContext) + Expect(status).Should(Equal(ValidationSuccess)) + Expect(message).Should(Equal(fmt.Sprintf("%s virtual IPs are not required: SNO", lcontext.name))) + }) + + It("vips undefined", func() { preprocessContext.cluster = &common.Cluster{Cluster: models.Cluster{ ID: &clusterID, }} @@ -342,6 +364,7 @@ var _ = Describe("areVipsValid", func() { Expect(status).Should(Equal(ValidationPending)) Expect(message).Should(Equal(fmt.Sprintf("%s virtual IPs are undefined.", lcontext.name))) }) + It("vips defined - no hosts", func() { preprocessContext.cluster = &common.Cluster{Cluster: models.Cluster{ ID: &clusterID, @@ -353,12 +376,43 @@ var _ = Describe("areVipsValid", func() { Expect(status).Should(Equal(ValidationPending)) Expect(message).Should(Equal("Hosts have not been discovered yet")) }) - It("vips defined - with hosts", func() { + + It("cluster-managed load balancer - vip outside machine network", func() { + preprocessContext.cluster = &common.Cluster{Cluster: models.Cluster{ + ID: &clusterID, + MachineNetworks: []*models.MachineNetwork{{Cidr: "1.2.4.0/24"}}, + APIVips: []*models.APIVip{{ClusterID: clusterID, IP: "1.2.3.5"}}, + IngressVips: []*models.IngressVip{{ClusterID: clusterID, IP: "1.2.3.6"}}, + Hosts: hosts, + }} + preprocessContext.hasHostsWithInventories = true + + status, message := lcontext.function(preprocessContext) + Expect(status).Should(Equal(ValidationFailure)) + Expect(message).Should(ContainSubstring("does not belong to machine-network-cidr")) + }) + + It("cluster-managed load balancer - vip is the broadcast address in machine network", func() { + preprocessContext.cluster = &common.Cluster{Cluster: models.Cluster{ + ID: &clusterID, + MachineNetworks: []*models.MachineNetwork{{Cidr: "1.2.3.0/24"}}, + APIVips: []*models.APIVip{{ClusterID: clusterID, IP: "1.2.3.255"}}, + IngressVips: []*models.IngressVip{{ClusterID: clusterID, IP: "1.2.3.255"}}, + Hosts: hosts, + }} + preprocessContext.hasHostsWithInventories = true + + status, message := lcontext.function(preprocessContext) + Expect(status).Should(Equal(ValidationFailure)) + Expect(message).Should(ContainSubstring("is the broadcast address of machine-network-cidr")) + }) + + It("cluster-managed load balancer - no free addresses", func() { preprocessContext.cluster = &common.Cluster{Cluster: models.Cluster{ ID: &clusterID, MachineNetworks: common.TestDualStackNetworking.MachineNetworks, - APIVips: clearApiVipsVerfication(common.TestDualStackNetworking.APIVips), - IngressVips: clearIngressVIpsVerification(common.TestDualStackNetworking.IngressVips), + APIVips: []*models.APIVip{{ClusterID: clusterID, IP: "1.2.3.5"}}, + IngressVips: []*models.IngressVip{{ClusterID: clusterID, IP: "1.2.3.6"}}, Hosts: hosts, }} preprocessContext.hasHostsWithInventories = true @@ -367,21 +421,183 @@ var _ = Describe("areVipsValid", func() { Expect(status).Should(Equal(ValidationFailure)) Expect(message).Should(MatchRegexp(fmt.Sprintf("%s vips <1.2.3.[56]> are not verified yet", strings.ToLower(lcontext.name)))) }) - It("ipv4 vips verified", func() { + + It("cluster-managed load balancer - vips in the free addresses list", func() { + hosts = []*models.Host{ + { + FreeAddresses: makeFreeNetworksAddressesStr(makeFreeAddresses( + "1.2.3.0/24", "1.2.3.5", "1.2.3.6", + )), + }, + { + FreeAddresses: makeFreeNetworksAddressesStr(makeFreeAddresses( + "1.2.3.0/24", "1.2.3.5", "1.2.3.6", + )), + }, + { + FreeAddresses: makeFreeNetworksAddressesStr(makeFreeAddresses( + "1.2.3.0/24", "1.2.3.5", "1.2.3.6", + )), + }, + } preprocessContext.cluster = &common.Cluster{Cluster: models.Cluster{ ID: &clusterID, MachineNetworks: common.TestDualStackNetworking.MachineNetworks, - APIVips: append(common.TestIPv4Networking.APIVips, clearApiVipsVerfication(common.TestIPv6Networking.APIVips)...), - IngressVips: append(common.TestIPv4Networking.IngressVips, clearIngressVIpsVerification(common.TestIPv6Networking.IngressVips)...), + APIVips: []*models.APIVip{{ClusterID: clusterID, IP: "1.2.3.5"}}, + IngressVips: []*models.IngressVip{{ClusterID: clusterID, IP: "1.2.3.6"}}, + Hosts: hosts, + }} + preprocessContext.hasHostsWithInventories = true + + status, message := lcontext.function(preprocessContext) + Expect(status).Should(Equal(ValidationSuccess)) + Expect(message).Should(MatchRegexp(fmt.Sprintf("%s vips 1.2.3.[56] belongs to the Machine CIDR and is not in use.", strings.ToLower(lcontext.name)))) + }) + + It("cluster-managed load balancer - verification succeeded already", func() { + preprocessContext.cluster = &common.Cluster{Cluster: models.Cluster{ + ID: &clusterID, + MachineNetworks: common.TestDualStackNetworking.MachineNetworks, + APIVips: []*models.APIVip{{ClusterID: clusterID, IP: "1.2.3.5", Verification: models.NewVipVerification(models.VipVerificationSucceeded)}}, + IngressVips: []*models.IngressVip{{ClusterID: clusterID, IP: "1.2.3.6", Verification: models.NewVipVerification(models.VipVerificationSucceeded)}}, + Hosts: hosts, + }} + preprocessContext.hasHostsWithInventories = true + + status, message := lcontext.function(preprocessContext) + Expect(status).Should(Equal(ValidationSuccess)) + Expect(message).Should(MatchRegexp(fmt.Sprintf("%s vips 1.2.3.[56] belongs to the Machine CIDR and is not in use.", strings.ToLower(lcontext.name)))) + }) + + It("cluster-managed load balancer - verification failed already", func() { + preprocessContext.cluster = &common.Cluster{Cluster: models.Cluster{ + ID: &clusterID, + MachineNetworks: common.TestDualStackNetworking.MachineNetworks, + APIVips: []*models.APIVip{{ClusterID: clusterID, IP: "1.2.3.5", Verification: models.NewVipVerification(models.VipVerificationFailed)}}, + IngressVips: []*models.IngressVip{{ClusterID: clusterID, IP: "1.2.3.6", Verification: models.NewVipVerification(models.VipVerificationFailed)}}, Hosts: hosts, }} preprocessContext.hasHostsWithInventories = true status, message := lcontext.function(preprocessContext) Expect(status).Should(Equal(ValidationFailure)) - Expect(message).Should(MatchRegexp(fmt.Sprintf("%s vips <1001:db8::6[45]> are not verified yet", strings.ToLower(lcontext.name)))) + Expect(message).Should(MatchRegexp(fmt.Sprintf("%s vips <1.2.3.[56]> is already in use in cidr 1.2.3.0/24", strings.ToLower(lcontext.name)))) + }) + + It("user-managed load balancer - vip outside machine network", func() { + preprocessContext.cluster = &common.Cluster{Cluster: models.Cluster{ + ID: &clusterID, + MachineNetworks: []*models.MachineNetwork{{Cidr: "1.2.4.0/24"}}, + APIVips: []*models.APIVip{{ClusterID: clusterID, IP: "1.2.3.5"}}, + IngressVips: []*models.IngressVip{{ClusterID: clusterID, IP: "1.2.3.6"}}, + Hosts: hosts, + LoadBalancer: &models.LoadBalancer{Type: models.LoadBalancerTypeUserManaged}, + }} + preprocessContext.hasHostsWithInventories = true + + status, message := lcontext.function(preprocessContext) + Expect(status).Should(Equal(ValidationFailure)) + Expect(message).Should(ContainSubstring("does not belong to machine-network-cidr")) }) - It("ipv4 vips verified", func() { + + It("user-managed load balancer - vip is the broadcast address in machine network", func() { + preprocessContext.cluster = &common.Cluster{Cluster: models.Cluster{ + ID: &clusterID, + MachineNetworks: []*models.MachineNetwork{{Cidr: "1.2.3.0/24"}}, + APIVips: []*models.APIVip{{ClusterID: clusterID, IP: "1.2.3.255"}}, + IngressVips: []*models.IngressVip{{ClusterID: clusterID, IP: "1.2.3.255"}}, + Hosts: hosts, + LoadBalancer: &models.LoadBalancer{Type: models.LoadBalancerTypeUserManaged}, + }} + preprocessContext.hasHostsWithInventories = true + + status, message := lcontext.function(preprocessContext) + Expect(status).Should(Equal(ValidationFailure)) + Expect(message).Should(ContainSubstring("is the broadcast address of machine-network-cidr")) + }) + + It("user-managed load balancer - no free addresses", func() { + preprocessContext.cluster = &common.Cluster{Cluster: models.Cluster{ + ID: &clusterID, + MachineNetworks: common.TestDualStackNetworking.MachineNetworks, + APIVips: []*models.APIVip{{ClusterID: clusterID, IP: "1.2.3.5"}}, + IngressVips: []*models.IngressVip{{ClusterID: clusterID, IP: "1.2.3.6"}}, + Hosts: hosts, + LoadBalancer: &models.LoadBalancer{Type: models.LoadBalancerTypeUserManaged}, + }} + preprocessContext.hasHostsWithInventories = true + + status, message := lcontext.function(preprocessContext) + Expect(status).Should(Equal(ValidationSuccess)) + Expect(message).Should(MatchRegexp(fmt.Sprintf("%s vip 1.2.3.[56] is valid", strings.ToLower(lcontext.name)))) + }) + + It("user-managed load balancer - vips in the free addresses list", func() { + hosts = []*models.Host{ + { + FreeAddresses: makeFreeNetworksAddressesStr(makeFreeAddresses( + "1.2.3.0/24", "1.2.3.5", "1.2.3.6", + )), + }, + { + FreeAddresses: makeFreeNetworksAddressesStr(makeFreeAddresses( + "1.2.3.0/24", "1.2.3.5", "1.2.3.6", + )), + }, + { + FreeAddresses: makeFreeNetworksAddressesStr(makeFreeAddresses( + "1.2.3.0/24", "1.2.3.5", "1.2.3.6", + )), + }, + } + preprocessContext.cluster = &common.Cluster{Cluster: models.Cluster{ + ID: &clusterID, + MachineNetworks: common.TestDualStackNetworking.MachineNetworks, + APIVips: []*models.APIVip{{ClusterID: clusterID, IP: "1.2.3.5"}}, + IngressVips: []*models.IngressVip{{ClusterID: clusterID, IP: "1.2.3.6"}}, + Hosts: hosts, + LoadBalancer: &models.LoadBalancer{Type: models.LoadBalancerTypeUserManaged}, + }} + preprocessContext.hasHostsWithInventories = true + + status, message := lcontext.function(preprocessContext) + Expect(status).Should(Equal(ValidationSuccess)) + Expect(message).Should(MatchRegexp(fmt.Sprintf("%s vip 1.2.3.[56] is valid", strings.ToLower(lcontext.name)))) + }) + + It("user-managed load balancer - verification succeeded already", func() { + preprocessContext.cluster = &common.Cluster{Cluster: models.Cluster{ + ID: &clusterID, + MachineNetworks: common.TestDualStackNetworking.MachineNetworks, + APIVips: []*models.APIVip{{ClusterID: clusterID, IP: "1.2.3.5", Verification: models.NewVipVerification(models.VipVerificationSucceeded)}}, + IngressVips: []*models.IngressVip{{ClusterID: clusterID, IP: "1.2.3.6", Verification: models.NewVipVerification(models.VipVerificationSucceeded)}}, + Hosts: hosts, + LoadBalancer: &models.LoadBalancer{Type: models.LoadBalancerTypeUserManaged}, + }} + preprocessContext.hasHostsWithInventories = true + + status, message := lcontext.function(preprocessContext) + Expect(status).Should(Equal(ValidationSuccess)) + Expect(message).Should(MatchRegexp(fmt.Sprintf("%s vip 1.2.3.[56] is valid", strings.ToLower(lcontext.name)))) + }) + + It("user-managed load balancer - verification failed already", func() { + preprocessContext.cluster = &common.Cluster{Cluster: models.Cluster{ + ID: &clusterID, + MachineNetworks: common.TestDualStackNetworking.MachineNetworks, + APIVips: []*models.APIVip{{ClusterID: clusterID, IP: "1.2.3.5", Verification: models.NewVipVerification(models.VipVerificationFailed)}}, + IngressVips: []*models.IngressVip{{ClusterID: clusterID, IP: "1.2.3.6", Verification: models.NewVipVerification(models.VipVerificationFailed)}}, + Hosts: hosts, + LoadBalancer: &models.LoadBalancer{Type: models.LoadBalancerTypeUserManaged}, + }} + preprocessContext.hasHostsWithInventories = true + + status, message := lcontext.function(preprocessContext) + Expect(status).Should(Equal(ValidationSuccess)) + Expect(message).Should(MatchRegexp(fmt.Sprintf("%s vip 1.2.3.[56] is valid", strings.ToLower(lcontext.name)))) + }) + + It("ipv6 vips verified", func() { preprocessContext.cluster = &common.Cluster{Cluster: models.Cluster{ ID: &clusterID, MachineNetworks: common.TestDualStackNetworking.MachineNetworks, @@ -395,6 +611,7 @@ var _ = Describe("areVipsValid", func() { Expect(status).Should(Equal(ValidationFailure)) Expect(message).Should(MatchRegexp(fmt.Sprintf("%s vips <1001:db8::6[45]> are not verified yet", strings.ToLower(lcontext.name)))) }) + It("all successful", func() { preprocessContext.cluster = &common.Cluster{Cluster: models.Cluster{ ID: &clusterID, @@ -409,6 +626,7 @@ var _ = Describe("areVipsValid", func() { Expect(status).Should(Equal(ValidationSuccess)) Expect(message).Should(MatchRegexp(fmt.Sprintf("%s vips 1.2.3.[56], 1001:db8::6[45] belongs to the Machine CIDR and is not in use", strings.ToLower(lcontext.name)))) }) + It("ipv4 verification failed", func() { preprocessContext.cluster = &common.Cluster{Cluster: models.Cluster{ ID: &clusterID, @@ -893,3 +1111,31 @@ var _ = Describe("SufficientMastersCount", func() { }) }) }) + +var _ = Describe("isMachineCidrEqualsToCalculatedCidr", func() { + var ( + validator clusterValidator + clusterID strfmt.UUID + ) + + BeforeEach(func() { + clusterID = strfmt.UUID(uuid.New().String()) + validator = clusterValidator{log: logrus.New()} + }) + + It("should pass validation with user-managed load balancer cluster", func() { + preprocessContext := &clusterPreprocessContext{ + clusterId: clusterID, + cluster: &common.Cluster{ + Cluster: models.Cluster{ + ID: &clusterID, + LoadBalancer: &models.LoadBalancer{Type: models.LoadBalancerTypeUserManaged}, + }, + }, + } + + status, msg := validator.isMachineCidrEqualsToCalculatedCidr(preprocessContext) + Expect(status).To(Equal(ValidationSuccess)) + Expect(msg).To(Equal("Calculating machine network CIDR is not enabled: User Managed Load Balancer")) + }) +}) diff --git a/internal/featuresupport/features_networking.go b/internal/featuresupport/features_networking.go index 6b6bb43e809..0940e116fb2 100644 --- a/internal/featuresupport/features_networking.go +++ b/internal/featuresupport/features_networking.go @@ -173,12 +173,15 @@ func (feature *DualStackFeature) getFeatureActiveLevel(cluster *common.Cluster, } func (feature *DualStackFeature) getIncompatibleFeatures(openshiftVersion string) *[]models.FeatureSupportLevelID { + unsupportedFeatures := []models.FeatureSupportLevelID{ + models.FeatureSupportLevelIDUSERMANAGEDLOADBALANCER, + } + if isNotSupported, err := common.BaseVersionLessThan("4.13", openshiftVersion); isNotSupported || err != nil { - return &[]models.FeatureSupportLevelID{ - models.FeatureSupportLevelIDVSPHEREINTEGRATION, - } + unsupportedFeatures = append(unsupportedFeatures, models.FeatureSupportLevelIDVSPHEREINTEGRATION) } - return nil + + return &unsupportedFeatures } func (feature *DualStackFeature) getIncompatibleArchitectures(_ *string) *[]models.ArchitectureSupportLevelID { @@ -231,6 +234,7 @@ func (feature *DualStackVipsFeature) getFeatureActiveLevel(cluster *common.Clust func (feature *DualStackVipsFeature) getIncompatibleFeatures(string) *[]models.FeatureSupportLevelID { return &[]models.FeatureSupportLevelID{ models.FeatureSupportLevelIDEXTERNALPLATFORMOCI, + models.FeatureSupportLevelIDUSERMANAGEDLOADBALANCER, } } @@ -501,6 +505,8 @@ func (feature *UserManagedLoadBalancerFeature) getIncompatibleFeatures(string) * models.FeatureSupportLevelIDUSERMANAGEDNETWORKING, models.FeatureSupportLevelIDVIPAUTOALLOC, models.FeatureSupportLevelIDVSPHEREINTEGRATION, + models.FeatureSupportLevelIDDUALSTACK, + models.FeatureSupportLevelIDDUALSTACKVIPS, } } diff --git a/internal/host/validations_test.go b/internal/host/validations_test.go index 8077f0729d0..45b83030620 100644 --- a/internal/host/validations_test.go +++ b/internal/host/validations_test.go @@ -30,6 +30,7 @@ import ( "github.com/openshift/assisted-service/internal/versions" "github.com/openshift/assisted-service/models" "github.com/openshift/assisted-service/pkg/conversions" + "github.com/openshift/assisted-service/pkg/s3wrapper" "github.com/samber/lo" "github.com/vincent-petithory/dataurl" "gorm.io/gorm" @@ -2672,4 +2673,673 @@ var _ = Describe("Validations test", func() { Entry("Day 2 cluster - the validation should be skipped", []*models.MtuReport{}, ValidationSuccess, false, true), ) }) + + Context("belongsToL2MajorityGroup", func() { + var ( + ctx = context.Background() + kubeApiEnabled = false + softTimeoutsEnabled = false + infraenv *common.InfraEnv = nil + s3wrapper s3wrapper.API = nil + validator = &validator{log: common.GetTestLog()} + inventoryCache = InventoryCache{} + ) + + BeforeEach(func() { + mockHwValidator.EXPECT().GetInfraEnvHostRequirements(gomock.Any(), gomock.Any()).Return(&models.ClusterHostRequirements{}, nil).AnyTimes() + mockHwValidator.EXPECT().GetPreflightInfraEnvHardwareRequirements(gomock.Any(), gomock.Any()).Return(&models.PreflightHardwareRequirements{}, nil).AnyTimes() + mockHwValidator.EXPECT().GetClusterHostRequirements(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return(&models.ClusterHostRequirements{ + Total: &models.ClusterHostRequirementsDetails{}, + }, nil) + mockHwValidator.EXPECT().GetPreflightHardwareRequirements(gomock.Any(), gomock.Any()).AnyTimes().Return(&models.PreflightHardwareRequirements{ + Ocp: &models.HostTypeHardwareRequirementsWrapper{ + Worker: &models.HostTypeHardwareRequirements{ + Quantitative: &models.ClusterHostRequirementsDetails{}, + }, + Master: &models.HostTypeHardwareRequirements{ + Quantitative: &models.ClusterHostRequirementsDetails{}, + }, + }, + }, nil) + }) + + Context("with cluster-managed load balancer", func() { + It("should return validation success when all machine networks exist in the majority group", func() { + machineNetworks := []*models.MachineNetwork{ + { + ClusterID: clusterID, + Cidr: "192.168.127.0/24", + }, + } + host := &models.Host{ + ID: &hostID, + ClusterID: &clusterID, + Role: models.HostRoleMaster, + } + cluster := &common.Cluster{ + Cluster: models.Cluster{ + ID: &clusterID, + MachineNetworks: machineNetworks, + LoadBalancer: &models.LoadBalancer{Type: models.LoadBalancerTypeClusterManaged}, + }, + } + majorityGroups := map[string][]strfmt.UUID{ + "192.168.127.0/24": {*host.ID}, + } + + vc, err := newValidationContext( + ctx, + host, + cluster, + infraenv, + db, + inventoryCache, + mockHwValidator, + kubeApiEnabled, + s3wrapper, + softTimeoutsEnabled, + ) + Expect(err).ToNot(HaveOccurred()) + + status := validator.belongsToL2MajorityGroup(vc, majorityGroups) + Expect(status).To(Equal(ValidationSuccess)) + }) + + It("should return validation failure when machine network is not in the majority groups", func() { + machineNetworks := []*models.MachineNetwork{ + { + ClusterID: clusterID, + Cidr: "192.168.127.0/24", + }, + } + host := &models.Host{ + ID: &hostID, + ClusterID: &clusterID, + Role: models.HostRoleMaster, + } + cluster := &common.Cluster{ + Cluster: models.Cluster{ + ID: &clusterID, + MachineNetworks: machineNetworks, + LoadBalancer: &models.LoadBalancer{Type: models.LoadBalancerTypeClusterManaged}, + }, + } + majorityGroups := map[string][]strfmt.UUID{ + "192.168.128.0/24": {*host.ID}, + } + + vc, err := newValidationContext( + ctx, + host, + cluster, + infraenv, + db, + inventoryCache, + mockHwValidator, + kubeApiEnabled, + s3wrapper, + softTimeoutsEnabled, + ) + Expect(err).ToNot(HaveOccurred()) + + status := validator.belongsToL2MajorityGroup(vc, majorityGroups) + Expect(status).To(Equal(ValidationFailure)) + }) + + It("should return validation failure when one of the machine networs is not in the majority groups", func() { + machineNetworks := []*models.MachineNetwork{ + { + ClusterID: clusterID, + Cidr: "192.168.127.0/24", + }, + { + ClusterID: clusterID, + Cidr: "192.168.128.0/24", + }, + } + host := &models.Host{ + ID: &hostID, + ClusterID: &clusterID, + Role: models.HostRoleMaster, + } + cluster := &common.Cluster{ + Cluster: models.Cluster{ + ID: &clusterID, + MachineNetworks: machineNetworks, + LoadBalancer: &models.LoadBalancer{Type: models.LoadBalancerTypeClusterManaged}, + }, + } + majorityGroups := map[string][]strfmt.UUID{ + "192.168.127.0/24": {*host.ID}, + } + + vc, err := newValidationContext( + ctx, + host, + cluster, + infraenv, + db, + inventoryCache, + mockHwValidator, + kubeApiEnabled, + s3wrapper, + softTimeoutsEnabled, + ) + Expect(err).ToNot(HaveOccurred()) + + status := validator.belongsToL2MajorityGroup(vc, majorityGroups) + Expect(status).To(Equal(ValidationFailure)) + }) + }) + + Context("with user-managed load balancer", func() { + It("should return validation success when all machine networks exist in the majority group", func() { + machineNetworks := []*models.MachineNetwork{ + { + ClusterID: clusterID, + Cidr: "192.168.127.0/24", + }, + } + host := &models.Host{ + ID: &hostID, + ClusterID: &clusterID, + Role: models.HostRoleMaster, + } + cluster := &common.Cluster{ + Cluster: models.Cluster{ + ID: &clusterID, + MachineNetworks: machineNetworks, + LoadBalancer: &models.LoadBalancer{Type: models.LoadBalancerTypeUserManaged}, + }, + } + majorityGroups := map[string][]strfmt.UUID{ + "192.168.127.0/24": {*host.ID}, + } + + vc, err := newValidationContext( + ctx, + host, + cluster, + infraenv, + db, + inventoryCache, + mockHwValidator, + kubeApiEnabled, + s3wrapper, + softTimeoutsEnabled, + ) + Expect(err).ToNot(HaveOccurred()) + + status := validator.belongsToL2MajorityGroup(vc, majorityGroups) + Expect(status).To(Equal(ValidationSuccess)) + }) + + It("should return validation failure when machine network is not in the majority groups", func() { + machineNetworks := []*models.MachineNetwork{ + { + ClusterID: clusterID, + Cidr: "192.168.127.0/24", + }, + } + host := &models.Host{ + ID: &hostID, + ClusterID: &clusterID, + Role: models.HostRoleMaster, + } + cluster := &common.Cluster{ + Cluster: models.Cluster{ + ID: &clusterID, + MachineNetworks: machineNetworks, + LoadBalancer: &models.LoadBalancer{Type: models.LoadBalancerTypeUserManaged}, + }, + } + majorityGroups := map[string][]strfmt.UUID{ + "192.168.128.0/24": {*host.ID}, + } + + vc, err := newValidationContext( + ctx, + host, + cluster, + infraenv, + db, + inventoryCache, + mockHwValidator, + kubeApiEnabled, + s3wrapper, + softTimeoutsEnabled, + ) + Expect(err).ToNot(HaveOccurred()) + + status := validator.belongsToL2MajorityGroup(vc, majorityGroups) + Expect(status).To(Equal(ValidationFailure)) + }) + + It("should return validation success when one of the machine networs is not in the majority groups", func() { + machineNetworks := []*models.MachineNetwork{ + { + ClusterID: clusterID, + Cidr: "192.168.127.0/24", + }, + { + ClusterID: clusterID, + Cidr: "192.168.128.0/24", + }, + } + host := &models.Host{ + ID: &hostID, + ClusterID: &clusterID, + Role: models.HostRoleMaster, + } + cluster := &common.Cluster{ + Cluster: models.Cluster{ + ID: &clusterID, + MachineNetworks: machineNetworks, + LoadBalancer: &models.LoadBalancer{Type: models.LoadBalancerTypeUserManaged}, + }, + } + majorityGroups := map[string][]strfmt.UUID{ + "192.168.127.0/24": {*host.ID}, + } + + vc, err := newValidationContext( + ctx, + host, + cluster, + infraenv, + db, + inventoryCache, + mockHwValidator, + kubeApiEnabled, + s3wrapper, + softTimeoutsEnabled, + ) + Expect(err).ToNot(HaveOccurred()) + + status := validator.belongsToL2MajorityGroup(vc, majorityGroups) + Expect(status).To(Equal(ValidationSuccess)) + }) + }) + }) + + Context("belongsToMachineCidr", func() { + var ( + ctx = context.Background() + kubeApiEnabled = false + softTimeoutsEnabled = false + infraenv *common.InfraEnv = nil + s3wrapper s3wrapper.API = nil + validator = &validator{log: common.GetTestLog()} + inventoryCache = InventoryCache{} + ) + + BeforeEach(func() { + mockHwValidator.EXPECT().GetInfraEnvHostRequirements(gomock.Any(), gomock.Any()).Return(&models.ClusterHostRequirements{}, nil).AnyTimes() + mockHwValidator.EXPECT().GetPreflightInfraEnvHardwareRequirements(gomock.Any(), gomock.Any()).Return(&models.PreflightHardwareRequirements{}, nil).AnyTimes() + mockHwValidator.EXPECT().GetClusterHostRequirements(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return(&models.ClusterHostRequirements{ + Total: &models.ClusterHostRequirementsDetails{}, + }, nil) + mockHwValidator.EXPECT().GetPreflightHardwareRequirements(gomock.Any(), gomock.Any()).AnyTimes().Return(&models.PreflightHardwareRequirements{ + Ocp: &models.HostTypeHardwareRequirementsWrapper{ + Worker: &models.HostTypeHardwareRequirements{ + Quantitative: &models.ClusterHostRequirementsDetails{}, + }, + Master: &models.HostTypeHardwareRequirements{ + Quantitative: &models.ClusterHostRequirementsDetails{}, + }, + }, + }, nil) + }) + + It("with user-managed networking, SNO, host is not bootstrap, there are machine networks", func() { + host := &models.Host{ + ID: &hostID, + ClusterID: &clusterID, + Role: models.HostRoleMaster, + } + + cluster := &common.Cluster{ + Cluster: models.Cluster{ + ID: &clusterID, + Hosts: []*models.Host{ + host, + }, + UserManagedNetworking: swag.Bool(true), + MachineNetworks: []*models.MachineNetwork{ + {ClusterID: clusterID, Cidr: "192.168.127.0/24"}, + }, + }, + } + + validationContext, err := newValidationContext( + ctx, + host, + cluster, + infraenv, + db, + inventoryCache, + mockHwValidator, + kubeApiEnabled, + s3wrapper, + softTimeoutsEnabled, + ) + Expect(err).ToNot(HaveOccurred()) + + status, msg := validator.belongsToMachineCidr(validationContext) + Expect(status).To(Equal(ValidationSuccess)) + Expect(msg).To(Equal("No machine network CIDR validation needed: User Managed Networking")) + }) + + It("with user-managed networking, SNO, host is bootstrap, no machine networks", func() { + host := &models.Host{ + ID: &hostID, + ClusterID: &clusterID, + Role: models.HostRoleMaster, + Bootstrap: true, + } + + cluster := &common.Cluster{ + Cluster: models.Cluster{ + ID: &clusterID, + Hosts: []*models.Host{ + host, + }, + UserManagedNetworking: swag.Bool(true), + }, + } + + validationContext, err := newValidationContext( + ctx, + host, + cluster, + infraenv, + db, + inventoryCache, + mockHwValidator, + kubeApiEnabled, + s3wrapper, + softTimeoutsEnabled, + ) + Expect(err).ToNot(HaveOccurred()) + + status, msg := validator.belongsToMachineCidr(validationContext) + Expect(status).To(Equal(ValidationSuccess)) + Expect(msg).To(Equal("No machine network CIDR validation needed: User Managed Networking")) + }) + + It("with day2 cluster", func() { + host := &models.Host{ + ID: &hostID, + ClusterID: &clusterID, + Role: models.HostRoleMaster, + } + + cluster := &common.Cluster{ + Cluster: models.Cluster{ + ID: &clusterID, + Hosts: []*models.Host{ + host, + }, + Kind: swag.String(models.ClusterKindAddHostsCluster), + }, + } + + validationContext, err := newValidationContext( + ctx, + host, + cluster, + infraenv, + db, + inventoryCache, + mockHwValidator, + kubeApiEnabled, + s3wrapper, + softTimeoutsEnabled, + ) + Expect(err).ToNot(HaveOccurred()) + + status, msg := validator.belongsToMachineCidr(validationContext) + Expect(status).To(Equal(ValidationSuccess)) + Expect(msg).To(Equal("No machine network CIDR validation needed: Day2 cluster")) + }) + + It("with host misses inventory", func() { + host := &models.Host{ + ID: &hostID, + ClusterID: &clusterID, + Role: models.HostRoleMaster, + } + + cluster := &common.Cluster{ + Cluster: models.Cluster{ + ID: &clusterID, + Hosts: []*models.Host{ + host, + }, + }, + } + + validationContext, err := newValidationContext( + ctx, + host, + cluster, + infraenv, + db, + inventoryCache, + mockHwValidator, + kubeApiEnabled, + s3wrapper, + softTimeoutsEnabled, + ) + Expect(err).ToNot(HaveOccurred()) + + status, msg := validator.belongsToMachineCidr(validationContext) + Expect(status).To(Equal(ValidationPending)) + Expect(msg).To(Equal("Missing inventory or machine network CIDR")) + }) + + It("with host misses machine network", func() { + host := &models.Host{ + ID: &hostID, + ClusterID: &clusterID, + Role: models.HostRoleMaster, + Inventory: common.GenerateTestInventory(), + } + + cluster := &common.Cluster{ + Cluster: models.Cluster{ + ID: &clusterID, + Hosts: []*models.Host{ + host, + }, + }, + } + + validationContext, err := newValidationContext( + ctx, + host, + cluster, + infraenv, + db, + inventoryCache, + mockHwValidator, + kubeApiEnabled, + s3wrapper, + softTimeoutsEnabled, + ) + Expect(err).ToNot(HaveOccurred()) + + status, msg := validator.belongsToMachineCidr(validationContext) + Expect(status).To(Equal(ValidationPending)) + Expect(msg).To(Equal("Missing inventory or machine network CIDR")) + }) + + It("with cluster-managed load balancer, host belongs to all machine networks", func() { + host := &models.Host{ + ID: &hostID, + ClusterID: &clusterID, + Role: models.HostRoleMaster, + Inventory: common.GenerateTestInventoryWithNetwork(common.NetAddress{ + IPv4Address: []string{"1.2.3.5/24"}, + }), + } + + cluster := &common.Cluster{ + Cluster: models.Cluster{ + ID: &clusterID, + Hosts: []*models.Host{ + host, + }, + MachineNetworks: []*models.MachineNetwork{ + {ClusterID: clusterID, Cidr: "1.2.3.0/24"}, + }, + }, + } + + validationContext, err := newValidationContext( + ctx, + host, + cluster, + infraenv, + db, + inventoryCache, + mockHwValidator, + kubeApiEnabled, + s3wrapper, + softTimeoutsEnabled, + ) + Expect(err).ToNot(HaveOccurred()) + + status, msg := validator.belongsToMachineCidr(validationContext) + Expect(status).To(Equal(ValidationSuccess)) + Expect(msg).To(Equal("Host belongs to all machine network CIDRs")) + }) + + It("with cluster-managed load balancer, host doesn't belongs to all machine networks", func() { + host := &models.Host{ + ID: &hostID, + ClusterID: &clusterID, + Role: models.HostRoleMaster, + Inventory: common.GenerateTestInventoryWithNetwork(common.NetAddress{ + IPv4Address: []string{"1.2.3.5/24"}, + }), + } + + cluster := &common.Cluster{ + Cluster: models.Cluster{ + ID: &clusterID, + Hosts: []*models.Host{ + host, + }, + MachineNetworks: []*models.MachineNetwork{ + {ClusterID: clusterID, Cidr: "1.2.4.0/24"}, + }, + }, + } + + validationContext, err := newValidationContext( + ctx, + host, + cluster, + infraenv, + db, + inventoryCache, + mockHwValidator, + kubeApiEnabled, + s3wrapper, + softTimeoutsEnabled, + ) + Expect(err).ToNot(HaveOccurred()) + + status, msg := validator.belongsToMachineCidr(validationContext) + Expect(status).To(Equal(ValidationFailure)) + Expect(msg).To(Equal("Host does not belong to machine network CIDRs. Verify that the host belongs to every CIDR listed under machine networks")) + }) + + It("with user-managed load balancer, host belongs to some of the machine networks", func() { + host := &models.Host{ + ID: &hostID, + ClusterID: &clusterID, + Role: models.HostRoleMaster, + Inventory: common.GenerateTestInventoryWithNetwork(common.NetAddress{ + IPv4Address: []string{"1.2.4.5/24"}, + }), + } + + cluster := &common.Cluster{ + Cluster: models.Cluster{ + ID: &clusterID, + Hosts: []*models.Host{ + host, + }, + MachineNetworks: []*models.MachineNetwork{ + {ClusterID: clusterID, Cidr: "1.2.3.0/24"}, + {ClusterID: clusterID, Cidr: "1.2.4.0/24"}, + }, + LoadBalancer: &models.LoadBalancer{Type: models.LoadBalancerTypeUserManaged}, + }, + } + + validationContext, err := newValidationContext( + ctx, + host, + cluster, + infraenv, + db, + inventoryCache, + mockHwValidator, + kubeApiEnabled, + s3wrapper, + softTimeoutsEnabled, + ) + Expect(err).ToNot(HaveOccurred()) + + status, msg := validator.belongsToMachineCidr(validationContext) + Expect(status).To(Equal(ValidationSuccess)) + Expect(msg).To(Equal("Host belongs to at least one machine network CIDR")) + }) + + It("with user-managed load balancer, host doesn't belongs to any machine network", func() { + host := &models.Host{ + ID: &hostID, + ClusterID: &clusterID, + Role: models.HostRoleMaster, + Inventory: common.GenerateTestInventoryWithNetwork(common.NetAddress{ + IPv4Address: []string{"1.2.5.5/24"}, + }), + } + + cluster := &common.Cluster{ + Cluster: models.Cluster{ + ID: &clusterID, + Hosts: []*models.Host{ + host, + }, + MachineNetworks: []*models.MachineNetwork{ + {ClusterID: clusterID, Cidr: "1.2.3.0/24"}, + {ClusterID: clusterID, Cidr: "1.2.4.0/24"}, + }, + LoadBalancer: &models.LoadBalancer{Type: models.LoadBalancerTypeUserManaged}, + }, + } + + validationContext, err := newValidationContext( + ctx, + host, + cluster, + infraenv, + db, + inventoryCache, + mockHwValidator, + kubeApiEnabled, + s3wrapper, + softTimeoutsEnabled, + ) + Expect(err).ToNot(HaveOccurred()) + + status, msg := validator.belongsToMachineCidr(validationContext) + Expect(status).To(Equal(ValidationFailure)) + Expect(msg).To(Equal("Host does not belong to any machine network CIDR. Verify that the host belongs to at least one CIDR listed under machine networks")) + }) + }) }) diff --git a/internal/host/validator.go b/internal/host/validator.go index b8ca45adf6b..53f04ea0494 100644 --- a/internal/host/validator.go +++ b/internal/host/validator.go @@ -666,13 +666,25 @@ func (v *validator) belongsToMachineCidr(c *validationContext) (ValidationStatus if swag.StringValue(c.cluster.Kind) == models.ClusterKindAddHostsCluster { return ValidationSuccess, "No machine network CIDR validation needed: Day2 cluster" } + if c.inventory == nil || !network.IsMachineCidrAvailable(c.cluster) { return ValidationPending, "Missing inventory or machine network CIDR" } - if !network.IsHostInPrimaryMachineNetCidr(v.log, c.cluster, c.host) { - return ValidationFailure, "Host does not belong to machine network CIDRs. Verify that the host belongs to every CIDR listed under machine networks" + + if !network.IsLoadBalancerUserManaged(c.cluster) { + if !network.IsHostInAllMachineNetworksCidr(v.log, c.cluster, c.host) { + return ValidationFailure, "Host does not belong to machine network CIDRs. Verify that the host belongs to every CIDR listed under machine networks" + } + + return ValidationSuccess, "Host belongs to all machine network CIDRs" } - return ValidationSuccess, "Host belongs to all machine network CIDRs" + + // user-managed load balancer + if !network.IsHostInAtLeastOneMachineNetworkCidr(v.log, c.cluster, c.host) { + return ValidationFailure, "Host does not belong to any machine network CIDR. Verify that the host belongs to at least one CIDR listed under machine networks" + } + + return ValidationSuccess, "Host belongs to at least one machine network CIDR" } func getRealHostname(host *models.Host, inventory *models.Inventory) string { @@ -739,8 +751,6 @@ func (v *validator) belongsToL2MajorityGroup(c *validationContext, majorityGroup return ValidationFailure } - // TODO(mko) This rule should be revised as soon as OCP supports multiple machineNetwork - // entries using the same IP stack. areNetworksEqual := func(ipnet1, ipnet2 *net.IPNet) bool { return ipnet1.IP.Equal(ipnet2.IP) && bytes.Equal(ipnet1.Mask, ipnet2.Mask) } @@ -761,16 +771,31 @@ func (v *validator) belongsToL2MajorityGroup(c *validationContext, majorityGroup return nil } + // For cluster-managed load balancer, each machine network should be a majority group. + // For user-managed load balancer, it is sufficient to have at least one machine network as majority group. for _, machineNet := range c.cluster.MachineNetworks { _, machineIpnet, err := net.ParseCIDR(string(machineNet.Cidr)) if err != nil { return ValidationError } if !funk.Contains(groupForNetwork(machineIpnet), *c.host.ID) { + if network.IsLoadBalancerUserManaged(c.cluster) { + continue + } return ValidationFailure + } else { + if network.IsLoadBalancerUserManaged(c.cluster) { + return ValidationSuccess + } } } + // We haven't found any machine network which is a majority group + if network.IsLoadBalancerUserManaged(c.cluster) { + return ValidationFailure + } + + // all machine networks are a majority group return ValidationSuccess } diff --git a/internal/network/cidr_validations.go b/internal/network/cidr_validations.go index 3618023ccb4..05532ca9022 100644 --- a/internal/network/cidr_validations.go +++ b/internal/network/cidr_validations.go @@ -14,6 +14,9 @@ const MinMaskDelta = 7 // Minimum mask size for Machine CIDR to allow at least 16 addresses const MinMachineMaskDelta = 4 +// Minimum mask size for Machine CIDR to allow at least 4 addresses +const MinUserManagedLoadBalancerMachineMaskDelta = 2 + // Minimum mask size for Machine CIDR to allow at least 2 addresses const MinSNOMachineMaskDelta = 1 @@ -78,10 +81,12 @@ func VerifyClusterOrServiceCIDR(cidrStr string) error { return verifySubnetCIDR(cidrStr, MinMaskDelta) } -func VerifyMachineCIDR(cidrStr string, isSNO bool) error { +func VerifyMachineCIDR(cidrStr string, isSNO bool, isUserManagedLoadBalancer bool) error { maskDelta := MinMachineMaskDelta if isSNO { maskDelta = MinSNOMachineMaskDelta + } else if isUserManagedLoadBalancer { + maskDelta = MinUserManagedLoadBalancerMachineMaskDelta } return verifySubnetCIDR(cidrStr, maskDelta) } diff --git a/internal/network/cidr_validations_test.go b/internal/network/cidr_validations_test.go index b6d8d570111..778e008b29a 100644 --- a/internal/network/cidr_validations_test.go +++ b/internal/network/cidr_validations_test.go @@ -34,24 +34,32 @@ var _ = Describe("CIDR validations", func() { }) Context("Verify CIDRs", func() { It("Machine CIDR 24 OK", func() { - Expect(VerifyMachineCIDR("1.2.3.0/24", false)).ToNot(HaveOccurred()) + Expect(VerifyMachineCIDR("1.2.3.0/24", false, false)).ToNot(HaveOccurred()) }) It("Machine CIDR 26 OK", func() { - Expect(VerifyMachineCIDR("1.2.3.128/26", false)).ToNot(HaveOccurred()) + Expect(VerifyMachineCIDR("1.2.3.128/26", false, false)).ToNot(HaveOccurred()) }) It("Machine CIDR 29 Fail", func() { - Expect(VerifyMachineCIDR("1.2.3.128/29", false)).To(HaveOccurred()) + Expect(VerifyMachineCIDR("1.2.3.128/29", false, false)).To(HaveOccurred()) }) It("Machine CIDR 27 OK", func() { - Expect(VerifyMachineCIDR("1.2.3.128/27", false)).ToNot(HaveOccurred()) + Expect(VerifyMachineCIDR("1.2.3.128/27", false, false)).ToNot(HaveOccurred()) }) It("Machine CIDR 31 Ok for SNO", func() { - Expect(VerifyMachineCIDR("1.2.3.128/31", true)).ToNot(HaveOccurred()) + Expect(VerifyMachineCIDR("1.2.3.128/31", true, false)).ToNot(HaveOccurred()) }) It("Machine CIDR 32 Fail for SNO", func() { - Expect(VerifyMachineCIDR("1.2.3.128/32", true)).To(HaveOccurred()) + Expect(VerifyMachineCIDR("1.2.3.128/32", true, false)).To(HaveOccurred()) + }) + + It("Machine CIDR 30 Ok for user managed load balancer", func() { + Expect(VerifyMachineCIDR("1.2.3.128/30", false, true)).ToNot(HaveOccurred()) + }) + + It("Machine CIDR 31 Fail for user managed load balancer", func() { + Expect(VerifyMachineCIDR("1.2.3.128/31", false, true)).To(HaveOccurred()) }) It("Service CIDR 26 Fail", func() { diff --git a/internal/network/machine_network_cidr.go b/internal/network/machine_network_cidr.go index 7edc03c3522..864f4dd2397 100644 --- a/internal/network/machine_network_cidr.go +++ b/internal/network/machine_network_cidr.go @@ -116,7 +116,7 @@ func VerifyVipFree(hosts []*models.Host, vip string, machineNetworkCidr string, return IpInFreeList(hosts, vip, machineNetworkCidr, log) } -func VerifyVip( +func VerifyVipWithSingleMachineNetwork( hosts []*models.Host, machineNetworkCidr string, vip string, @@ -157,6 +157,64 @@ func VerifyVip( return ret, errors.New(msg) } +func VerifyVipWithMachineNetworkList( + hosts []*models.Host, + machineNetworks []*models.MachineNetwork, + vip string, + vipName string, + verification *models.VipVerification, + verifyVipFree bool, + log logrus.FieldLogger, +) (models.VipVerification, error) { + var ( + resultVerification models.VipVerification = models.VipVerificationFailed + accumulatedError *multierror.Error + ) + + if len(machineNetworks) == 0 { + return models.VipVerificationFailed, fmt.Errorf("no machine networks to verify VIP: %s with", vip) + } + + for _, machineNetwork := range machineNetworks { + if machineNetwork == nil { + continue + } + + currentVerfication, err := VerifyVipWithSingleMachineNetwork( + hosts, + string(machineNetwork.Cidr), + vip, + vipName, + verification, + verifyVipFree, + log, + ) + if err != nil { + accumulatedError = multierror.Append(accumulatedError, + fmt.Errorf("machine network CIDR <%s> verification failed: %s", + string(machineNetwork.Cidr), + err.Error()), + ) + } + + if resultVerification == models.VipVerificationFailed && currentVerfication == models.VipVerificationUnverified { + resultVerification = currentVerfication + } + + if currentVerfication == models.VipVerificationSucceeded { + return currentVerfication, nil + } + } + + var err error + if accumulatedError != nil { + err = errors.New(strings.Join(lo.Map(accumulatedError.Errors, func(e error, _ int) string { return e.Error() }), "; ")) + } + err = errors.Wrapf(err, "%s '%s' failed verification: none of the machine networks is satisfying the requirements, description", vipName, vip) + + return resultVerification, err +} + func ValidateNoVIPAddressesDuplicates(apiVips []*models.APIVip, ingressVips []*models.IngressVip, allowCommonVIP bool) error { var ( err error @@ -204,32 +262,52 @@ func ValidateNoVIPAddressesDuplicates(apiVips []*models.APIVip, ingressVips []*m return nil } -// This function is called from places which assume it is OK for a VIP to be unverified. +// VerifyVipsForClusterManangedLoadBalancer is called from places which assume it is OK for a VIP to be unverified. // The assumption is that VIPs are eventually verified by cluster validation // (i.e api-vips-valid, ingress-vips-valid) -func VerifyVips( +func VerifyVipsForClusterManagedLoadBalancer( hosts []*models.Host, machineNetworkCidr string, apiVip string, ingressVip string, - userManagedLoadBalancer bool, log logrus.FieldLogger, ) error { - // When using user managed load balancer, the VIPs are the load balancer IP (not virtual), - // therefore will not be free. In this case it is also allowed to have one IP for both API and Ingress. - shouldVerifyVIPIsFree := !userManagedLoadBalancer - allowCommonVIP := userManagedLoadBalancer + verification, err := VerifyVipWithSingleMachineNetwork(hosts, machineNetworkCidr, apiVip, "api-vip", nil, true, log) + // Error is ignored if the verification didn't fail + if verification != models.VipVerificationFailed { + verification, err = VerifyVipWithSingleMachineNetwork(hosts, machineNetworkCidr, ingressVip, "ingress-vip", nil, true, log) + } + if verification != models.VipVerificationFailed { + return ValidateNoVIPAddressesDuplicates( + []*models.APIVip{{IP: models.IP(apiVip)}}, + []*models.IngressVip{{IP: models.IP(ingressVip)}}, + false, + ) + } + return err +} - verification, err := VerifyVip(hosts, machineNetworkCidr, apiVip, "api-vip", nil, shouldVerifyVIPIsFree, log) +// VerifyVipsForUserManangedLoadBalancer is called from places which assume it is OK for a VIP to be unverified. +// The assumption is that VIPs are eventually verified by cluster validation +// (i.e api-vips-valid, ingress-vips-valid) +func VerifyVipsForUserManangedLoadBalancer( + hosts []*models.Host, + machineNetworks []*models.MachineNetwork, + apiVip string, + ingressVip string, + log logrus.FieldLogger, +) error { + verification, err := VerifyVipWithMachineNetworkList(hosts, machineNetworks, apiVip, "api-vip", nil, false, log) // Error is ignored if the verification didn't fail + fmt.Println(verification, err) if verification != models.VipVerificationFailed { - verification, err = VerifyVip(hosts, machineNetworkCidr, ingressVip, "ingress-vip", nil, shouldVerifyVIPIsFree, log) + verification, err = VerifyVipWithMachineNetworkList(hosts, machineNetworks, ingressVip, "ingress-vip", nil, false, log) } if verification != models.VipVerificationFailed { return ValidateNoVIPAddressesDuplicates( []*models.APIVip{{IP: models.IP(apiVip)}}, []*models.IngressVip{{IP: models.IP(ingressVip)}}, - allowCommonVIP, + true, ) } return err @@ -560,10 +638,8 @@ func parseMachineNetworks(machineNetworks []*models.MachineNetwork) ([]*net.IPNe return parsedCidr, nil } -// Check if a host belongs to all the networks specified as Machine Networks. -func IsHostInPrimaryMachineNetCidr(log logrus.FieldLogger, cluster *common.Cluster, host *models.Host) bool { - // TODO(mko) This rule should be revised as soon as OCP supports multiple machineNetwork - // entries using the same IP stack. +// IsHostInAllMachineNetworksCidr Check if a host belongs to all the networks specified as Machine Networks. +func IsHostInAllMachineNetworksCidr(log logrus.FieldLogger, cluster *common.Cluster, host *models.Host) bool { return forEachMachineNetwork(log, cluster, func(agg bool, machineIpnet *net.IPNet, index int) bool { // initialize agg to true the first time this function is called if index == 0 { @@ -574,6 +650,17 @@ func IsHostInPrimaryMachineNetCidr(log logrus.FieldLogger, cluster *common.Clust }) } +func IsHostInAtLeastOneMachineNetworkCidr(log logrus.FieldLogger, cluster *common.Cluster, host *models.Host) bool { + return forEachMachineNetwork(log, cluster, func(agg bool, machineIpnet *net.IPNet, index int) bool { + // initialize agg to false the first time this function is called + if index == 0 { + agg = false + } + + return agg || belongsToNetwork(log, host, machineIpnet) + }) +} + // Check if an interface is part of one of the networks specified as Machine Networks func IsInterfaceInPrimaryMachineNetCidr(log logrus.FieldLogger, cluster *common.Cluster, nic *models.Interface) bool { return forEachMachineNetwork(log, cluster, func(agg bool, machineIpnet *net.IPNet, index int) bool { diff --git a/internal/network/machine_network_cidr_test.go b/internal/network/machine_network_cidr_test.go index 87f60146ad6..f962574a90e 100644 --- a/internal/network/machine_network_cidr_test.go +++ b/internal/network/machine_network_cidr_test.go @@ -46,6 +46,11 @@ func createCluster(apiVip string, machineCidr string, inventories ...string) *co }} } +type FreeAddress struct { + Network string `json:"network"` + FreeAddresses []string `json:"free_addresses"` +} + var _ = Describe("inventory", func() { Context("CalculateMachineNetworkCIDR", func() { @@ -133,126 +138,449 @@ var _ = Describe("inventory", func() { }) }) - Context("VerifyVips", func() { + + Context("VerifyVipsForClusterManagedLoadBalancer", func() { var ( - log logrus.FieldLogger - primaryMachineCidr = "1.2.4.0/23" + log logrus.FieldLogger ) BeforeEach(func() { log = logrus.New() }) - It("Same vips", func() { - cluster := createCluster("1.2.5.6", primaryMachineCidr, - createInventory(createInterface("1.2.5.7/23"))) - cluster.Hosts = []*models.Host{ - { - FreeAddresses: "[{\"network\":\"1.2.4.0/23\",\"free_addresses\":[\"1.2.5.6\",\"1.2.5.8\"]}]", - }, - } - cluster.IngressVips = []*models.IngressVip{{IP: models.IP(GetApiVipById(cluster, 0))}} - err := VerifyVips(cluster.Hosts, primaryMachineCidr, GetApiVipById(cluster, 0), GetIngressVipById(cluster, 0), false, log) - Expect(err).To(HaveOccurred()) - }) - - It("Same vips - user managed load balancer", func() { - cluster := createCluster("1.2.5.6", primaryMachineCidr, - createInventory(createInterface("1.2.5.7/23"))) - cluster.Hosts = []*models.Host{ - { - FreeAddresses: "[{\"network\":\"1.2.4.0/23\",\"free_addresses\":[\"1.2.5.6\",\"1.2.5.8\"]}]", - }, - } - cluster.IngressVips = []*models.IngressVip{{IP: models.IP(GetApiVipById(cluster, 0))}} - cluster.LoadBalancer = &models.LoadBalancer{Type: models.LoadBalancerTypeUserManaged} - - err := VerifyVips(cluster.Hosts, primaryMachineCidr, GetApiVipById(cluster, 0), GetIngressVipById(cluster, 0), true, log) - Expect(err).ToNot(HaveOccurred()) - }) - - It("Different vips", func() { - cluster := createCluster("1.2.5.6", primaryMachineCidr, - createInventory(createInterface("1.2.5.7/23"))) - cluster.IngressVips = []*models.IngressVip{{IP: "1.2.5.8"}} - cluster.Hosts = []*models.Host{ - { - FreeAddresses: "[{\"network\":\"1.2.4.0/23\",\"free_addresses\":[\"1.2.5.6\",\"1.2.5.8\"]}]", - }, - } - err := VerifyVips(cluster.Hosts, primaryMachineCidr, GetApiVipById(cluster, 0), GetIngressVipById(cluster, 0), false, log) - Expect(err).ToNot(HaveOccurred()) - }) - - It("Not free", func() { - cluster := createCluster("1.2.5.6", primaryMachineCidr, - createInventory(createInterface("1.2.5.7/23"))) - cluster.IngressVips = []*models.IngressVip{{IP: "1.2.5.8"}} - cluster.Hosts = []*models.Host{ - { - FreeAddresses: "[{\"network\":\"1.2.4.0/23\",\"free_addresses\":[\"1.2.5.9\"]}]", - }, - } - err := VerifyVips(cluster.Hosts, primaryMachineCidr, GetApiVipById(cluster, 0), GetIngressVipById(cluster, 0), false, log) - Expect(err).To(HaveOccurred()) + Context("should fail verification", func() { + It("when IP is not in the machine network", func() { + machineNetworkCidr := "192.168.128.0/24" + apiVip := "192.168.127.1" + ingressVIP := "192.168.127.2" + + bytes, err := json.Marshal([]FreeAddress{{ + Network: "192.168.127.0/24", + FreeAddresses: []string{"192.168.127.1", "192.168.127.2"}, + }}) + Expect(err).ToNot(HaveOccurred()) + hosts := []*models.Host{{FreeAddresses: string(bytes)}} + + err = VerifyVipsForClusterManagedLoadBalancer( + hosts, + machineNetworkCidr, + apiVip, + ingressVIP, + log, + ) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("does not belong to machine-network-cidr")) + }) + + It("when IP is the broadcast address of the machine network", func() { + machineNetworkCidr := "192.168.127.0/24" + apiVip := "192.168.127.255" + ingressVIP := "192.168.127.2" + + bytes, err := json.Marshal([]FreeAddress{{ + Network: "192.168.127.0/24", + FreeAddresses: []string{"192.168.127.1", "192.168.127.2"}, + }}) + Expect(err).ToNot(HaveOccurred()) + hosts := []*models.Host{{FreeAddresses: string(bytes)}} + + err = VerifyVipsForClusterManagedLoadBalancer( + hosts, + machineNetworkCidr, + apiVip, + ingressVIP, + log, + ) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("is the broadcast address of machine-network-cidr")) + }) + + It("when API VIP is not in the free addresses list", func() { + machineNetworkCidr := "192.168.127.0/24" + apiVip := "192.168.127.1" + ingressVIP := "192.168.127.2" + + bytes, err := json.Marshal([]FreeAddress{{ + Network: "192.168.127.0/24", + FreeAddresses: []string{"192.168.127.2"}, + }}) + Expect(err).ToNot(HaveOccurred()) + hosts := []*models.Host{{FreeAddresses: string(bytes)}} + + err = VerifyVipsForClusterManagedLoadBalancer( + hosts, + machineNetworkCidr, + apiVip, + ingressVIP, + log, + ) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("is already in use in cidr")) + }) + + It("when Ingress VIP is not in the free addresses list", func() { + machineNetworkCidr := "192.168.127.0/24" + apiVip := "192.168.127.1" + ingressVIP := "192.168.127.2" + + bytes, err := json.Marshal([]FreeAddress{{ + Network: "192.168.127.0/24", + FreeAddresses: []string{"192.168.127.1"}, + }}) + Expect(err).ToNot(HaveOccurred()) + hosts := []*models.Host{{FreeAddresses: string(bytes)}} + + err = VerifyVipsForClusterManagedLoadBalancer( + hosts, + machineNetworkCidr, + apiVip, + ingressVIP, + log, + ) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("is already in use in cidr")) + }) + + It("when using common VIP", func() { + machineNetworkCidr := "192.168.127.0/24" + apiVip := "192.168.127.1" + ingressVIP := "192.168.127.1" + + bytes, err := json.Marshal([]FreeAddress{{ + Network: "192.168.127.0/24", + FreeAddresses: []string{"192.168.127.1"}, + }}) + Expect(err).ToNot(HaveOccurred()) + hosts := []*models.Host{{FreeAddresses: string(bytes)}} + + err = VerifyVipsForClusterManagedLoadBalancer( + hosts, + machineNetworkCidr, + apiVip, + ingressVIP, + log, + ) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("appears both in apiVIPs and ingressVIPs")) + }) + }) + + Context("should pass verification", func() { + It("with empty machine network", func() { + machineNetworkCidr := "" + apiVip := "192.168.127.1" + ingressVIP := "192.168.127.2" + + bytes, err := json.Marshal([]FreeAddress{{ + Network: "192.168.127.0/24", + FreeAddresses: []string{"192.168.127.1", "192.168.127.2"}, + }}) + Expect(err).ToNot(HaveOccurred()) + hosts := []*models.Host{{FreeAddresses: string(bytes)}} + + err = VerifyVipsForClusterManagedLoadBalancer( + hosts, + machineNetworkCidr, + apiVip, + ingressVIP, + log, + ) + Expect(err).ToNot(HaveOccurred()) + }) + + It("with no free addresses", func() { + machineNetworkCidr := "192.168.127.0/24" + apiVip := "192.168.127.1" + ingressVIP := "192.168.127.2" + hosts := []*models.Host{{FreeAddresses: ""}} + + err := VerifyVipsForClusterManagedLoadBalancer( + hosts, + machineNetworkCidr, + apiVip, + ingressVIP, + log, + ) + Expect(err).ToNot(HaveOccurred()) + }) + + It("with standard configuration", func() { + machineNetworkCidr := "192.168.127.0/24" + apiVip := "192.168.127.1" + ingressVIP := "192.168.127.2" + + bytes, err := json.Marshal([]FreeAddress{{ + Network: "192.168.127.0/24", + FreeAddresses: []string{"192.168.127.1", "192.168.127.2"}, + }}) + Expect(err).ToNot(HaveOccurred()) + hosts := []*models.Host{{FreeAddresses: string(bytes)}} + + err = VerifyVipsForClusterManagedLoadBalancer( + hosts, + machineNetworkCidr, + apiVip, + ingressVIP, + log, + ) + Expect(err).ToNot(HaveOccurred()) + }) }) + }) - It("Not free - user managed load balancer", func() { - cluster := createCluster("1.2.5.6", primaryMachineCidr, - createInventory(createInterface("1.2.5.7/23"))) - cluster.IngressVips = []*models.IngressVip{{IP: "1.2.5.8"}} - cluster.Hosts = []*models.Host{ - { - FreeAddresses: "[{\"network\":\"1.2.4.0/23\",\"free_addresses\":[\"1.2.5.9\"]}]", - }, - } - cluster.LoadBalancer = &models.LoadBalancer{Type: models.LoadBalancerTypeUserManaged} - - err := VerifyVips(cluster.Hosts, primaryMachineCidr, GetApiVipById(cluster, 0), GetIngressVipById(cluster, 0), true, log) - Expect(err).ToNot(HaveOccurred()) - }) + Context("VerifyVipsForUserManangedLoadBalancer", func() { + var log logrus.FieldLogger - It("Empty", func() { - cluster := createCluster("1.2.5.6", primaryMachineCidr, - createInventory(createInterface("1.2.5.7/23"))) - cluster.IngressVips = []*models.IngressVip{{IP: "1.2.5.8"}} - cluster.Hosts = []*models.Host{ - { - FreeAddresses: "", - }, - } - err := VerifyVips(cluster.Hosts, primaryMachineCidr, GetApiVipById(cluster, 0), GetIngressVipById(cluster, 0), false, log) - Expect(err).ToNot(HaveOccurred()) + BeforeEach(func() { + log = logrus.New() }) - It("Free", func() { - cluster := createCluster("1.2.5.6", primaryMachineCidr, - createInventory(createInterface("1.2.5.7/23"))) - cluster.IngressVips = []*models.IngressVip{{IP: "1.2.5.8"}} - cluster.Hosts = []*models.Host{ - { - FreeAddresses: "[{\"network\":\"1.2.4.0/23\",\"free_addresses\":[\"1.2.5.6\",\"1.2.5.8\",\"1.2.5.9\"]}]", - }, - } - err := VerifyVips(cluster.Hosts, primaryMachineCidr, GetApiVipById(cluster, 0), GetIngressVipById(cluster, 0), false, log) - Expect(err).ToNot(HaveOccurred()) - }) + Context("should fail verification", func() { + It("with no machine networks", func() { + machineNetworks := []*models.MachineNetwork{} + apiVip := "192.168.127.1" + ingressVIP := "192.168.127.2" + + bytes, err := json.Marshal([]FreeAddress{{ + Network: "192.168.127.0/24", + FreeAddresses: []string{"192.168.127.1", "192.168.127.2"}, + }}) + Expect(err).ToNot(HaveOccurred()) + hosts := []*models.Host{{FreeAddresses: string(bytes)}} + + err = VerifyVipsForUserManangedLoadBalancer( + hosts, + machineNetworks, + apiVip, + ingressVIP, + log, + ) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("no machine networks to verify VIP")) + }) + + It("with API VIP is not part of any machine network", func() { + machineNetworks := []*models.MachineNetwork{ + {Cidr: "192.168.127.0/24"}, + {Cidr: "192.168.128.0/24"}, + } + apiVip := "192.168.126.1" + ingressVIP := "192.168.127.2" + + bytes, err := json.Marshal([]FreeAddress{{ + Network: "192.168.127.0/24", + FreeAddresses: []string{"192.168.127.1", "192.168.126.2"}, + }}) + Expect(err).ToNot(HaveOccurred()) + hosts := []*models.Host{{FreeAddresses: string(bytes)}} + + err = VerifyVipsForUserManangedLoadBalancer( + hosts, + machineNetworks, + apiVip, + ingressVIP, + log, + ) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("does not belong to machine-network-cidr")) + }) + + It("with Ingress VIP is not part of any machine network", func() { + machineNetworks := []*models.MachineNetwork{ + {Cidr: "192.168.127.0/24"}, + {Cidr: "192.168.128.0/24"}, + } + apiVip := "192.168.127.1" + ingressVIP := "192.168.126.2" + + bytes, err := json.Marshal([]FreeAddress{{ + Network: "192.168.127.0/24", + FreeAddresses: []string{"192.168.127.1", "192.168.126.2"}, + }}) + Expect(err).ToNot(HaveOccurred()) + hosts := []*models.Host{{FreeAddresses: string(bytes)}} + + err = VerifyVipsForUserManangedLoadBalancer( + hosts, + machineNetworks, + apiVip, + ingressVIP, + log, + ) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("does not belong to machine-network-cidr")) + }) + + It("with API VIP that is broadcast address in all machine networks", func() { + machineNetworks := []*models.MachineNetwork{ + {Cidr: "192.168.127.0/24"}, + } + apiVip := "192.168.127.255" + ingressVIP := "192.168.127.1" + + bytes, err := json.Marshal([]FreeAddress{{ + Network: "192.168.127.0/24", + FreeAddresses: []string{"192.168.127.1", "192.168.127.2"}, + }}) + Expect(err).ToNot(HaveOccurred()) + hosts := []*models.Host{{FreeAddresses: string(bytes)}} + + err = VerifyVipsForUserManangedLoadBalancer( + hosts, + machineNetworks, + apiVip, + ingressVIP, + log, + ) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("is the broadcast address of machine-network-cidr")) + }) + + It("with Ingress VIP that is broadcast address in all machine networks", func() { + machineNetworks := []*models.MachineNetwork{ + {Cidr: "192.168.127.0/24"}, + } + apiVip := "192.168.127.1" + ingressVIP := "192.168.127.255" + + bytes, err := json.Marshal([]FreeAddress{{ + Network: "192.168.127.0/24", + FreeAddresses: []string{"192.168.127.1", "192.168.127.2"}, + }}) + Expect(err).ToNot(HaveOccurred()) + hosts := []*models.Host{{FreeAddresses: string(bytes)}} + + err = VerifyVipsForUserManangedLoadBalancer( + hosts, + machineNetworks, + apiVip, + ingressVIP, + log, + ) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("is the broadcast address of machine-network-cidr")) + }) + + It("when no machine networks satisfies the requirments", func() { + machineNetworks := []*models.MachineNetwork{ + {Cidr: "192.168.127.0/24"}, + {Cidr: "192.168.128.0/24"}, + } + apiVip := "192.168.129.1" + ingressVIP := "192.168.128.1" + hosts := []*models.Host{{}} + + err := VerifyVipsForUserManangedLoadBalancer( + hosts, + machineNetworks, + apiVip, + ingressVIP, + log, + ) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal( + "api-vip '192.168.129.1' failed verification: " + + "none of the machine networks is satisfying the requirements, description: " + + "machine network CIDR <192.168.127.0/24> verification failed: " + + "api-vip <192.168.129.1> does not belong to machine-network-cidr <192.168.127.0/24>; " + + "machine network CIDR <192.168.128.0/24> verification failed: " + + "api-vip <192.168.129.1> does not belong to machine-network-cidr <192.168.128.0/24>", + )) + }) + }) + + Context("should pass verification", func() { + It("when API VIP is not in the free addresses list", func() { + machineNetworks := []*models.MachineNetwork{ + {Cidr: "192.168.127.0/24"}, + } + apiVip := "192.168.127.1" + ingressVIP := "192.168.127.2" + + bytes, err := json.Marshal([]FreeAddress{{ + Network: "192.168.127.0/24", + FreeAddresses: []string{"192.168.127.2"}, + }}) + Expect(err).ToNot(HaveOccurred()) + hosts := []*models.Host{{FreeAddresses: string(bytes)}} + + err = VerifyVipsForUserManangedLoadBalancer( + hosts, + machineNetworks, + apiVip, + ingressVIP, + log, + ) + Expect(err).ToNot(HaveOccurred()) + }) + + It("when Ingress VIP is not in the free addresses list", func() { + machineNetworks := []*models.MachineNetwork{ + {Cidr: "192.168.127.0/24"}, + } + apiVip := "192.168.127.1" + ingressVIP := "192.168.127.2" + + bytes, err := json.Marshal([]FreeAddress{{ + Network: "192.168.127.0/24", + FreeAddresses: []string{"192.168.127.1"}, + }}) + Expect(err).ToNot(HaveOccurred()) + hosts := []*models.Host{{FreeAddresses: string(bytes)}} + + err = VerifyVipsForUserManangedLoadBalancer( + hosts, + machineNetworks, + apiVip, + ingressVIP, + log, + ) + Expect(err).ToNot(HaveOccurred()) + }) + + It("with common VIPs", func() { + machineNetworks := []*models.MachineNetwork{ + {Cidr: "192.168.127.0/24"}, + } + apiVip := "192.168.127.1" + ingressVIP := "192.168.127.1" + + bytes, err := json.Marshal([]FreeAddress{{ + Network: "192.168.127.0/24", + FreeAddresses: []string{"192.168.127.1"}, + }}) + Expect(err).ToNot(HaveOccurred()) + hosts := []*models.Host{{FreeAddresses: string(bytes)}} + + err = VerifyVipsForUserManangedLoadBalancer( + hosts, + machineNetworks, + apiVip, + ingressVIP, + log, + ) + Expect(err).ToNot(HaveOccurred()) + }) + + It("when at least one of the machine networks satisfies each VIP", func() { + machineNetworks := []*models.MachineNetwork{ + {Cidr: "192.168.127.0/24"}, + {Cidr: "192.168.128.0/24"}, + } + apiVip := "192.168.127.1" + ingressVIP := "192.168.128.1" + hosts := []*models.Host{{}} + + err := VerifyVipsForUserManangedLoadBalancer( + hosts, + machineNetworks, + apiVip, + ingressVIP, + log, + ) + Expect(err).ToNot(HaveOccurred()) + }) - It("machine cidr is too small", func() { - cluster := createCluster("1.2.5.2", "1.2.5.0/29", createInventory( - createInterface("1.2.5.2/29"), - createInterface("1.2.5.3/29"), - createInterface("1.2.5.4/29"), - createInterface("1.2.5.5/29"), - createInterface("1.2.5.6/29"))) - h := &models.Host{ - FreeAddresses: "[{\"network\":\"1.2.5.0/29\",\"free_addresses\":[\"1.2.5.7\"]}]", - } - cluster.Hosts = []*models.Host{h, h, h, h, h} - cluster.APIVips = []*models.APIVip{{IP: "1.2.5.2"}} - err := VerifyVips(cluster.Hosts, "1.2.5.0/29", GetApiVipById(cluster, 0), GetIngressVipById(cluster, 0), false, log) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("The machine network range is too small for the cluster")) }) }) @@ -371,7 +699,7 @@ var _ = Describe("inventory", func() { }) - Context("IsHostInPrimaryMachineNetCidr", func() { + Context("IsHostInAllMachineNetworksCidr", func() { var log logrus.FieldLogger @@ -380,11 +708,11 @@ var _ = Describe("inventory", func() { }) DescribeTable( - "IsHostInPrimaryMachineNetCidr", + "IsHostInAllMachineNetworksCidr", func(nics []*models.Interface, machineNetworks []*models.MachineNetwork, expectedResult bool) { cluster := createCluster("", "", createInventory(nics...)) cluster.MachineNetworks = machineNetworks - res := IsHostInPrimaryMachineNetCidr(log, cluster, cluster.Hosts[0]) + res := IsHostInAllMachineNetworksCidr(log, cluster, cluster.Hosts[0]) Expect(res).To(Equal(expectedResult)) }, Entry("MachineNetworks is empty", []*models.Interface{createInterface("1.2.3.4/24")}, []*models.MachineNetwork{}, false), diff --git a/internal/network/utils.go b/internal/network/utils.go index a9129a1023a..120155b0802 100644 --- a/internal/network/utils.go +++ b/internal/network/utils.go @@ -208,6 +208,15 @@ func DerefClusterNetworks(obj interface{}) []*models.ClusterNetwork { } } +func DerefClusterLoadBalancer(obj interface{}) *models.LoadBalancer { + switch v := obj.(type) { + case *models.LoadBalancer: + return v + default: + return nil + } +} + func GetMachineNetworkCidrs(cluster *common.Cluster) (ret []string) { for _, n := range cluster.MachineNetworks { ret = append(ret, string(n.Cidr))