Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,7 @@
.idea/
bin/
examples/secret.yaml
config/bootstrap-master.yaml
config/bootstrap-worker.yaml


2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ bin:
##@ Development
CONTROLLER_GEN = $(shell pwd)/bin/controller-gen
controller-gen: ## Download controller-gen locally if necessary.
$(call go-get-tool,$(CONTROLLER_GEN),sigs.k8s.io/controller-tools/cmd/controller-gen@v0.4.1)
$(call go-get-tool,$(CONTROLLER_GEN),sigs.k8s.io/controller-tools/cmd/controller-gen@v0.7.0)


deepcopy: controller-gen ## Generate code containing DeepCopy, DeepCopyInto, and DeepCopyObject method implementations.
Expand Down
52 changes: 52 additions & 0 deletions config/master-machine-with-tags.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
---
apiVersion: machine.openshift.io/v1beta1
kind: Machine
metadata:
name: master-machine
namespace: default
labels:
machine.openshift.io/cluster-api-cluster: alibabacloud-actuator-k8s
spec:
metadata:
labels:
node-role.kubernetes.io/master: ""
providerSpec:
value:
apiVersion: alibabacloudproviderconfig.openshift.io/v1alpha1
kind: AlibabaCloudMachineProviderConfig
instanceType: FILLIN
imageId: FILLIN
regionId: FILLIN
zoneId: FILLIN
securityGroups:
- tags:
- key: Name
value: ocp-test-sg
- key: "kubernetes.io/cluster/abc-def"
value: owned
- key: OCP
value: ISVAlibaba
vpcId: FILLIN
vSwitch:
tags:
- key: Name
value: ocp-test-vswitch
- key: "kubernetes.io/cluster/abc-def"
value: owned
- key: OCP
value: ISVAlibaba
systemDiskCategory: FILLIN
systemDiskSize: FILLIN
internetMaxBandwidthOut: FILLIN
password: FILLIN
tags:
- key: openshift-node-group-config
value: node-config-node
- key: host-type
value: node
- key: sub-host-type
value: default
userDataSecret:
name: master-user-data-secret
credentialsSecret:
name: alibabacloud-credentials-secret
52 changes: 52 additions & 0 deletions config/worker-machine-with-tags.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
---
apiVersion: machine.openshift.io/v1beta1
kind: Machine
metadata:
name: worker-machine
namespace: default
labels:
machine.openshift.io/cluster-api-cluster: alicloud-actuator-k8s
spec:
metadata:
labels:
node-role.kubernetes.io/infra: ""
providerSpec:
value:
apiVersion: alicloudproviderconfig.openshift.io/v1alpha1
kind: AlibabaCloudMachineProviderConfig
instanceType: FILLIN
imageId: FILLIN
regionId: FILLIN
zoneId: FILLIN
securityGroups:
- tags:
- key: Name
value: ocp-test-sg
- key: "kubernetes.io/cluster/abc-def"
value: owned
- key: OCP
value: ISVAlibaba
vpcId: FILLIN
vSwitch:
tags:
- key: Name
value: ocp-test-vswitch
- key: "kubernetes.io/cluster/abc-def"
value: owned
- key: OCP
value: ISVAlibaba
systemDiskCategory: FILLIN
systemDiskSize: FILLIN
internetMaxBandwidthOut: FILLIN
password: FILLIN
tags:
- key: openshift-node-group-config
value: node-config-node
- key: host-type
value: node
- key: sub-host-type
value: default
userDataSecret:
name: worker-user-data-secret
credentialsSecret:
name: alibabacloud-credentials-secret
189 changes: 138 additions & 51 deletions pkg/actuators/machine/instances.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import (
"strings"
"time"

"github.com/aliyun/alibaba-cloud-sdk-go/services/vpc"

"k8s.io/klog"

"github.com/aliyun/alibaba-cloud-sdk-go/sdk/requests"
Expand Down Expand Up @@ -85,12 +87,18 @@ func runInstances(machine *machinev1.Machine, machineProviderConfig *alibabaclou
return nil, mapierrors.InvalidMachineConfiguration("error getting ImageID: %v", err)
}

// SecurgityGroupId
securityGroupID, err := getSecurityGroupID(machineKey, machineProviderConfig, client)
// SecurgityGroupIds
securityGroupIDs, err := getSecurityGroupIDs(machineKey, machineProviderConfig, client)
if err != nil {
return nil, mapierrors.InvalidMachineConfiguration("error getting security groups ID: %v", err)
}

// VSwitchID
vSwitchID, err := getVSwitchID(machineKey, machineProviderConfig, client)
if err != nil {
return nil, mapierrors.InvalidMachineConfiguration("error getting vswitch ID: %v", err)
}

clusterID, ok := getClusterID(machine)
if !ok {
klog.Errorf("Unable to get cluster ID for machine: %q", machine.Name)
Expand All @@ -105,8 +113,13 @@ func runInstances(machine *machinev1.Machine, machineProviderConfig *alibabaclou
// RegionID
runInstancesRequest.RegionId = machineProviderConfig.RegionID

// SecurityGroupID
runInstancesRequest.SecurityGroupId = securityGroupID
// ResourceGroupID
if machineProviderConfig.ResourceGroupID != "" {
runInstancesRequest.ResourceGroupId = machineProviderConfig.ResourceGroupID
}

// SecurityGroupIDs
runInstancesRequest.SecurityGroupIds = securityGroupIDs

// Add tags to the created machine
tagList := buildTagList(machine.Name, clusterID, machineProviderConfig.Tags)
Expand Down Expand Up @@ -147,7 +160,7 @@ func runInstances(machine *machinev1.Machine, machineProviderConfig *alibabaclou
}

// VswitchId
runInstancesRequest.VSwitchId = machineProviderConfig.VSwitchID
runInstancesRequest.VSwitchId = vSwitchID

// SystemDisk
runInstancesRequest.SystemDiskCategory = machineProviderConfig.SystemDiskCategory
Expand Down Expand Up @@ -363,68 +376,142 @@ func getImageID(machine runtimeclient.ObjectKey, machineProviderConfig *alibabac
return image.ImageId, nil
}

func getSecurityGroupID(machine runtimeclient.ObjectKey, machineProviderConfig *alibabacloudproviderv1.AlibabaCloudMachineProviderConfig, client alibabacloudClient.Client) (string, error) {
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)
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 != "" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When ResourceGroupID is specified I am unable to query for the correct security group. I would remove resource group id.

aliyun ecs DescribeSecurityGroups --RegionId us-east-1 \
 --Tag.0.Key owned --Tag.0.Value "kubernetes.io/cluster/test-2mbh7" \
 --Tag.1.Key Name --Tag.1.Value "test-2mbh7-sg-worker" \
 --Tag.2.Key OCP --Tag.2.Value "ISV Integration"
{
	"PageNumber": 1,
	"PageSize": 10,
	"RegionId": "us-east-1",
	"RequestId": "BCD4DF49-FEED-57F6-A3AE-D670FB876026",
	"SecurityGroups": {
		"SecurityGroup": [
			{

				"CreationTime": "2021-11-01T17:11:52Z",
				"Description": "Created By OpenShift Installer",
				"ResourceGroupId": "",
				"SecurityGroupId": "sg-0xi9838n99r7i86s6rri",
				"SecurityGroupName": "test-2mbh7-sg-worker",
				"SecurityGroupType": "normal",
				"ServiceManaged": false,
				"Tags": {
					"Tag": [
						{
							"TagKey": "kubernetes.io/cluster/test-2mbh7",
							"TagValue": "owned"
						},
						{
							"TagKey": "Name",
							"TagValue": "test-2mbh7-sg-worker"
						},
						{
							"TagKey": "OCP",
							"TagValue": "ISV Integration"
						}
					]
				},
				"VpcId": "vpc-0xiosfgntngzl5ali1wj5"
...

Notice the resourceGroupID is not set.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if you don't assign an instance(ig. ECS Instance Or SecurityGroup ,etc) to a resource group. You don't need to set the resourceGroupID. If you set the resourceGroupID. The API simply returns the resources under a given resource group.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kwoodson I think I can explain the problem, it should be that the resource_group is not specified here
resource_group_id parameter, I have fixed it and submitted it in #5348

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)
}
}
}
}
}

request := ecs.CreateDescribeSecurityGroupsRequest()
request.VpcId = machineProviderConfig.VpcID
request.RegionId = machineProviderConfig.RegionID
request.SecurityGroupId = machineProviderConfig.SecurityGroupID
request.Scheme = "https"
return &securityGroupIDs, nil
}

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 "", fmt.Errorf("error describing securitygroup: %v", err)
func getMaxInstancesBySecurityGroupType(securityGroupType string) int {
switch securityGroupType {
case SecurityGroupTypeNoraml:
return MaxInstanceOfSecurityGroupTypeNoraml
case SecurityGroupTypeEnterprise:
return MaxInstanceOfSecurityGroupTypeEnterprise
default:
return MaxInstanceOfSecurityGroupTypeNoraml
}
}

