OCM-13218 | test: fix the failure of cluster provision and resource c…

…lean up

tests/ci/config/config.go

@@ -55,6 +55,7 @@ type GlobalENVVariables struct {
 	NamePrefix            string `env:"NAME_PREFIX"`
 	ClusterWaitingTime    int    `env:"CLUSTER_TIMEOUT" default:"60"`
 	WaitSetupClusterReady bool   `env:"WAIT_SETUP_CLUSTER_READY" default:"true"`
+	AWSCredetialsFile     string `env:"AWS_SHARED_CREDENTIALS_FILE" default:""`
 	SVPC_CREDENTIALS_FILE string `env:"SHARED_VPC_AWS_SHARED_CREDENTIALS_FILE" default:""`
 	OCM_LOGIN_ENV         string `env:"OCM_LOGIN_ENV" default:""`
 }
@@ -125,6 +126,7 @@ func init() {
 		ProvisionShard:        os.Getenv("PROVISION_SHARD"),
 		NamePrefix:            os.Getenv("NAME_PREFIX"),
 		SVPC_CREDENTIALS_FILE: os.Getenv("SHARED_VPC_AWS_SHARED_CREDENTIALS_FILE"),
+		AWSCredetialsFile:     os.Getenv("AWS_SHARED_CREDENTIALS_FILE"),
 		OCM_LOGIN_ENV:         os.Getenv("OCM_LOGIN_ENV"),
 		ClusterWaitingTime:    waitingTime,
 		WaitSetupClusterReady: waitSetupClusterReady,

tests/e2e/dummy_test.go

@@ -56,9 +56,9 @@ var _ = Describe("ROSA CLI Test", func() {
 		It("VPCClientTesting", func() {
 			client := rosacli.NewClient()
 			region := "us-east-1"
-			resourcesHandler, err := handler.NewTempResourcesHandler(client, region, "")
+			resourcesHandler, err := handler.NewTempResourcesHandler(client, region, "", "")
 			Expect(err).ToNot(HaveOccurred())
-			vpcClient, err := resourcesHandler.PrepareVPC("xueli-test", "10.0.0.0/16", false)
+			vpcClient, err := resourcesHandler.PrepareVPC("xueli-test", "10.0.0.0/16", false, false)
 			Expect(err).ToNot(HaveOccurred())
 			defer resourcesHandler.DestroyResources()
 			subnets, err := resourcesHandler.PrepareSubnets([]string{}, true)

tests/e2e/hcp_cluster_test.go

@@ -236,13 +236,12 @@ var _ = Describe("HCP cluster testing",
 				jsonData := rosaClient.Parser.JsonData.Input(jsonOutput).Parse()
 				installRoleArn := jsonData.DigString("aws", "sts", "role_arn")
 
-				By("Get additional principal credentials")
-				awsSharedCredentialFile := ciConfig.Test.GlobalENV.SVPC_CREDENTIALS_FILE
-
 				By("Create additional account roles")
 				accrolePrefix := "arPrefix74556"
 
-				resourcesHandler, err := handler.NewTempResourcesHandler(rosaClient, profile.Region, awsSharedCredentialFile)
+				resourcesHandler, err := handler.NewTempResourcesHandler(rosaClient, profile.Region,
+					ciConfig.Test.GlobalENV.AWSCredetialsFile,
+					ciConfig.Test.GlobalENV.SVPC_CREDENTIALS_FILE)
 				Expect(err).ToNot(HaveOccurred())
 				additionalPrincipalRoleName := fmt.Sprintf("%s-%s", accrolePrefix, "additional-principal-role")
 				additionalPrincipalRoleArn, err := resourcesHandler.PrepareAdditionalPrincipalsRole(

tests/e2e/hcp_machine_pool_test.go

@@ -10,6 +10,7 @@ import (
 	. "github.com/onsi/gomega"
 	"github.com/openshift-online/ocm-common/pkg/test/vpc_client"
 
+	ciConfig "github.com/openshift/rosa/tests/ci/config"
 	"github.com/openshift/rosa/tests/ci/labels"
 	"github.com/openshift/rosa/tests/utils/config"
 	"github.com/openshift/rosa/tests/utils/constants"
@@ -541,11 +542,13 @@ var _ = Describe("HCP Machine Pool", labels.Feature.Machinepool, func() {
 
 			By("with subnet not in VPC")
 			vpcPrefix := helper.TrimNameByLength("c56786", 20)
-			resourcesHandler, err := handler.NewTempResourcesHandler(rosaClient, profile.Region, "")
+			resourcesHandler, err := handler.NewTempResourcesHandler(rosaClient, profile.Region,
+				ciConfig.Test.GlobalENV.AWSCredetialsFile,
+				ciConfig.Test.GlobalENV.SVPC_CREDENTIALS_FILE)
 			Expect(err).ToNot(HaveOccurred())
 			defer resourcesHandler.DestroyResources()
 
-			vpc, err := resourcesHandler.PrepareVPC(vpcPrefix, constants.DefaultVPCCIDRValue, false)
+			vpc, err := resourcesHandler.PrepareVPC(vpcPrefix, constants.DefaultVPCCIDRValue, false, false)
 			Expect(err).ToNot(HaveOccurred())
 			zones, err := vpc.AWSClient.ListAvaliableZonesForRegion(profile.Region, "availability-zone")
 			Expect(err).ToNot(HaveOccurred())

tests/e2e/test_rosacli_cluster.go

@@ -1360,7 +1360,7 @@ var _ = Describe("Classic cluster creation validation",
 
 				By("Prepare a vpc for the testing")
 				resourcesHandler := clusterHandler.GetResourcesHandler()
-				_, err := resourcesHandler.PrepareVPC(caseNumber, "", true)
+				_, err := resourcesHandler.PrepareVPC(caseNumber, "", true, false)
 				Expect(err).ToNot(HaveOccurred())
 				subnetMap, err := resourcesHandler.PrepareSubnets([]string{}, false)
 				Expect(err).ToNot(HaveOccurred())
@@ -2484,7 +2484,7 @@ var _ = Describe("HCP cluster creation negative testing",
 
 				By("Prepare a vpc for the testing")
 				resourcesHandler := clusterHandler.GetResourcesHandler()
-				vpc, err := resourcesHandler.PrepareVPC(vpcName, "", false)
+				vpc, err := resourcesHandler.PrepareVPC(vpcName, "", false, false)
 				Expect(err).ToNot(HaveOccurred())
 				defer vpc.DeleteVPCChain()
 

tests/utils/handler/cluster_handler.go

@@ -73,41 +73,37 @@ func newClusterHandler(client *rosacli.Client,
 		if err != nil {
 			return nil, err
 		}
-
 	}
 
 	// Make sure shared VPC credentials file based on profile
-	awsSharedCredentialsFile := ""
-	if profile.ClusterConfig.SharedVPC || profile.ClusterConfig.AdditionalPrincipals {
-		awsSharedCredentialsFile = config.Test.GlobalENV.SVPC_CREDENTIALS_FILE
-		if awsSharedCredentialsFile == "" {
-			log.Logger.Errorf(envVariableErrMsg, awsSharedCredentialsFile)
-			return nil, fmt.Errorf(envVariableErrMsg, awsSharedCredentialsFile)
-		}
+	awsCredentialsFile := config.Test.GlobalENV.AWSCredetialsFile
+	awsSharedAccountCredentialsFile := config.Test.GlobalENV.SVPC_CREDENTIALS_FILE
+	if profile.ClusterConfig.SharedVPC && awsSharedAccountCredentialsFile == "" {
+		return nil, fmt.Errorf(envVariableErrMsg, awsSharedAccountCredentialsFile)
 	}
 
 	resourcesHandler, err := newResourcesHandler(
 		client,
 		profile.Region,
 		persist,
 		loadFromFilesystem,
-		awsSharedCredentialsFile,
+		awsCredentialsFile,
+		awsSharedAccountCredentialsFile,
 	)
 	if err != nil {
 		return nil, err
 	}
 
 	// Make sure shared VPC credentials file based on loaded resources
-	if resourcesHandler.resources.SharedVPCRole != "" ||
+	if (resourcesHandler.resources.SharedVPCRole != "" ||
 		resourcesHandler.resources.AdditionalPrincipals != "" ||
-		resourcesHandler.resources.ResourceShareArn != "" {
-		awsSharedCredentialsFile = config.Test.GlobalENV.SVPC_CREDENTIALS_FILE
-		if awsSharedCredentialsFile == "" {
-			log.Logger.Errorf(envVariableErrMsg, awsSharedCredentialsFile)
-			return nil, fmt.Errorf(envVariableErrMsg, awsSharedCredentialsFile)
-		}
+		resourcesHandler.resources.ResourceShareArn != "") &&
+		resourcesHandler.awsSharedAccountCredentialsFile == "" {
+
+		log.Logger.Errorf(envVariableErrMsg, awsCredentialsFile)
+		return nil, fmt.Errorf(envVariableErrMsg, awsCredentialsFile)
+
 	}
-	resourcesHandler.awsSharedCredentialsFile = awsSharedCredentialsFile
 
 	return &clusterHandler{
 		rosaClient:       client,
@@ -477,7 +473,7 @@ func (ch *clusterHandler) GenerateClusterCreateFlags() ([]string, error) {
 			cidrValue = ch.clusterConfig.Networking.MachineCIDR
 		}
 
-		vpc, err = resourcesHandler.PrepareVPC(vpcPrefix, cidrValue, false)
+		vpc, err = resourcesHandler.PrepareVPC(vpcPrefix, cidrValue, false, ch.profile.ClusterConfig.SharedVPC)
 		if err != nil {
 			return flags, err
 		}

tests/utils/handler/interface.go

@@ -65,20 +65,26 @@ type ClusterConfig struct {
 
 // Resources will record the resources prepared
 type Resources struct {
-	AccountRolesPrefix   string `json:"account_roles_prefix,omitempty"`
-	AdditionalPrincipals string `json:"additional_principals,omitempty"`
-	AuditLogArn          string `json:"audit_log,omitempty"`
-	DNSDomain            string `json:"dns_domain,omitempty"`
-	EtcdKMSKey           string `json:"etcd_kms_key,omitempty"`
-	HostedZoneID         string `json:"hosted_zone_id,omitempty"`
-	KMSKey               string `json:"kms_key,omitempty"`
-	OIDCConfigID         string `json:"oidc_config_id,omitempty"`
-	OIDCProviderID       string `json:"oidc_provider_id,omitempty"`
-	OperatorRolesPrefix  string `json:"operator_roles_prefix,omitempty"`
-	Region               string `json:"region,omitempty"`
-	ResourceShareArn     string `json:"resource_share,omitempty"`
-	SharedVPCRole        string `json:"shared_vpc_role,omitempty"`
-	VpcID                string `json:"vpc_id,omitempty"`
+	AccountRolesPrefix   string                `json:"account_roles_prefix,omitempty"`
+	AdditionalPrincipals string                `json:"additional_principals,omitempty"`
+	AuditLogArn          string                `json:"audit_log,omitempty"`
+	DNSDomain            string                `json:"dns_domain,omitempty"`
+	EtcdKMSKey           string                `json:"etcd_kms_key,omitempty"`
+	FromSharedAWSAccount *FromSharedAWSAccount `json:"from_shard_aws_account,omitempty"`
+	HostedZoneID         string                `json:"hosted_zone_id,omitempty"`
+	KMSKey               string                `json:"kms_key,omitempty"`
+	OIDCConfigID         string                `json:"oidc_config_id,omitempty"`
+	OIDCProviderID       string                `json:"oidc_provider_id,omitempty"`
+	OperatorRolesPrefix  string                `json:"operator_roles_prefix,omitempty"`
+	Region               string                `json:"region,omitempty"`
+	ResourceShareArn     string                `json:"resource_share,omitempty"`
+	SharedVPCRole        string                `json:"shared_vpc_role,omitempty"`
+	VpcID                string                `json:"vpc_id,omitempty"`
+}
+
+type FromSharedAWSAccount struct {
+	VPC                 bool `json:"vpc,omitempty"`
+	AdditionalPrincipls bool `json:"additional_principals,omitempty"`
 }
 
 // ClusterDetail will record basic cluster info to support other team's testing

tests/utils/handler/resources_handler.go

@@ -33,7 +33,7 @@ type ResourcesHandler interface {
 
 	PrepareVersion(versionRequirement string, channelGroup string, hcp bool) (*rosacli.OpenShiftVersionTableOutput, error)
 	PreparePrefix(profilePrefix string, nameLength int) string
-	PrepareVPC(vpcName string, cidrValue string, useExisting bool) (*vpc_client.VPC, error)
+	PrepareVPC(vpcName string, cidrValue string, useExisting bool, withSharedAccount bool) (*vpc_client.VPC, error)
 	PrepareSubnets(zones []string, multiZone bool) (map[string][]string, error)
 	PrepareProxy(zone string, sshPemFileName string, sshPemFileRecordDir string, caFile string) (*ProxyDetail, error)
 	PrepareKMSKey(multiRegion bool, testClient string, hcp bool, etcdKMS bool) (string, error)
@@ -57,7 +57,7 @@ type ResourcesHandler interface {
 	PrepareSubnetArns(subnetIDs string) ([]string, error)
 	PrepareResourceShare(resourceShareName string, resourceArns []string) (string, error)
 
-	DeleteVPCChain() error
+	DeleteVPCChain(withSharedAccount bool) error
 	DeleteKMSKey(etcdKMS bool) error
 	DeleteAuditLogRoleArn() error
 	DeleteHostedZone() error
@@ -71,45 +71,49 @@ type ResourcesHandler interface {
 }
 
 type resourcesHandler struct {
-	resources                *Resources
-	persist                  bool
-	rosaClient               *rosacli.Client
-	awsSharedCredentialsFile string
+	resources                       *Resources
+	persist                         bool
+	rosaClient                      *rosacli.Client
+	awsCredentialsFile              string
+	awsSharedAccountCredentialsFile string
 
 	// Optional
 	vpc *vpc_client.VPC
 }
 
 // NewResourcesHandler create a new resources handler with data persisted to Filesystem
 func NewResourcesHandler(client *rosacli.Client, region string,
-	awsSharedCredentialsFile string) (ResourcesHandler, error) {
+	awsCredentialsFile string, awsSharedAccountCredentialsFile string) (ResourcesHandler, error) {
 
-	return newResourcesHandler(client, region, true, false, awsSharedCredentialsFile)
+	return newResourcesHandler(client, region, true, false, awsCredentialsFile, awsSharedAccountCredentialsFile)
 }
 
 // NewTempResourcesHandler create a new resources handler WITHOUT data written to Filesystem
 // Useful for test cases needed resources. Do not forget to delete the resources afterwards
+// awsSharedAccountCredentialsFile is the second AWS account for shared resources
 func NewTempResourcesHandler(client *rosacli.Client, region string,
-	awsSharedCredentialsFile string) (ResourcesHandler, error) {
+	awsCredentialsFile string, awsSharedAccountCredentialsFile string) (ResourcesHandler, error) {
 
-	return newResourcesHandler(client, region, false, false, awsSharedCredentialsFile)
+	return newResourcesHandler(client, region, false, false, awsCredentialsFile, awsSharedAccountCredentialsFile)
 }
 
 // NewResourcesHandlerFromFilesystem create a new resources handler from data saved on Filesystem
 func NewResourcesHandlerFromFilesystem(client *rosacli.Client,
-	awsSharedCredentialsFile string) (ResourcesHandler, error) {
+	awsCredentialsFile string, awsSharedAccountCredentialsFile string) (ResourcesHandler, error) {
 
-	return newResourcesHandler(client, "", true, true, awsSharedCredentialsFile)
+	return newResourcesHandler(client, "", true, true, awsCredentialsFile, awsSharedAccountCredentialsFile)
 }
 
 func newResourcesHandler(client *rosacli.Client, region string, persist bool,
-	loadFilesystem bool, awsSharedCredentialsFile string) (*resourcesHandler, error) {
+	loadFilesystem bool, awsCredentialsFile string,
+	awsSharedAccountCredentialsFile string) (*resourcesHandler, error) {
 
 	resourcesHandler := &resourcesHandler{
-		rosaClient:               client,
-		resources:                &Resources{Region: region},
-		persist:                  persist,
-		awsSharedCredentialsFile: awsSharedCredentialsFile,
+		rosaClient:                      client,
+		resources:                       &Resources{Region: region},
+		persist:                         persist,
+		awsCredentialsFile:              awsCredentialsFile,
+		awsSharedAccountCredentialsFile: awsSharedAccountCredentialsFile,
 	}
 	if loadFilesystem {
 		err := helper.ReadFileContentToObject(config.Test.UserDataFile, &resourcesHandler.resources)
@@ -198,10 +202,10 @@ func (rh *resourcesHandler) DestroyResources() (errors []error) {
 	// delete vpc chain
 	if resources.VpcID != "" {
 		log.Logger.Infof("Find prepared vpc id: %s", resources.VpcID)
-		err = rh.DeleteVPCChain()
+		err = rh.DeleteVPCChain(resources.FromSharedAWSAccount != nil && resources.FromSharedAWSAccount.VPC)
 		success := destroyLog(err, "vpc chain")
 		if success {
-			rh.registerVpcID("")
+			rh.registerVpcID("", false)
 		}
 	}
 	// delete shared vpc role
@@ -334,6 +338,10 @@ func (rh *resourcesHandler) registerAccountRolesPrefix(accountRolesPrefix string
 
 func (rh *resourcesHandler) registerAdditionalPrincipals(additionalPrincipals string) error {
 	rh.resources.AdditionalPrincipals = additionalPrincipals
+	if rh.resources.FromSharedAWSAccount == nil {
+		rh.resources.FromSharedAWSAccount = &FromSharedAWSAccount{}
+	}
+	rh.resources.FromSharedAWSAccount.AdditionalPrincipls = true
 	return rh.saveToFile()
 }
 
@@ -382,8 +390,12 @@ func (rh *resourcesHandler) registerSharedVPCRole(sharedVPCRole string) error {
 	return rh.saveToFile()
 }
 
-func (rh *resourcesHandler) registerVpcID(vpcID string) error {
+func (rh *resourcesHandler) registerVpcID(vpcID string, fromSharedAccount bool) error {
 	rh.resources.VpcID = vpcID
+	if rh.resources.FromSharedAWSAccount == nil {
+		rh.resources.FromSharedAWSAccount = &FromSharedAWSAccount{}
+	}
+	rh.resources.FromSharedAWSAccount.VPC = fromSharedAccount
 	return rh.saveToFile()
 }
 
@@ -396,9 +408,12 @@ func (rh *resourcesHandler) GetVPC() *vpc_client.VPC {
 	return rh.vpc
 }
 
-func (rh *resourcesHandler) GetAWSClient(useSharedVPCIfAvailable bool) (*aws_client.AWSClient, error) {
-	if useSharedVPCIfAvailable && rh.awsSharedCredentialsFile != "" {
-		return aws_client.CreateAWSClient("", rh.resources.Region, rh.awsSharedCredentialsFile)
+func (rh *resourcesHandler) GetAWSClient(useSharedAccount bool) (*aws_client.AWSClient, error) {
+	if useSharedAccount {
+		if rh.awsSharedAccountCredentialsFile == "" {
+			return nil, fmt.Errorf("the shared aws account credential file is empty. Please set it by export ")
+		}
+		return aws_client.CreateAWSClient("", rh.resources.Region, rh.awsSharedAccountCredentialsFile)
 	}
 	return aws_client.CreateAWSClient("", rh.resources.Region)
 }

tests/utils/handler/resources_handler_clean.go

@@ -7,27 +7,29 @@ import (
 	"github.com/openshift-online/ocm-common/pkg/test/kms_key"
 	"github.com/openshift-online/ocm-common/pkg/test/vpc_client"
 
-	ciConfig "github.com/openshift/rosa/tests/ci/config"
 	"github.com/openshift/rosa/tests/utils/log"
 )
 
-func (rh *resourcesHandler) DeleteVPCChain() error {
+func (rh *resourcesHandler) DeleteVPCChain(withSharedAccount bool) error {
 	var err error
+	var awsclient *aws_client.AWSClient
+	awsSharedCredentialFile := rh.awsCredentialsFile
+	if withSharedAccount {
+		awsSharedCredentialFile = rh.awsSharedAccountCredentialsFile
+	}
+	if awsSharedCredentialFile == "" {
+		awsclient, err = aws_client.CreateAWSClient("", rh.resources.Region)
+	} else {
+		awsclient, err = aws_client.CreateAWSClient("", rh.resources.Region, awsSharedCredentialFile)
+	}
+	if err != nil {
+		return err
+	}
 	if rh.vpc == nil {
-		var awsclient *aws_client.AWSClient
-		awsSharedCredentialFile := ciConfig.Test.GlobalENV.SVPC_CREDENTIALS_FILE
-		if awsSharedCredentialFile == "" {
-			awsclient, err = aws_client.CreateAWSClient("", rh.resources.Region)
-		} else {
-			awsclient, err = aws_client.CreateAWSClient("", rh.resources.Region, awsSharedCredentialFile)
-		}
-		if err != nil {
-			return err
-		}
-		VPCclient := vpc_client.NewVPC().AWSclient(awsclient)
-		VPCclient.VpcID = rh.resources.VpcID
-		return VPCclient.DeleteVPCChain(true)
+		rh.vpc = vpc_client.NewVPC()
+		rh.vpc.VpcID = rh.resources.VpcID
 	}
+	rh.vpc.AWSClient = awsclient
 	return rh.vpc.DeleteVPCChain(true)
 }
 
@@ -84,8 +86,8 @@ func (rh *resourcesHandler) DeleteAdditionalPrincipalsRole(managedPolicy bool) e
 	if err != nil {
 		return err
 	}
-
-	err = awsClient.DeleteRoleAndPolicy(rh.resources.AdditionalPrincipals, managedPolicy)
+	roleName := strings.Split(rh.resources.AdditionalPrincipals, "/")[len(strings.Split(rh.resources.AdditionalPrincipals, "/"))-1]
+	err = awsClient.DeleteRoleAndPolicy(roleName, managedPolicy)
 	return err
 }