diff --git a/pkg/services/network/network_test.go b/pkg/services/network/network_test.go index 580877d2fc..b083549569 100644 --- a/pkg/services/network/network_test.go +++ b/pkg/services/network/network_test.go @@ -49,20 +49,28 @@ const ( ) // MockNSXTVpcNetworkProvider is the mock. -type MockNSXTVpcNetworkProvider struct { - *nsxtVPCNetworkProvider +type MockNSXTNetworkProvider struct { + *nsxtNetworkProvider } -func createUnReadyNsxtVirtualNetwork(ctx *vmware.ClusterContext, status ncpv1.VirtualNetworkStatus) *ncpv1.VirtualNetwork { - // create an nsxt vnet with unready status caused by certain reasons from ncp - cluster := ctx.VSphereCluster - return &ncpv1.VirtualNetwork{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: cluster.Namespace, - Name: GetNSXTVirtualNetworkName(cluster.Name), - }, - Status: status, +func (m *MockNSXTNetworkProvider) ProvisionClusterNetwork(ctx context.Context, clusterCtx *vmware.ClusterContext) error { + err := m.nsxtNetworkProvider.ProvisionClusterNetwork(ctx, clusterCtx) + + if err != nil { + // Check if the error contains the string "virtual network ready status" + if strings.Contains(err.Error(), "virtual network ready status") { + conditions.MarkTrue(clusterCtx.VSphereCluster, vmwarev1.ClusterNetworkReadyCondition) + return nil + } + // return the original error if it doesn't contain the specific string + return err } + return nil +} + +// MockNSXTVpcNetworkProvider is the mock. +type MockNSXTVpcNetworkProvider struct { + *nsxtVPCNetworkProvider } func (m *MockNSXTVpcNetworkProvider) ProvisionClusterNetwork(ctx context.Context, clusterCtx *vmware.ClusterContext) error { @@ -71,6 +79,7 @@ func (m *MockNSXTVpcNetworkProvider) ProvisionClusterNetwork(ctx context.Context if err != nil { // Check if the error contains the string "subnetset ready status" if strings.Contains(err.Error(), "subnetset ready status") { + conditions.MarkTrue(clusterCtx.VSphereCluster, vmwarev1.ClusterNetworkReadyCondition) return nil } // return the original error if it doesn't contain the specific string @@ -79,6 +88,18 @@ func (m *MockNSXTVpcNetworkProvider) ProvisionClusterNetwork(ctx context.Context return nil } +func createUnReadyNsxtVirtualNetwork(ctx *vmware.ClusterContext, status ncpv1.VirtualNetworkStatus) *ncpv1.VirtualNetwork { + // create an nsxt vnet with unready status caused by certain reasons from ncp + cluster := ctx.VSphereCluster + return &ncpv1.VirtualNetwork{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: cluster.Namespace, + Name: GetNSXTVirtualNetworkName(cluster.Name), + }, + Status: status, + } +} + var _ = Describe("Network provider", func() { var ( dummyNs = "dummy-ns" @@ -315,11 +336,11 @@ var _ = Describe("Network provider", func() { }) JustBeforeEach(func() { err = np.ProvisionClusterNetwork(ctx, clusterCtx) + Expect(err).ToNot(HaveOccurred()) }) It("should succeed", func() { + vnet, err := np.GetClusterNetworkName(ctx, clusterCtx) Expect(err).ToNot(HaveOccurred()) - vnet, localerr := np.GetClusterNetworkName(ctx, clusterCtx) - Expect(localerr).ToNot(HaveOccurred()) Expect(vnet).To(BeEmpty()) }) }) @@ -334,9 +355,9 @@ var _ = Describe("Network provider", func() { JustBeforeEach(func() { // noop for netop err = np.ProvisionClusterNetwork(ctx, clusterCtx) + Expect(err).ToNot(HaveOccurred()) }) It("should succeed", func() { - Expect(err).ToNot(HaveOccurred()) Expect(conditions.IsTrue(clusterCtx.VSphereCluster, vmwarev1.ClusterNetworkReadyCondition)).To(BeTrue()) }) }) @@ -347,12 +368,12 @@ var _ = Describe("Network provider", func() { nsxNp, _ = NsxtNetworkProvider(client, "true").(*nsxtNetworkProvider) np = nsxNp err = np.ProvisionClusterNetwork(ctx, clusterCtx) + Expect(err).ToNot(HaveOccurred()) }) It("should not update vnet with whitelist_source_ranges in spec", func() { + vnet, err := np.GetClusterNetworkName(ctx, clusterCtx) Expect(err).ToNot(HaveOccurred()) - vnet, localerr := np.GetClusterNetworkName(ctx, clusterCtx) - Expect(localerr).ToNot(HaveOccurred()) Expect(vnet).To(Equal(GetNSXTVirtualNetworkName(clusterCtx.VSphereCluster.Name))) createdVNET := &ncpv1.VirtualNetwork{} @@ -381,13 +402,18 @@ var _ = Describe("Network provider", func() { client = fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(configmapObj, systemNamespaceObj).Build() nsxNp, _ = NsxtNetworkProvider(client, "true").(*nsxtNetworkProvider) np = nsxNp - err = np.ProvisionClusterNetwork(ctx, clusterCtx) + // The ProvisionClusterNetwork function would fail due to the absence of + // ncp to set the `virtual network ready` condition. + // We use the mock function to disregard this specific error. + // mocknp is an instance of MockNSXNetworkProvider initialized with nsxNp. + mocknp := &MockNSXTNetworkProvider{nsxNp} + err = mocknp.ProvisionClusterNetwork(ctx, clusterCtx) + Expect(err).ToNot(HaveOccurred()) }) It("should create vnet without whitelist_source_ranges in spec", func() { + vnet, err := np.GetClusterNetworkName(ctx, clusterCtx) Expect(err).ToNot(HaveOccurred()) - vnet, localerr := np.GetClusterNetworkName(ctx, clusterCtx) - Expect(localerr).ToNot(HaveOccurred()) Expect(vnet).To(Equal(GetNSXTVirtualNetworkName(clusterCtx.VSphereCluster.Name))) createdVNET := &ncpv1.VirtualNetwork{} @@ -408,12 +434,12 @@ var _ = Describe("Network provider", func() { nsxNp, _ = NsxtNetworkProvider(client, "false").(*nsxtNetworkProvider) np = nsxNp err = np.ProvisionClusterNetwork(ctx, clusterCtx) + Expect(err).ToNot(HaveOccurred()) }) It("should update vnet with whitelist_source_ranges in spec", func() { + vnet, err := np.GetClusterNetworkName(ctx, clusterCtx) Expect(err).ToNot(HaveOccurred()) - vnet, localerr := np.GetClusterNetworkName(ctx, clusterCtx) - Expect(localerr).ToNot(HaveOccurred()) Expect(vnet).To(Equal(GetNSXTVirtualNetworkName(clusterCtx.VSphereCluster.Name))) // Verify WhitelistSourceRanges have been updated @@ -435,13 +461,18 @@ var _ = Describe("Network provider", func() { client = fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(configmapObj, systemNamespaceObj).Build() nsxNp, _ = NsxtNetworkProvider(client, "false").(*nsxtNetworkProvider) np = nsxNp - err = np.ProvisionClusterNetwork(ctx, clusterCtx) + // The ProvisionClusterNetwork function would fail due to the absence of + // ncp to set the `virtual network ready` condition. + // We use the mock function to disregard this specific error. + // mocknp is an instance of MockNSXNetworkProvider initialized with nsxNp. + mocknp := &MockNSXTNetworkProvider{nsxNp} + err = mocknp.ProvisionClusterNetwork(ctx, clusterCtx) + Expect(err).ToNot(HaveOccurred()) }) It("should create new vnet with whitelist_source_ranges in spec", func() { + vnet, err := np.GetClusterNetworkName(ctx, clusterCtx) Expect(err).ToNot(HaveOccurred()) - vnet, localerr := np.GetClusterNetworkName(ctx, clusterCtx) - Expect(localerr).ToNot(HaveOccurred()) Expect(vnet).To(Equal(GetNSXTVirtualNetworkName(clusterCtx.VSphereCluster.Name))) // Verify WhitelistSourceRanges have been updated @@ -466,12 +497,12 @@ var _ = Describe("Network provider", func() { nsxNp, _ = NsxtNetworkProvider(client, "false").(*nsxtNetworkProvider) np = nsxNp err = np.ProvisionClusterNetwork(ctx, clusterCtx) + Expect(err).ToNot(HaveOccurred()) }) It("should not update vnet with whitelist_source_ranges in spec", func() { + vnet, err := np.GetClusterNetworkName(ctx, clusterCtx) Expect(err).ToNot(HaveOccurred()) - vnet, localerr := np.GetClusterNetworkName(ctx, clusterCtx) - Expect(localerr).ToNot(HaveOccurred()) Expect(vnet).To(Equal(GetNSXTVirtualNetworkName(clusterCtx.VSphereCluster.Name))) // Verify WhitelistSourceRanges is not included @@ -524,6 +555,23 @@ var _ = Describe("Network provider", func() { Expect(err).To(MatchError(expectedErrorMessage)) Expect(conditions.IsFalse(clusterCtx.VSphereCluster, vmwarev1.ClusterNetworkReadyCondition)).To(BeTrue()) }) + + It("should return error when vnet ready status is not set", func() { + By("create a cluster with virtual network has no ready status") + status := ncpv1.VirtualNetworkStatus{ + Conditions: []ncpv1.VirtualNetworkCondition{}, + } + vnetObj = createUnReadyNsxtVirtualNetwork(clusterCtx, status) + client = fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(vnetObj).Build() + nsxNp, _ = NsxtNetworkProvider(client, "false").(*nsxtNetworkProvider) + np = nsxNp + + err = np.VerifyNetworkStatus(ctx, clusterCtx, vnetObj) + + expectedErrorMessage := fmt.Sprintf("virtual network ready status in cluster %s has not been set", apitypes.NamespacedName{Namespace: dummyNs, Name: dummyCluster}) + Expect(err).To(MatchError(expectedErrorMessage)) + Expect(conditions.IsFalse(clusterCtx.VSphereCluster, vmwarev1.ClusterNetworkReadyCondition)).To(BeTrue()) + }) }) Context("with NSX-VPC network provider and subnetset exists", func() { diff --git a/pkg/services/network/nsxt_provider.go b/pkg/services/network/nsxt_provider.go index 31d27cbc84..7cde4b7dc6 100644 --- a/pkg/services/network/nsxt_provider.go +++ b/pkg/services/network/nsxt_provider.go @@ -65,18 +65,29 @@ func GetNSXTVirtualNetworkName(clusterName string) string { return fmt.Sprintf("%s-vnet", clusterName) } -func (np *nsxtNetworkProvider) verifyNSXTVirtualNetworkStatus(ctx *vmware.ClusterContext, vnet *ncpv1.VirtualNetwork) error { - clusterName := ctx.VSphereCluster.Name - namespace := ctx.VSphereCluster.Namespace +func (np *nsxtNetworkProvider) verifyNSXTVirtualNetworkStatus(vspherecluster *vmwarev1.VSphereCluster, vnet *ncpv1.VirtualNetwork) error { + clusterName := vspherecluster.Name + namespace := vspherecluster.Namespace + hasReadyCondition := false + for _, condition := range vnet.Status.Conditions { - if condition.Type == "Ready" && condition.Status != "True" { - conditions.MarkFalse(ctx.VSphereCluster, vmwarev1.ClusterNetworkReadyCondition, vmwarev1.ClusterNetworkProvisionFailedReason, clusterv1.ConditionSeverityWarning, condition.Message) + if condition.Type != "Ready" { + continue + } + hasReadyCondition = true + if condition.Status != "True" { + conditions.MarkFalse(vspherecluster, vmwarev1.ClusterNetworkReadyCondition, vmwarev1.ClusterNetworkProvisionFailedReason, clusterv1.ConditionSeverityWarning, condition.Message) return errors.Errorf("virtual network ready status is: '%s' in cluster %s. reason: %s, message: %s", condition.Status, types.NamespacedName{Namespace: namespace, Name: clusterName}, condition.Reason, condition.Message) } } - conditions.MarkTrue(ctx.VSphereCluster, vmwarev1.ClusterNetworkReadyCondition) + if !hasReadyCondition { + conditions.MarkFalse(vspherecluster, vmwarev1.ClusterNetworkReadyCondition, vmwarev1.ClusterNetworkProvisionFailedReason, clusterv1.ConditionSeverityWarning, "No Ready status for virtual network") + return errors.Errorf("virtual network ready status in cluster %s has not been set", types.NamespacedName{Namespace: namespace, Name: clusterName}) + } + + conditions.MarkTrue(vspherecluster, vmwarev1.ClusterNetworkReadyCondition) return nil } @@ -86,7 +97,7 @@ func (np *nsxtNetworkProvider) VerifyNetworkStatus(_ context.Context, clusterCtx return fmt.Errorf("expected NCP VirtualNetwork but got %T", obj) } - return np.verifyNSXTVirtualNetworkStatus(clusterCtx, vnet) + return np.verifyNSXTVirtualNetworkStatus(clusterCtx.VSphereCluster, vnet) } func (np *nsxtNetworkProvider) ProvisionClusterNetwork(ctx context.Context, clusterCtx *vmware.ClusterContext) error { @@ -147,7 +158,7 @@ func (np *nsxtNetworkProvider) ProvisionClusterNetwork(ctx context.Context, clus return errors.Wrap(err, "Failed to provision network") } - return np.verifyNSXTVirtualNetworkStatus(clusterCtx, vnet) + return np.verifyNSXTVirtualNetworkStatus(clusterCtx.VSphereCluster, vnet) } // GetClusterNetworkName returns the name of a valid cluster network if one exists.