vSwitch and SecurityGroup query refactor

README.md

@@ -1,4 +1,4 @@
-# OpenShift cluster-api-provider-alibabacloud
+# OpenShift cluster-api-provider-alibaba
 
 This repository hosts an implementation of a provider for AlibabaCloud for the
 OpenShift [machine-api](https://github.com/openshift/cluster-api).

cmd/manager/main.go

@@ -23,6 +23,7 @@ import (
 
 	"github.com/openshift/cluster-api-provider-alibaba/pkg/version"
 
+	"github.com/openshift/machine-api-operator/pkg/controller/machine"
 	"github.com/openshift/machine-api-operator/pkg/metrics"
 
 	"sigs.k8s.io/controller-runtime/pkg/cache"
@@ -35,11 +36,11 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 
 	configv1 "github.com/openshift/api/config/v1"
+	"github.com/openshift/api/machine/v1beta1"
 	machineactuator "github.com/openshift/cluster-api-provider-alibaba/pkg/actuators/machine"
 	machinesetcontroller "github.com/openshift/cluster-api-provider-alibaba/pkg/actuators/machineset"
 	"github.com/openshift/cluster-api-provider-alibaba/pkg/apis"
-	"github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1"
-	"github.com/openshift/machine-api-operator/pkg/controller/machine"
+
 	"k8s.io/klog/v2"
 	"k8s.io/klog/v2/klogr"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -53,7 +54,7 @@ var (
 	leaseDuration    = 120 * time.Second
 	renewDealine     = 110 * time.Second
 	retryPeriod      = 20 * time.Second
-	leaderElectionID = "cluster-api-provider-alibabacloud-leader"
+	leaderElectionID = "cluster-api-provider-alibaba-leader"
 )
 
 func main() {
@@ -111,7 +112,7 @@ func main() {
 
 	// print release version
 	if printVersion != nil && *printVersion {
-		klog.Infof("The cluster-api-provider-alibabacloud version {%s}", version.PrintVerboseVersionInfo())
+		klog.Infof("The cluster-api-provider-alibaba version {%s}", version.PrintVerboseVersionInfo())
 	}
 
 	cfg := config.GetConfigOrDie()

go.mod

@@ -7,23 +7,18 @@ require (
 	github.com/blang/semver v3.5.1+incompatible
 	github.com/go-logr/logr v0.4.0
 	github.com/golang/mock v1.6.0
-	github.com/onsi/gomega v1.10.5
-	github.com/openshift/api v0.0.0-20210416115537-a60c0dc032fd
-	github.com/openshift/machine-api-operator v0.2.1-0.20210504014029-a132ec00f7dd
+	github.com/onsi/gomega v1.14.0
+	github.com/openshift/api v0.0.0-20211108165917-be1be0e89115
+	github.com/openshift/machine-api-operator v0.2.1-0.20211102083422-ee77ca7b9fd1
 	github.com/stretchr/testify v1.7.0
 
 	// kube 1.18
-	k8s.io/api v0.21.0
-	k8s.io/apimachinery v0.21.0
-	k8s.io/client-go v0.21.0
+	k8s.io/api v0.22.1
+	k8s.io/apimachinery v0.22.1
+	k8s.io/client-go v0.22.1
 	k8s.io/klog v1.0.0
-	k8s.io/klog/v2 v2.8.0
-	sigs.k8s.io/controller-runtime v0.9.0-beta.1.0.20210512131817-ce2f0c92d77e
-	sigs.k8s.io/controller-tools v0.3.0
+	k8s.io/klog/v2 v2.9.0
+	sigs.k8s.io/controller-runtime v0.9.6
+	sigs.k8s.io/controller-tools v0.6.3-0.20210916130746-94401651a6c3
 	sigs.k8s.io/yaml v1.2.0
 )
-
-replace (
-	sigs.k8s.io/cluster-api-provider-aws => github.com/openshift/cluster-api-provider-aws v0.2.1-0.20210420175812-638f9f3fbb42
-	sigs.k8s.io/cluster-api-provider-azure => github.com/openshift/cluster-api-provider-azure v0.1.0-alpha.3.0.20210408182022-987bc3d6a107
-)

pkg/actuators/machine/actuator.go

@@ -19,9 +19,8 @@ package machine
 import (
 	"context"
 
+	machinev1 "github.com/openshift/api/machine/v1beta1"
 	alibabacloudClient "github.com/openshift/cluster-api-provider-alibaba/pkg/client"
-
-	machinev1 "github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1"
 	machineapierrors "github.com/openshift/machine-api-operator/pkg/controller/machine"
 
 	corev1 "k8s.io/api/core/v1"

pkg/actuators/machine/actuator_test.go

@@ -38,7 +38,7 @@ import (
 	"k8s.io/client-go/tools/record"
 
 	configv1 "github.com/openshift/api/config/v1"
-	machinev1 "github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1"
+	machinev1 "github.com/openshift/api/machine/v1beta1"
 	"k8s.io/client-go/kubernetes/scheme"
 )
 

pkg/actuators/machine/filter.go

@@ -27,7 +27,7 @@ const (
 	clusterFilterValue     = "owned"
 	clusterFilterName      = "Name"
 	clusterOwnedKey        = "kubernetes-sigs/cluster-api"
-	clusterOwnedValue      = "cluster-api-provider-alibabacloud"
+	clusterOwnedValue      = "cluster-api-provider-alibaba"
 )
 
 func clusterTagFilter(clusterID, machineName string) []ecs.DescribeInstancesTag {

pkg/actuators/machine/instances.go

@@ -18,6 +18,7 @@ package machine
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"sort"
 	"strconv"
@@ -32,9 +33,9 @@ import (
 	"github.com/aliyun/alibaba-cloud-sdk-go/services/ecs"
 	mapierrors "github.com/openshift/machine-api-operator/pkg/controller/machine"
 
+	machinev1 "github.com/openshift/api/machine/v1beta1"
 	alibabacloudproviderv1 "github.com/openshift/cluster-api-provider-alibaba/pkg/apis/alibabacloudprovider/v1beta1"
 	alibabacloudClient "github.com/openshift/cluster-api-provider-alibaba/pkg/client"
-	machinev1 "github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1"
 	"github.com/openshift/machine-api-operator/pkg/metrics"
 	runtimeclient "sigs.k8s.io/controller-runtime/pkg/client"
 )
@@ -247,7 +248,6 @@ func runInstances(machine *machinev1.Machine, machineProviderConfig *alibabaclou
 			alibabacloudproviderv1.DefaultTenancy,
 			alibabacloudproviderv1.HostTenancy)
 	}
-
 	runResponse, err := client.RunInstances(runInstancesRequest)
 	if err != nil {
 		metrics.RegisterFailedInstanceCreate(&metrics.MachineLabels{
@@ -373,57 +373,62 @@ func getImageID(machine runtimeclient.ObjectKey, machineProviderConfig *alibabac
 }
 
 func getSecurityGroupIDs(machine runtimeclient.ObjectKey, machineProviderConfig *alibabacloudproviderv1.AlibabaCloudMachineProviderConfig, client alibabacloudClient.Client) (*[]string, error) {
-	klog.Infof("%s validate security group in region %s", machineProviderConfig.SecurityGroupID, machineProviderConfig.RegionID)
+	klog.Infof("query security groups in region %s", machineProviderConfig.RegionID)
 	var securityGroupIDs []string
 
 	// If SecurityGroupID is assigned, use it directly
-	if machineProviderConfig.SecurityGroupID != "" {
-		securityGroupIDs = append(securityGroupIDs, machineProviderConfig.SecurityGroupID)
-	} else {
-		// Otherwise, the query securityGroupIDs by the tags
-		for _, sg := range machineProviderConfig.SecurityGroups {
-			if sg.ID != "" {
-				securityGroupIDs = append(securityGroupIDs, sg.ID)
-			} else {
-				if sg.Tags != nil {
-					request := ecs.CreateDescribeSecurityGroupsRequest()
-					if machineProviderConfig.VpcID != "" {
-						request.VpcId = machineProviderConfig.VpcID
-					}
-					if machineProviderConfig.ResourceGroupID != "" {
-						request.ResourceGroupId = machineProviderConfig.ResourceGroupID
-					}
-					request.RegionId = machineProviderConfig.RegionID
-					request.Tag = buildDescribeSecurityGroupsTag(sg.Tags)
-					request.Scheme = "https"
-
-					response, err := client.DescribeSecurityGroups(request)
-					if err != nil {
-						metrics.RegisterFailedInstanceCreate(&metrics.MachineLabels{
-							Name:      machine.Name,
-							Namespace: machine.Namespace,
-							Reason:    err.Error(),
-						})
-						klog.Errorf("error describing securitygroup: %v", err)
-						return nil, fmt.Errorf("error describing securitygroup: %v", err)
-					}
-
-					if len(response.SecurityGroups.SecurityGroup) < 1 {
-						klog.Errorf("no securitygroup for given tags not found")
-						return nil, fmt.Errorf("no securitygroup for given tags not found")
-					}
-
-					for _, sg := range response.SecurityGroups.SecurityGroup {
-						securityGroupIDs = append(securityGroupIDs, sg.SecurityGroupId)
-					}
+	if len(machineProviderConfig.SecurityGroups) == 0 {
+		return nil, errors.New("no security configuration provided")
+	}
+
+	for _, sg := range machineProviderConfig.SecurityGroups {
+		if sg.ID != "" {
+			securityGroupIDs = append(securityGroupIDs, sg.ID)
+		} else {
+			if sg.Tags != nil {
+				ids, err := getSecurityGroupIDByTags(machine, machineProviderConfig, sg.Tags, client)
+				if err != nil {
+					return nil, err
 				}
+				securityGroupIDs = append(securityGroupIDs, ids...)
 			}
 		}
 	}
-
+	if len(securityGroupIDs) == 0 {
+		return nil, errors.New("no securitygroup IDs found from configuration")
+	}
 	return &securityGroupIDs, nil
 }
 
+func getSecurityGroupIDByTags(machine runtimeclient.ObjectKey, machineProviderConfig *alibabacloudproviderv1.AlibabaCloudMachineProviderConfig, tags []alibabacloudproviderv1.Tag, client alibabacloudClient.Client) ([]string, error) {
+	request := ecs.CreateDescribeSecurityGroupsRequest()
+	request.VpcId = machineProviderConfig.VpcID
+	request.ResourceGroupId = machineProviderConfig.ResourceGroupID
+	request.RegionId = machineProviderConfig.RegionID
+	request.Tag = buildDescribeSecurityGroupsTag(tags)
+	request.Scheme = "https"
+
+	response, err := client.DescribeSecurityGroups(request)
+	if err != nil {
+		metrics.RegisterFailedInstanceCreate(&metrics.MachineLabels{
+			Name:      machine.Name,
+			Namespace: machine.Namespace,
+			Reason:    err.Error(),
+		})
+		klog.Errorf("error describing securitygroup: %v", err)
+		return nil, fmt.Errorf("error describing securitygroup: %v", err)
+	}
+	if len(response.SecurityGroups.SecurityGroup) < 1 {
+		klog.Errorf("no securitygroup for given tags not found")
+		return nil, fmt.Errorf("no securitygroup for given tags not found")
+	}
+	securityGroupIDs := []string{}
+	for _, sg := range response.SecurityGroups.SecurityGroup {
+		securityGroupIDs = append(securityGroupIDs, sg.SecurityGroupId)
+	}
+	return securityGroupIDs, nil
+}
+
 func getMaxInstancesBySecurityGroupType(securityGroupType string) int {
 	switch securityGroupType {
 	case SecurityGroupTypeNormal:
@@ -450,51 +455,43 @@ func buildDescribeSecurityGroupsTag(tags []alibabacloudproviderv1.Tag) *[]ecs.De
 
 func getVSwitchID(machine runtimeclient.ObjectKey, machineProviderConfig *alibabacloudproviderv1.AlibabaCloudMachineProviderConfig, client alibabacloudClient.Client) (string, error) {
 	klog.Infof("validate vswitch in region %s", machineProviderConfig.RegionID)
-	vSwitchID := ""
-	if machineProviderConfig.VSwitchID != "" {
-		vSwitchID = machineProviderConfig.VSwitchID
+	if machineProviderConfig.VSwitch.ID == "" && len(machineProviderConfig.VSwitch.Tags) == 0 {
+		return "", errors.New("no vswitch configuration provided")
 	}
 
-	if machineProviderConfig.VSwitch != nil {
-		if machineProviderConfig.VSwitch.ID != "" {
-			vSwitchID = machineProviderConfig.VSwitch.ID
-		} else {
-			if machineProviderConfig.VSwitch.Tags != nil {
-				describeVSwitchesRequest := vpc.CreateDescribeVSwitchesRequest()
-				describeVSwitchesRequest.Scheme = "https"
-
-				describeVSwitchesRequest.RegionId = machineProviderConfig.RegionID
-				if machineProviderConfig.VpcID != "" {
-					describeVSwitchesRequest.VpcId = machineProviderConfig.VpcID
-				}
-				describeVSwitchesRequest.Tag = buildDescribeVSwitchesTag(machineProviderConfig.VSwitch.Tags)
+	if machineProviderConfig.VSwitch.ID != "" {
+		return machineProviderConfig.VSwitch.ID, nil
+	}
 
-				describeVSwitchesResponse, err := client.DescribeVSwitches(describeVSwitchesRequest)
-				if err != nil {
-					metrics.RegisterFailedInstanceCreate(&metrics.MachineLabels{
-						Name:      machine.Name,
-						Namespace: machine.Namespace,
-						Reason:    err.Error(),
-					})
-					klog.Errorf("error describing vswitches: %v", err)
-					return "", fmt.Errorf("error describing vswitches: %v", err)
-				}
+	if machineProviderConfig.VSwitch.Tags != nil {
+		return getVSwitchIDFromTags(machine, machineProviderConfig, client)
+	}
 
-				if len(describeVSwitchesResponse.VSwitches.VSwitch) < 1 {
-					klog.Errorf("no vswitches for given tags not found")
-					return "", fmt.Errorf("no vswitches for given tags not found")
-				}
+	return "", fmt.Errorf("no vSwitch found from configuration")
+}
 
-				vSwitchID = describeVSwitchesResponse.VSwitches.VSwitch[0].VSwitchId
-			}
-		}
+func getVSwitchIDFromTags(machine runtimeclient.ObjectKey, mpc *alibabacloudproviderv1.AlibabaCloudMachineProviderConfig, client alibabacloudClient.Client) (string, error) {
+	// Build a request to fetch the vSwitchID from the tags provided
+	describeVSwitchesRequest := vpc.CreateDescribeVSwitchesRequest()
+	describeVSwitchesRequest.Scheme = "https"
+	describeVSwitchesRequest.RegionId = mpc.RegionID
+	describeVSwitchesRequest.VpcId = mpc.VpcID
+	describeVSwitchesRequest.Tag = buildDescribeVSwitchesTag(mpc.VSwitch.Tags)
+	describeVSwitchesResponse, err := client.DescribeVSwitches(describeVSwitchesRequest)
+	if err != nil {
+		metrics.RegisterFailedInstanceCreate(&metrics.MachineLabels{
+			Name:      machine.Name,
+			Namespace: machine.Namespace,
+			Reason:    err.Error(),
+		})
+		klog.Errorf("error describing vswitches: %v", err)
+		return "", fmt.Errorf("error describing vswitches: %v", err)
 	}
-
-	if vSwitchID == "" {
-		return "", fmt.Errorf("no vswitches were found")
+	if len(describeVSwitchesResponse.VSwitches.VSwitch) < 1 {
+		klog.Errorf("no vswitches found for given tags, vpcid, and regionid")
+		return "", fmt.Errorf("no vswitches found for given tags, vpcid, and regionid")
 	}
-
-	return vSwitchID, nil
+	return describeVSwitchesResponse.VSwitches.VSwitch[0].VSwitchId, nil
 }
 
 func buildDescribeVSwitchesTag(tags []alibabacloudproviderv1.Tag) *[]vpc.DescribeVSwitchesTag {

pkg/actuators/machine/machine_scope.go

@@ -27,10 +27,9 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/klog"
 
-	alibabacloudClient "github.com/openshift/cluster-api-provider-alibaba/pkg/client"
-
+	machinev1 "github.com/openshift/api/machine/v1beta1"
 	v1beta1 "github.com/openshift/cluster-api-provider-alibaba/pkg/apis/alibabacloudprovider/v1beta1"
-	machinev1 "github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1"
+	alibabacloudClient "github.com/openshift/cluster-api-provider-alibaba/pkg/client"
 	machineapierros "github.com/openshift/machine-api-operator/pkg/controller/machine"
 	runtimeclient "sigs.k8s.io/controller-runtime/pkg/client"
 )

pkg/actuators/machine/stubs.go

@@ -9,7 +9,7 @@ import (
 
 	"github.com/aliyun/alibaba-cloud-sdk-go/services/ecs"
 
-	machinev1 "github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1"
+	machinev1 "github.com/openshift/api/machine/v1beta1"
 	machinecontroller "github.com/openshift/machine-api-operator/pkg/controller/machine"
 
 	alibabacloudproviderv1 "github.com/openshift/cluster-api-provider-alibaba/pkg/apis/alibabacloudprovider/v1beta1"
@@ -34,7 +34,7 @@ const (
 	stubVpcID                   = "vpc-3ze4u29pd4lniym7i1xnp"
 	stubVSwitchID               = "vsw-7ze567qrl5das7q8s4rei"
 	stubInstanceID              = "i-2ze3hj0qh9d290rpax7w"
-	stubSecurityGroupId         = "sg-2zeebk9qd965vc2xqq4w"
+	stubSecurityGroupID         = "sg-2zeebk9qd965vc2xqq4w"
 	stubSystemDiskCategory      = "cloud_essd"
 	stubSystemDiskSize          = 120
 	stubInternetMaxBandwidthOut = 100
@@ -57,13 +57,15 @@ func stubAlibabaCloudCredentialsSecret() *corev1.Secret {
 
 func stubProviderConfig() *alibabacloudproviderv1.AlibabaCloudMachineProviderConfig {
 	return &alibabacloudproviderv1.AlibabaCloudMachineProviderConfig{
-		InstanceType:            stubInstanceType,
-		ImageID:                 stubImageID,
-		RegionID:                stubRegionID,
-		ZoneID:                  stubZoneID,
-		SecurityGroupID:         stubSecurityGroupId,
+		InstanceType: stubInstanceType,
+		ImageID:      stubImageID,
+		RegionID:     stubRegionID,
+		ZoneID:       stubZoneID,
+		SecurityGroups: []alibabacloudproviderv1.AlibabaResourceReference{
+			{ID: stubVSwitchID},
+		},
 		VpcID:                   stubVpcID,
-		VSwitchID:               stubVSwitchID,
+		VSwitch:                 alibabacloudproviderv1.AlibabaResourceReference{ID: stubVSwitchID},
 		SystemDiskCategory:      stubSystemDiskCategory,
 		SystemDiskSize:          stubSystemDiskSize,
 		InternetMaxBandwidthOut: stubInternetMaxBandwidthOut,
@@ -143,7 +145,7 @@ func stubRunInstancesRequest() *ecs.RunInstancesRequest {
 	request.InstanceType = stubInstanceType
 	request.ImageId = stubImageID
 	request.VSwitchId = stubVSwitchID
-	request.SecurityGroupId = stubSecurityGroupId
+	request.SecurityGroupId = stubSecurityGroupID
 	request.Password = stubPassword
 	request.MinAmount = requests.NewInteger(1)
 	request.Amount = requests.NewInteger(1)

pkg/actuators/machine/utils.go

@@ -4,8 +4,8 @@ import (
 	"fmt"
 	"time"
 
+	machinev1 "github.com/openshift/api/machine/v1beta1"
 	alibabacloudproviderv1 "github.com/openshift/cluster-api-provider-alibaba/pkg/apis/alibabacloudprovider/v1beta1"
-	machinev1 "github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1"
 	machinecontroller "github.com/openshift/machine-api-operator/pkg/controller/machine"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"