ensure no duplicate networks for cloudstack (#7943)

cmd/eks-a-tool/cmd/cloudstackrmvms.go

@@ -25,7 +25,7 @@ var cloudstackRmVmsCmd = &cobra.Command{
 		if err != nil {
 			return err
 		}
-		err = cleanup.CleanUpCloudstackTestResources(cmd.Context(), clusterName, viper.GetBool(dryRunFlag))
+		err = cleanup.CloudstackTestResources(cmd.Context(), clusterName, viper.GetBool(dryRunFlag), false)
 		if err != nil {
 			log.Fatalf("Error removing vms: %v", err)
 		}

cmd/integration_test/build/buildspecs/cloudstack-test-eks-a-cli.yml

@@ -82,7 +82,8 @@ phases:
       - >
         ./bin/test e2e cleanup cloudstack
         -n ${CLUSTER_NAME_PREFIX}
-        -v 4
+        --delete-duplicate-networks
+        -v 6
   build:
     commands:
       - export JOB_ID=$CODEBUILD_BUILD_ID

cmd/integration_test/build/buildspecs/quick-test-eks-a-cli.yml

@@ -177,7 +177,8 @@ phases:
       - >
         ./bin/test e2e cleanup cloudstack
         -n ${CLUSTER_NAME_PREFIX}
-        -v 4
+        --delete-duplicate-networks
+        -v 6
   build:
     commands:
       - export JOB_ID=$CODEBUILD_BUILD_ID

cmd/integration_test/cmd/cleanupcloudstack.go

@@ -37,11 +37,14 @@ func preRunCleanUpCloudstackSetup(cmd *cobra.Command, args []string) {
 	})
 }
 
