Skip to content

Bug 2037209: refactor AlibabaResourceReference #1096

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
openshift-merge-robot merged 6 commits into from
Jan 24, 2022
Merged

Bug 2037209: refactor AlibabaResourceReference #1096

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
824f27a
update AlibabaResourceReference to follow a union pattern
elmiko Jan 13, 2022
075e263
refactor ResourceGroup in AlibabaCloudMachineProviderConfig
elmiko Jan 13, 2022
1dc6e45
update Alibaba resource reference documentation
elmiko Jan 13, 2022
3af5513
Add a Name field to AlibabaResourceReference
elmiko Jan 14, 2022
6598a1f
update required fields for AlibabaCloudMachineProviderConfig
elmiko Jan 14, 2022
8e1988f
update deepcopy script to include machine:v1
elmiko Jan 21, 2022
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
2 changes: 1 addition & 1 deletion hack/update-deepcopy.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ verify="${VERIFY:-}"
GOFLAGS="" bash ${CODEGEN_PKG}/generate-groups.sh "deepcopy" \
github.com/openshift/api/generated \
github.com/openshift/api \
"apiserver:v1 apps:v1 authorization:v1 build:v1 config:v1 helm:v1beta1 console:v1 console:v1alpha1 image:v1,docker10,dockerpre012 imageregistry:v1 kubecontrolplane:v1 legacyconfig:v1 cloudnetwork:v1 network:v1 networkoperator:v1 oauth:v1 openshiftcontrolplane:v1 operator:v1 operator:v1alpha1 operatorcontrolplane:v1alpha1 operatoringress:v1 osin:v1 project:v1 quota:v1 route:v1 samples:v1 security:v1 securityinternal:v1 servicecertsigner:v1alpha1 sharedresource:v1alpha1 template:v1 user:v1 machine:v1beta1" \
"apiserver:v1 apps:v1 authorization:v1 build:v1 config:v1 helm:v1beta1 console:v1 console:v1alpha1 image:v1,docker10,dockerpre012 imageregistry:v1 kubecontrolplane:v1 legacyconfig:v1 cloudnetwork:v1 network:v1 networkoperator:v1 oauth:v1 openshiftcontrolplane:v1 operator:v1 operator:v1alpha1 operatorcontrolplane:v1alpha1 operatoringress:v1 osin:v1 project:v1 quota:v1 route:v1 samples:v1 security:v1 securityinternal:v1 servicecertsigner:v1alpha1 sharedresource:v1alpha1 template:v1 user:v1 machine:v1beta1 machine:v1" \
--go-header-file ${SCRIPT_ROOT}/hack/empty.txt \
${verify}

49 changes: 36 additions & 13 deletions machine/v1/types_alibabaprovider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ type AlibabaDiskEncryptionMode string
// AlibabaDiskPreservationPolicy enum attribute to describe whether to preserve or delete a disk upon instance removal
type AlibabaDiskPreservationPolicy string

// AlibabaResourceReferenceType enum attribute to identify the type of resource reference
type AlibabaResourceReferenceType string