func buildDescribeSecurityGroupsTag(tags []alibabacloudproviderv1.Tag) *[]ecs.DescribeSecurityGroupsTag {
describeSecurityGroupsTag := make([]ecs.DescribeSecurityGroupsTag, len(tags))

if len(response.SecurityGroups.SecurityGroup) < 1 {
klog.Errorf("no securitygroup for given filters not found")
return "", fmt.Errorf("no securitygroup for given filters not found")
for index, tag := range tags {
describeSecurityGroupsTag[index] = ecs.DescribeSecurityGroupsTag{
Key: tag.Key,
Value: tag.Value,
}
}

securityGroup := response.SecurityGroups.SecurityGroup[0]
return &describeSecurityGroupsTag
}

// Query how many instances are under the security group
describeInstancesRequest := ecs.CreateDescribeInstancesRequest()
describeInstancesRequest.RegionId = machineProviderConfig.RegionID
describeInstancesRequest.SecurityGroupId = securityGroup.SecurityGroupId
describeInstancesRequest.PageSize = requests.NewInteger(1)
describeInstancesRequest.Scheme = "https"
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
}

describeInstancesResponse, err := client.DescribeInstances(describeInstancesRequest)
if err != nil {
metrics.RegisterFailedInstanceCreate(&metrics.MachineLabels{
Name: machine.Name,
Namespace: machine.Namespace,
Reason: err.Error(),
})
klog.Errorf("error describing instances: %v", err)
return "", fmt.Errorf("error describing instances: %v", err)
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)

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 len(describeVSwitchesResponse.VSwitches.VSwitch) < 1 {
klog.Errorf("no vswitches for given tags not found")
return "", fmt.Errorf("no vswitches for given tags not found")
}

