Skip to content

Commit

Permalink
Set ClusterNetworkReady to false if VirtualNetwork ready condition is…
Browse files Browse the repository at this point in the history 
…n't set
  • Loading branch information
Danqing Hou committed Apr 10, 2024
1 parent daa233c commit 4efbf97
Show file tree
Hide file tree
Showing 2 changed files with 93 additions and 34 deletions.
100 changes: 74 additions & 26 deletions pkg/services/network/network_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand All @@ -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"
Expand Down Expand Up @@ -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())
})
})
Expand All @@ -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())
})
})
Expand All @@ -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{}
Expand Down Expand Up @@ -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{}
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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() {
Expand Down
27 changes: 19 additions & 8 deletions pkg/services/network/nsxt_provider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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 {
Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit 4efbf97

Please sign in to comment.