const (
// DeleteWithInstance enum property to delete disk with instance deletion
DeleteWithInstance AlibabaDiskPreservationPolicy = "DeleteWithInstance"
Expand Down Expand Up @@ -61,6 +64,13 @@ const (
AlibabaDiskCatagoryESSD AlibabaDiskCategory = "cloud_essd"
// AlibabaDiskCategoryBasic enum proprty to set the category of disk to basic
AlibabaDiskCatagoryBasic AlibabaDiskCategory = "cloud"

// AlibabaResourceReferenceTypeID enum property to identify an ID type resource reference
AlibabaResourceReferenceTypeID AlibabaResourceReferenceType = "ID"
// AlibabaResourceReferenceTypeName enum property to identify an Name type resource reference
AlibabaResourceReferenceTypeName AlibabaResourceReferenceType = "Name"
// AlibabaResourceReferenceTypeTags enum property to identify a tags type resource reference
AlibabaResourceReferenceTypeTags AlibabaResourceReferenceType = "Tags"
)

// +genclient
Expand All @@ -81,8 +91,7 @@ type AlibabaCloudMachineProviderConfig struct {
InstanceType string `json:"instanceType"`

// The ID of the vpc
// +optional
VpcID string `json:"vpcId,omitempty"`
VpcID string `json:"vpcId"`

// The ID of the region in which to create the instance. You can call the DescribeRegions operation to query the most recent region list.
RegionID string `json:"regionId"`
Expand All @@ -97,10 +106,12 @@ type AlibabaCloudMachineProviderConfig struct {
// +optional
DataDisks []DataDiskProperties `json:"dataDisk,omitempty"`

// SecurityGroups is a list of security group references to assign to the instance. A reference holds either the security group ID
// or the required tags to search. The limit of N values varies based on the maximum number of security groups to which an instance can belong.
// SecurityGroups is a list of security group references to assign to the instance.
// A reference holds either the security group ID, the resource name, or the required tags to search.
// When more than one security group is returned for a tag search, all the groups are associated with the instance up to the
// maximum number of security groups to which an instance can belong.
// For more information, see the "Security group limits" section in Limits.
// https://www.alibabacloud.com/help/doc-detail/101348.htm?spm=a2c63.p38356.879954.48.78f0199aX3dfIE
// https://www.alibabacloud.com/help/en/doc-detail/25412.htm
SecurityGroups []AlibabaResourceReference `json:"securityGroups,omitempty"`

// Bandwidth describes the internet bandwidth strategy for the instance
Expand All @@ -111,18 +122,22 @@ type AlibabaCloudMachineProviderConfig struct {
// +optional
SystemDisk SystemDiskProperties `json:"systemDisk,omitempty"`

// VSwitch is a reference to the vswitch to use for this instance
// VSwitch is a reference to the vswitch to use for this instance.
// A reference holds either the vSwitch ID, the resource name, or the required tags to search.
// When more than one vSwitch is returned for a tag search, only the first vSwitch returned will be used.
Copy link
Contributor

@deads2k deads2k Jan 14, 2022

Choose a reason for hiding this comment

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

is this ordering guaranteed to be consistent from the API you're searching with?

Copy link
Contributor Author

@elmiko elmiko Jan 14, 2022

Choose a reason for hiding this comment

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

that is a great question, it's not immediately clear from the docs[0] if the ordering is consistent. i will have to confer with alibaba about this. should we add a note here about that?

[0] https://www.alibabacloud.com/help/en/doc-detail/110425.htm?spm=a2c63.p38356.0.0.322619b8Xb6y0V#doc-api-Ecs-ListTagResources

// This parameter is required when you create an instance of the VPC type.
// You can call the DescribeVSwitches operation to query the created vSwitches.
VSwitch AlibabaResourceReference `json:"vSwitch,omitempty"`
VSwitch AlibabaResourceReference `json:"vSwitch"`

// RAMRoleName is the name of the instance Resource Access Management (RAM) role. This allows the instance to perform API calls as this specified RAM role.
// +optional
RAMRoleName string `json:"ramRoleName,omitempty"`

// ResourceGroupID is the unique ID of the resource group to which to assign the instance.
// +optional
ResourceGroupID string `json:"resourceGroupId,omitempty"`
// ResourceGroup references the resource group to which to assign the instance.
// A reference holds either the resource group ID, the resource name, or the required tags to search.
// When more than one resource group are returned for a search, an error will be produced and the Machine will not be created.
// Resource Groups do not support searching by tags.
ResourceGroup AlibabaResourceReference `json:"resourceGroup"`
Copy link
Contributor

@JoelSpeed JoelSpeed Jan 17, 2022

Choose a reason for hiding this comment

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

Based on some of the conversations I've seen on slack, I think this should be optional

Copy link
Contributor Author

@elmiko elmiko Jan 20, 2022

Choose a reason for hiding this comment

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

is this still true?

the latest conversations seem to indicate that we can require this field, even for the default resource group.


// Tenancy specifies whether to create the instance on a dedicated host.
// Valid values:
Expand Down Expand Up @@ -153,13 +168,21 @@ type AlibabaCloudMachineProviderConfig struct {
// Only one of ID or Tags may be specified. Specifying more than one will result in
// a validation error.
type AlibabaResourceReference struct {
// type identifies the resource reference type for this entry.
Type AlibabaResourceReferenceType `json:"type"`

// ID of resource
// +optional
ID string `json:"id,omitempty"`
ID *string `json:"id,omitempty"`

// Name of the resource
// +optional
Name *string `json:"name,omitempty"`

// Tags is a set of metadata based upon ECS object tags used to identify a resource
// Tags is a set of metadata based upon ECS object tags used to identify a resource.
// For details about usage when multiple resources are found, please see the owning parent field documentation.
// +optional
Tags []Tag `json:"tags,omitempty"`
Tags *[]Tag `json:"tags,omitempty"`
}

// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
Expand Down
244 changes: 244 additions & 0 deletions machine/v1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.