vSwitchID = describeVSwitchesResponse.VSwitches.VSwitch[0].VSwitchId
}
}
}

maxInstances := getMaxInstancesBySecurityGroupType(securityGroup.SecurityGroupType)
if describeInstancesResponse.TotalCount >= maxInstances {
return "", fmt.Errorf("the maximum number of instances in the security group has been exceeded: %d", maxInstances)
if vSwitchID == "" {
return "", fmt.Errorf("no vswitches were found")
}

return securityGroup.SecurityGroupId, nil
return vSwitchID, nil
}

func getMaxInstancesBySecurityGroupType(securityGroupType string) int {
switch securityGroupType {
case SecurityGroupTypeNoraml:
return MaxInstanceOfSecurityGroupTypeNoraml
case SecurityGroupTypeEnterprise:
return MaxInstanceOfSecurityGroupTypeEnterprise
default:
return MaxInstanceOfSecurityGroupTypeNoraml
func buildDescribeVSwitchesTag(tags []alibabacloudproviderv1.Tag) *[]vpc.DescribeVSwitchesTag {
describeVSwitchesTag := make([]vpc.DescribeVSwitchesTag, len(tags))

for index, tag := range tags {
describeVSwitchesTag[index] = vpc.DescribeVSwitchesTag{
Key: tag.Key,
Value: tag.Value,
}
}

return &describeVSwitchesTag
}

// buildTagList compile a list of ecs tags from machine provider spec and infrastructure object platform spec
Expand Down
6 changes: 1 addition & 5 deletions pkg/actuators/machine/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -309,11 +309,7 @@ func (r *Reconciler) setProviderID(instance *ecs.Instance) error {
return nil
}

providerID := fmt.Sprintf("alibabacloud:///%s/%s/%s", instance.RegionId, instance.ZoneId, instance.InstanceId)
// If resourceGroupId is not empty, set to providerId
if instance.ResourceGroupId != "" {
providerID = fmt.Sprintf("alibabacloud:///%s/%s/%s/%s", instance.ResourceGroupId, instance.RegionId, instance.ZoneId, instance.InstanceId)
}
providerID := fmt.Sprintf("alicloud://%s.%s", instance.RegionId, instance.InstanceId)

if existingProviderID != nil && *existingProviderID == providerID {
klog.Infof("%s: ProviderID already set in the machine Spec with value:%s", r.machine.Name, *existingProviderID)
Expand Down
Loading