From 8a5c19afe5ba38537615c2298167f6827389946d Mon Sep 17 00:00:00 2001 From: Xue Li Date: Thu, 18 Apr 2024 16:19:39 +0800 Subject: [PATCH] OCM-7517 | test: fix vpc client meet nil pointer when create subnets pair remove the testing code Enhance Fix failure Fix golint check --- pkg/aws/aws_client/subnet.go | 15 ++++++--------- pkg/aws/aws_client/vpc.go | 10 +++++----- pkg/test/vpc_client/cidr.go | 7 ++++--- pkg/test/vpc_client/nat_gateway.go | 4 ++++ pkg/test/vpc_client/proxy.go | 5 +++-- pkg/test/vpc_client/subnet.go | 14 +++++--------- pkg/test/vpc_client/vpc.go | 22 +++++++++++++++------- 7 files changed, 42 insertions(+), 35 deletions(-) diff --git a/pkg/aws/aws_client/subnet.go b/pkg/aws/aws_client/subnet.go index ce8fb11..f7e462a 100644 --- a/pkg/aws/aws_client/subnet.go +++ b/pkg/aws/aws_client/subnet.go @@ -7,21 +7,18 @@ import ( "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/service/ec2" "github.com/aws/aws-sdk-go-v2/service/ec2/types" - CON "github.com/openshift-online/ocm-common/pkg/aws/consts" "github.com/openshift-online/ocm-common/pkg/log" ) -func (client *AWSClient) CreateSubnet(vpcID string, region string, zone string, subnetCidr string) (*types.Subnet, error) { - if region == "" { - region = CON.DefaultAWSRegion - } +func (client *AWSClient) CreateSubnet(vpcID string, zone string, subnetCidr string) (*types.Subnet, error) { + if zone == "" { - zone = CON.DefaultAWSZone + return nil, fmt.Errorf("zone must be not empty for subnet creation") } input := &ec2.CreateSubnetInput{ VpcId: aws.String(vpcID), - AvailabilityZone: aws.String(fmt.Sprintf(region + zone)), + AvailabilityZone: aws.String(zone), AvailabilityZoneId: nil, CidrBlock: aws.String(subnetCidr), DryRun: nil, @@ -64,10 +61,10 @@ func (client *AWSClient) DeleteSubnet(subnetID string) (*ec2.DeleteSubnetOutput, resp, err := client.Ec2Client.DeleteSubnet(context.TODO(), input) if err != nil { - log.LogError("Delete subnet error " + err.Error()) + log.LogError("Delete subnet %s meets error %s", subnetID, err.Error()) return nil, err } - log.LogInfo("Delete subnet success " + subnetID) + log.LogInfo("Delete subnet %s successfully ", subnetID) return resp, err } diff --git a/pkg/aws/aws_client/vpc.go b/pkg/aws/aws_client/vpc.go index 026118d..83eb759 100644 --- a/pkg/aws/aws_client/vpc.go +++ b/pkg/aws/aws_client/vpc.go @@ -15,7 +15,7 @@ func (client *AWSClient) ListVPCByName(vpcName string) ([]types.Vpc, error) { vpcs := []types.Vpc{} filterKey := "tag:Name" filter := []types.Filter{ - types.Filter{ + { Name: &filterKey, Values: []string{vpcName}, }, @@ -91,7 +91,7 @@ func (client *AWSClient) ModifyVpcDnsAttribute(vpcID string, dnsAttribute string log.LogError("Modify vpc dns attribute failed " + err.Error()) return nil, err } - log.LogInfo("Modify vpc dns attribute %s success for %s", dnsAttribute, vpcID) + log.LogInfo("Modify vpc dns attribute %s successfully for %s", dnsAttribute, vpcID) return resp, err } @@ -103,10 +103,10 @@ func (client *AWSClient) DeleteVpc(vpcID string) (*ec2.DeleteVpcOutput, error) { resp, err := client.Ec2Client.DeleteVpc(context.TODO(), input) if err != nil { - log.LogError("Delete vpc %s failed "+err.Error(), vpcID) + log.LogError("Delete vpc %s failed with error %s", vpcID, err.Error()) return nil, err } - log.LogInfo("Delete vpc success " + vpcID) + log.LogInfo("Delete vpc %s successfuly ", vpcID) return resp, err } @@ -127,7 +127,7 @@ func (client *AWSClient) DescribeVPC(vpcID string) (types.Vpc, error) { func (client *AWSClient) ListEndpointAssociation(vpcID string) ([]types.VpcEndpoint, error) { vpcFilterKey := "vpc-id" filters := []types.Filter{ - types.Filter{ + { Name: &vpcFilterKey, Values: []string{vpcID}, }, diff --git a/pkg/test/vpc_client/cidr.go b/pkg/test/vpc_client/cidr.go index 9b4af40..852512d 100644 --- a/pkg/test/vpc_client/cidr.go +++ b/pkg/test/vpc_client/cidr.go @@ -24,9 +24,11 @@ func NewCIDRPool(vpcCIDR string) *VPCCIDRPool { func (v *VPCCIDRPool) GenerateSubnetPool(prefix int) { subnetcidrs := []*SubnetCIDR{} _, vpcSubnet, _ := net.ParseCIDR(v.CIDR) - currentSubnet, finished := cidr.PreviousSubnet(vpcSubnet, prefix) + currentSubnet, _ := cidr.PreviousSubnet(vpcSubnet, prefix) + var loopFinished bool for { - if !finished && vpcSubnet.Contains(currentSubnet.IP) { + currentSubnet, loopFinished = cidr.NextSubnet(currentSubnet, prefix) + if !loopFinished && vpcSubnet.Contains(currentSubnet.IP) { subnetcidr := SubnetCIDR{ IPNet: currentSubnet, CIDR: currentSubnet.String(), @@ -35,7 +37,6 @@ func (v *VPCCIDRPool) GenerateSubnetPool(prefix int) { } else { break } - currentSubnet, finished = cidr.NextSubnet(currentSubnet, prefix) } v.SubNetPool = subnetcidrs } diff --git a/pkg/test/vpc_client/nat_gateway.go b/pkg/test/vpc_client/nat_gateway.go index 9994074..f4eb700 100644 --- a/pkg/test/vpc_client/nat_gateway.go +++ b/pkg/test/vpc_client/nat_gateway.go @@ -2,9 +2,12 @@ package vpc_client import ( "sync" + + "github.com/openshift-online/ocm-common/pkg/log" ) func (vpc *VPC) DeleteVPCNatGateways(vpcID string) error { + var delERR error natGateways, err := vpc.AWSClient.ListNatGateWays(vpcID) if err != nil { @@ -12,6 +15,7 @@ func (vpc *VPC) DeleteVPCNatGateways(vpcID string) error { } var wg sync.WaitGroup for _, ngw := range natGateways { + log.LogInfo("Deleting nat gateway %s", *ngw.NatGatewayId) wg.Add(1) go func(gateWayID string) { defer wg.Done() diff --git a/pkg/test/vpc_client/proxy.go b/pkg/test/vpc_client/proxy.go index bc736b6..6d5094b 100644 --- a/pkg/test/vpc_client/proxy.go +++ b/pkg/test/vpc_client/proxy.go @@ -12,6 +12,7 @@ import ( // LaunchProxyInstance will launch a proxy instance on the indicated zone. // If set imageID to empty, it will find the proxy image in the ProxyImageMap map +// LaunchProxyInstance will return proxyInstance detail, privateIPAddress,CAcontent and error func (vpc *VPC) LaunchProxyInstance(imageID string, zone string, sshKey string) (types.Instance, string, string, error) { var inst types.Instance if imageID == "" { @@ -102,7 +103,7 @@ func (vpc *VPC) CopyImageToProxy(name string) (destinationImageID string, err er } func (vpc *VPC) WaitImageToActive(imageID string, timeout time.Duration) (imageAvailable bool, err error) { - + log.LogInfo("Waiting for image %s status to active. Timeout after %v mins", imageID, timeout) startTime := time.Now() imageAvailable = false for time.Now().Before(startTime.Add(timeout * time.Minute)) { @@ -115,7 +116,7 @@ func (vpc *VPC) WaitImageToActive(imageID string, timeout time.Duration) (imageA imageAvailable = true return imageAvailable, nil } - log.LogInfo("Wait for image %s status to active", imageID) + time.Sleep(time.Minute) } err = fmt.Errorf("timeout for waiting image active") diff --git a/pkg/test/vpc_client/subnet.go b/pkg/test/vpc_client/subnet.go index 9289e95..d4f5ab7 100644 --- a/pkg/test/vpc_client/subnet.go +++ b/pkg/test/vpc_client/subnet.go @@ -230,15 +230,11 @@ func (vpc *VPC) CreatePairSubnet(zone string) (*VPC, []*Subnet, error) { // Otherwise it will create a pair. // If single one missing, it will create another one based on the zone func (vpc *VPC) PreparePairSubnetByZone(zone string) (map[string]*Subnet, error) { - log.LogInfo("Going to prepare") + log.LogInfo("Going to prepare proper pair of subnets") result := map[string]*Subnet{} for _, subnet := range vpc.SubnetList { - trimedSubnetZone := strings.Split(subnet.Zone, vpc.Region)[1] // Cannot use Trim because it will remove the whole string if the zone is ap-northeast-1a and region is ap-northeast-1 - if trimedSubnetZone == "" { - log.LogError("Got empty trimed zone. But the subnet zone is: %s, vpc region is: %s", subnet.Zone, vpc.Region) - } - log.LogInfo("Subnet %s in zone: %s, and trimed zone %s and region %s", subnet.Name, subnet.Zone, trimedSubnetZone, vpc.Region) - if trimedSubnetZone == zone { + log.LogInfo("Subnet %s in zone: %s, region %s", subnet.ID, subnet.Zone, vpc.Region) + if subnet.Zone == zone { if subnet.Private { if _, ok := result["private"]; !ok { log.LogInfo("Got private subnet %s and set it to the result", subnet.ID) @@ -316,7 +312,7 @@ func (vpc *VPC) CreateSubnet(zone string) (*Subnet, error) { } subnetcidr := vpc.CIDRPool.Allocate().CIDR - respCreateSubnet, err := vpc.AWSClient.CreateSubnet(vpc.VpcID, vpc.Region, zone, subnetcidr) + respCreateSubnet, err := vpc.AWSClient.CreateSubnet(vpc.VpcID, zone, subnetcidr) if err != nil { log.LogError("create subnet error " + err.Error()) return nil, err @@ -332,7 +328,7 @@ func (vpc *VPC) CreateSubnet(zone string) (*Subnet, error) { subnet := &Subnet{ ID: *respCreateSubnet.SubnetId, Private: true, - Zone: fmt.Sprintf(vpc.Region + zone), + Zone: zone, Cidr: subnetcidr, Region: vpc.Region, VpcID: vpc.VpcID, diff --git a/pkg/test/vpc_client/vpc.go b/pkg/test/vpc_client/vpc.go index cd0e138..0689cfc 100644 --- a/pkg/test/vpc_client/vpc.go +++ b/pkg/test/vpc_client/vpc.go @@ -19,7 +19,7 @@ func (vpc *VPC) GenerateVPCBySubnet(subnetID string) (*VPC, error) { return nil, err } log.LogInfo("Subnet info loaded from AWS by subnet: %s", subnetID) - vpc, err = GenerateVPCByID(vpc.AWSClient, *subnetDetail[0].VpcId) + vpc, err = GenerateVPCByID(*subnetDetail[0].VpcId, vpc.Region) log.LogInfo("VPC info loaded from AWS by subnet: %s", subnetID) return vpc, err } @@ -82,7 +82,7 @@ func (vpc *VPC) DeleteVPCChain(totalClean ...bool) error { log.LogError("Delete vpc proxy security group meets error: %s", err.Error()) return err } - log.LogInfo("Delete vpc vpc proxy security group successfully") + log.LogInfo("Delete vpc proxy security group successfully") err = vpc.DeleteVPCRouteTables(vpcID) if err != nil { @@ -136,7 +136,7 @@ func (vpc *VPC) DeleteVPCChain(totalClean ...bool) error { _, err = vpc.AWSClient.DeleteVpc(vpc.VpcID) if err != nil { - log.LogError("Delete vpc meets error: %s", err.Error()) + log.LogError("Delete vpc %s meets error: %s", vpc.VpcID, err.Error()) return err } return nil @@ -176,7 +176,7 @@ func PrepareVPC(vpcName string, region string, vpcCIDR string, checkExisting boo vpcID := *vpcs[0].VpcId log.LogInfo("Got a vpc %s with name %s on region %s. Just load it for usage", vpcID, vpcName, region) - vpc, err := GenerateVPCByID(awsclient, vpcID) + vpc, err := GenerateVPCByID(vpcID, region) if err != nil { log.LogError("Load vpc %s details meets error %s", vpcID, err.Error()) @@ -217,7 +217,11 @@ func PrepareVPC(vpcName string, region string, vpcCIDR string, checkExisting boo // GenerateVPCByID will return a VPC with CIDRpool and subnets // If you know the vpc ID on AWS, then try to generate it -func GenerateVPCByID(awsClient *aws_client.AWSClient, vpcID string) (*VPC, error) { +func GenerateVPCByID(vpcID string, region string) (*VPC, error) { + awsClient, err := aws_client.CreateAWSClient("", region) + if err != nil { + return nil, err + } vpc := NewVPC().AWSclient(awsClient).ID(vpcID) vpcResp, err := vpc.AWSClient.DescribeVPC(vpcID) if err != nil { @@ -246,11 +250,15 @@ func GenerateVPCByID(awsClient *aws_client.AWSClient, vpcID string) (*VPC, error // GenerateVPCBySubnet will return a VPC with CIDRpool and subnets based on one of the subnet ID // If you know the subnet ID on AWS, then try to generate it on AWS. -func GenerateVPCBySubnet(awsClient *aws_client.AWSClient, subnetID string) (*VPC, error) { +func GenerateVPCBySubnet(subnetID string, region string) (*VPC, error) { + awsClient, err := aws_client.CreateAWSClient("", region) + if err != nil { + return nil, err + } subnetDetail, err := awsClient.ListSubnetDetail(subnetID) if err != nil { return nil, err } - vpc, err := GenerateVPCByID(awsClient, *subnetDetail[0].VpcId) + vpc, err := GenerateVPCByID(*subnetDetail[0].VpcId, region) return vpc, err }