+const deleteDuplicateNetworksFlag = "delete-duplicate-networks"
+
 var requiredCloudstackCleanUpFlags = []string{clusterNameFlagName}
 
 func init() {
 	cleanUpInstancesCmd.AddCommand(cleanUpCloudstackCmd)
 	cleanUpCloudstackCmd.Flags().StringP(clusterNameFlagName, "n", "", "Cluster name for associated vms")
+	cleanUpCloudstackCmd.Flags().Bool(deleteDuplicateNetworksFlag, false, "Delete duplicate isolated networks")
 
 	for _, flag := range requiredCloudstackCleanUpFlags {
 		if err := cleanUpCloudstackCmd.MarkFlagRequired(flag); err != nil {
@@ -52,7 +55,8 @@ func init() {
 
 func cleanUpCloudstackTestResources(ctx context.Context) error {
 	clusterName := viper.GetString(clusterNameFlagName)
-	err := cleanup.CleanUpCloudstackTestResources(ctx, clusterName, false)
+	deleteDuplicateNetworks := viper.IsSet(deleteDuplicateNetworksFlag)
+	err := cleanup.CloudstackTestResources(ctx, clusterName, false, deleteDuplicateNetworks)
 	if err != nil {
 		return fmt.Errorf("running cleanup for cloudstack vms: %v", err)
 	}

internal/test/cleanup/cleanup.go

@@ -3,6 +3,7 @@ package cleanup
 import (
 	"context"
 	"fmt"
+	"os"
 	"strconv"
 	"time"
 
@@ -22,8 +23,9 @@ import (
 )
 
 const (
-	cleanupRetries = 5
-	retryBackoff   = 10 * time.Second
+	cleanupRetries       = 5
+	retryBackoff         = 10 * time.Second
+	cloudstackNetworkVar = "T_CLOUDSTACK_NETWORK"
 )
 
 func CleanUpAwsTestResources(storageBucket string, maxAge string, tag string) error {
@@ -95,7 +97,9 @@ func VsphereRmVms(ctx context.Context, clusterName string, opts ...executables.G
 	return govc.CleanupVms(ctx, clusterName, false)
 }
 
-func CleanUpCloudstackTestResources(ctx context.Context, clusterName string, dryRun bool) error {
+// CloudstackTestResources cleans up resources on the CloudStack environment.
+// This can include VMs as well as duplicate networks.
+func CloudstackTestResources(ctx context.Context, clusterName string, dryRun bool, deleteDuplicateNetworks bool) error {
 	executableBuilder, close, err := executables.InitInDockerExecutablesBuilder(ctx, executables.DefaultEksaImage())
 	if err != nil {
 		return fmt.Errorf("unable to initialize executables: %v", err)
@@ -128,6 +132,25 @@ func CleanUpCloudstackTestResources(ctx context.Context, clusterName string, dry
 	if len(errorsMap) > 0 {
 		return fmt.Errorf("cleaning up VMs: %+v", errorsMap)
 	}
+
+	return cleanupCloudstackDuplicateNetworks(ctx, cmk, execConfig, deleteDuplicateNetworks)
+}
+
+func cleanupCloudstackDuplicateNetworks(ctx context.Context, cmk *executables.Cmk, execConfig *decoder.CloudStackExecConfig, deleteDuplicateNetworks bool) error {
+	if !deleteDuplicateNetworks {
+		return nil
+	}
+
+	networkName, set := os.LookupEnv(cloudstackNetworkVar)
+	if !set {
+		return fmt.Errorf("ensuring no duplicate networks, %s is not set", cloudstackNetworkVar)
+	}
+
+	for _, profile := range execConfig.Profiles {
+		if err := cmk.EnsureNoDuplicateNetwork(ctx, profile.Name, networkName); err != nil {
+			return err
+		}
+	}
 	return nil
 }
 

pkg/executables/cmk.go

@@ -291,6 +291,35 @@ func (c *Cmk) ValidateDomainAndGetId(ctx context.Context, profile string, domain
 	return domainId, nil
 }
 
+// EnsureNoDuplicateNetwork ensures that there are no duplicate networks with the name networkName.
+// If it finds duplicates that are not shared networks, it deletes them.
+func (c *Cmk) EnsureNoDuplicateNetwork(ctx context.Context, profile string, networkName string) error {
+	command := newCmkCommand(fmt.Sprintf("list networks filter=name,id,type keyword=%s", networkName))
+	result, err := c.exec(ctx, profile, command...)
+	if err != nil {
+		return fmt.Errorf("getting network info - %s: %v", result.String(), err)
+	}
+
+	response := struct {
+		CmkNetworks []cmkNetwork `json:"network"`
+	}{}
+	if err = json.Unmarshal(result.Bytes(), &response); err != nil {
+		return fmt.Errorf("parsing response into json: %v", err)
+	}
+
+	for _, network := range response.CmkNetworks {
+		if !strings.EqualFold(network.Type, "Shared") {
+			command := newCmkCommand(fmt.Sprintf("delete network id=%s force=true", network.ID))
+			result, err := c.exec(ctx, profile, command...)
+			if err != nil {
+				return fmt.Errorf("deleting duplicate network with ID %s - %s: %v", network, result.String(), err)
+			}
+		}
+	}
+
+	return nil
+}
+
 func (c *Cmk) ValidateNetworkPresent(ctx context.Context, profile string, domainId string, network v1alpha1.CloudStackResourceIdentifier, zoneId string, account string) error {
 	command := newCmkCommand("list networks")
 	// account must be specified within a domainId
@@ -436,10 +465,6 @@ func (c *Cmk) CleanupVms(ctx context.Context, profile string, clusterName string
 }
 
 func (c *Cmk) exec(ctx context.Context, profile string, args ...string) (stdout bytes.Buffer, err error) {
-	if err != nil {
-		return bytes.Buffer{}, fmt.Errorf("failed get environment map: %v", err)
-	}
-
 	configFile, err := c.buildCmkConfigFile(profile)
 	if err != nil {
 		return bytes.Buffer{}, fmt.Errorf("failed cmk validations: %v", err)
@@ -490,6 +515,12 @@ type cmkServiceOffering struct {
 	Name      string `json:"name"`
 }
 
+type cmkNetwork struct {
+	ID   string `json:"id"`
+	Name string `json:"name"`
+	Type string `json:"type"`
+}
+
 type cmkResourceIdentifier struct {
 	Id   string `json:"id"`
 	Name string `json:"name"`

pkg/executables/cmk_test.go

@@ -215,6 +215,79 @@ func TestCmkCleanupVms(t *testing.T) {
 	}
 }
 
+func TestCmkEnsureNoDuplicateNetwork(t *testing.T) {
+	_, writer := test.NewWriter(t)
+	configFilePath, _ := filepath.Abs(filepath.Join(writer.Dir(), "generated", cmkConfigFileName))
+	tests := []struct {
+		testName           string
+		argumentsExecCalls [][]string
+		jsonResponseFile   string
+		cmkFunc            func(cmk executables.Cmk, ctx context.Context) error
+		cmkResponseError   error
+		wantErr            bool
+	}{
+		{
+			testName:         "EnsureNoDuplicateNetwork success on no duplicate networks",
+			jsonResponseFile: "testdata/cmk_list_network_multiple.json",
+			argumentsExecCalls: [][]string{
+				{
+					"-c", configFilePath,
+					"list", "networks", "filter=name,id,type", "keyword=eksa-cloudstack-ci-net",
+				},
+			},
+			cmkFunc: func(cmk executables.Cmk, ctx context.Context) error {
+				return cmk.EnsureNoDuplicateNetwork(ctx, execConfig.Profiles[0].Name, "eksa-cloudstack-ci-net")
+			},
+			cmkResponseError: nil,
+			wantErr:          false,
+		},
+		{
+			testName:         "EnsureNoDuplicateNetwork success on deleting duplicate networks",
+			jsonResponseFile: "testdata/cmk_list_network_duplicates.json",
+			argumentsExecCalls: [][]string{
+				{
+					"-c", configFilePath,
+					"list", "networks", "filter=name,id,type", "keyword=eksa-cloudstack-ci-net",
+				},
+				{
+					"-c", configFilePath,
+					"delete", "network", "id=fe1a7310-51d4-4299-b3d0-a627a57bb4b0", "force=true",
+				},
+				{
+					"-c", configFilePath,
+					"delete", "network", "id=24fd6849-3016-4afe-948d-4ce2bb396cf5", "force=true",
+				},
+			},
+			cmkFunc: func(cmk executables.Cmk, ctx context.Context) error {
+				return cmk.EnsureNoDuplicateNetwork(ctx, execConfig.Profiles[0].Name, "eksa-cloudstack-ci-net")
+			},
+			cmkResponseError: nil,
+			wantErr:          false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.testName, func(t *testing.T) {
+			fileContent := test.ReadFile(t, tt.jsonResponseFile)
+
+			ctx := context.Background()
+			mockCtrl := gomock.NewController(t)
+
+			executable := mockexecutables.NewMockExecutable(mockCtrl)
+			for _, argsList := range tt.argumentsExecCalls {
+				executable.EXPECT().Execute(ctx, argsList).
+					Return(*bytes.NewBufferString(fileContent), tt.cmkResponseError)
+			}
+			cmk, _ := executables.NewCmk(executable, writer, execConfig)
+			err := tt.cmkFunc(*cmk, ctx)
+			if tt.wantErr && err != nil || !tt.wantErr && err == nil {
+				return
+			}
+			t.Fatalf("Cmk error: %v", err)
+		})
+	}
+}
+
 func TestNewCmkNilConfig(t *testing.T) {
 	_, err := executables.NewCmk(nil, nil, nil)
 	if err == nil {

pkg/executables/testdata/cmk_list_network_duplicates.json

@@ -0,0 +1,20 @@
+{
+  "count": 3,
+  "network": [
+    {
+      "id": "fe1a7310-51d4-4299-b3d0-a627a57bb4b0",
+      "name": "eksa-cloudstack-ci-net",
+      "type": "Isolated"
+    },
+    {
+      "id": "24fd6849-3016-4afe-948d-4ce2bb396cf5",
+      "name": "eksa-cloudstack-ci-net",
+      "type": "Isolated"
+    },
+    {
+      "id": "13b501c1-5629-40e1-ba1e-a31caa9aead4",
+      "name": "eksa-cloudstack-ci-net",
+      "type": "Shared"
+    }
+  ]
+}

test/framework/cloudstack.go

@@ -255,7 +255,7 @@ func (c *CloudStack) ClusterConfigUpdates() []api.ClusterConfigFiller {
 }
 
 func (c *CloudStack) CleanupVMs(clusterName string) error {
-	return cleanup.CleanUpCloudstackTestResources(context.Background(), clusterName, false)
+	return cleanup.CloudstackTestResources(context.Background(), clusterName, false, false)
 }
 
 func (c *CloudStack) WithProviderUpgrade(fillers ...api.CloudStackFiller) ClusterE2ETestOpt {