Skip to content
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.

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

Defining new types for Cloudstack AvailabilityZone development #2412

Merged
merged 27 commits into from
Jun 27, 2022
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
f01e5f0
Defining new types for Cloudstack multiendpoint development
maxdrib Jun 15, 2022
4e9717f
Adding unit test
maxdrib Jun 15, 2022
48a49c6
Adding Name and generating manifests
maxdrib Jun 16, 2022
7a1a24b
Fixing incorrect json label
maxdrib Jun 16, 2022
e0209e1
Deprecating old fields and marking them as optional
maxdrib Jun 16, 2022
d5a4cb3
Removing premature deprecation comment and linting
maxdrib Jun 16, 2022
79d1698
Removing deprecated flag
maxdrib Jun 17, 2022
836e096
Adding unit tests
maxdrib Jun 17, 2022
8732bec
Linting
maxdrib Jun 17, 2022
8dd9e44
More unit testing
maxdrib Jun 17, 2022
5d59551
responding to comments
maxdrib Jun 20, 2022
545d2d1
Updating manifests
maxdrib Jun 20, 2022
d4aa8b6
Generated release manifests
maxdrib Jun 20, 2022
7c6cb84
Adding credentialsRef instead of unique name for az
maxdrib Jun 20, 2022
dd081c4
Generating manifests
maxdrib Jun 20, 2022
2f78096
Merge branch 'main' into multiple-endpoints
maxdrib Jun 21, 2022
b9db281
Updating design to reflect credentialsref
maxdrib Jun 21, 2022
50f7b73
Updating design and readding Name field to AZ struct
maxdrib Jun 22, 2022
83a3828
Updating AZ equality definition to build a map instead of nested for …
maxdrib Jun 22, 2022
aab0350
linting
maxdrib Jun 22, 2022
6201e80
Updating description for credentialsRef
maxdrib Jun 22, 2022
3e56b0a
Adding todo to validator
maxdrib Jun 22, 2022
fd06a1d
Updating design to reflect that secrets should be immutable
maxdrib Jun 23, 2022
75b321c
Adding notes about EKS-A policy on creating but not updating secrets
maxdrib Jun 23, 2022
b353a4e
Flushing out details on credential management in the design
maxdrib Jun 24, 2022
7475853
Responding to Joey's comments
maxdrib Jun 24, 2022
2c76dbb
Update designs/cloudstack-multiple-endpoints.md
maxdrib Jun 27, 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
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,80 @@ spec:
on resources within that account. If an account name is provided,
a domain must also be provided.
type: string
availabilityZones:
description: List of AvailabilityZones to distribute VMs across -
corresponds to a list of CAPI failure domains
items:
description: CloudStackAvailabilityZone maps to a CAPI failure domain
to distribute machines across Cloudstack infrastructure
properties:
account:
description: Account typically represents a customer of the
service provider or a department in a large organization.
Multiple users can exist in an account, and all CloudStack
resources belong to an account. Accounts have users and users
have credentials to operate on resources within that account.
If an account name is provided, a domain must also be provided.
type: string
domain:
description: Domain contains a grouping of accounts. Domains
usually contain multiple accounts that have some logical relationship
to each other and a set of delegated administrators with some
authority over the domain and its subdomains This field is
considered as a fully qualified domain name which is the same
as the domain path without "ROOT/" prefix. For example, if
"foo" is specified then a domain with "ROOT/foo" domain path
is picked. The value "ROOT" is a special case that points
to "the" ROOT domain of the CloudStack. That is, a domain
with a path "ROOT/ROOT" is not allowed.
type: string
managementApiEndpoint:
description: CloudStack Management API endpoint's IP. It is
added to VM's noproxy list
type: string
name:
description: Name is the name of the availability zone and should
match with the input cloud-config credentials file
type: string
zone:
description: Zone represents the properties of the CloudStack
zone in which clusters should be created, like the network.
properties:
id:
description: Zone is the name or UUID of the CloudStack
zone in which clusters should be created. Zones should
be managed by a single CloudStack Management endpoint.
type: string
name:
type: string
network:
description: Network is the name or UUID of the CloudStack
network in which clusters should be created. It can either
be an isolated or shared network. If it doesn’t already
exist in CloudStack, it’ll automatically be created by
CAPC as an isolated network. It can either be specified
as a UUID or name In multiple-zones situation, only 'Shared'
network is supported.
properties:
id:
description: Id of a resource in the CloudStack environment.
Mutually exclusive with Name
type: string
name:
description: Name of a resource in the CloudStack environment.
Mutually exclusive with Id
type: string
type: object
required:
- network
type: object
required:
- domain
- managementApiEndpoint
- name
- zone
type: object
type: array
domain:
description: Domain contains a grouping of accounts. Domains usually
contain multiple accounts that have some logical relationship to
Expand Down Expand Up @@ -98,10 +172,6 @@ spec:
- network
type: object
type: array
required:
- domain
- managementApiEndpoint
- zones
type: object
status:
description: CloudStackDatacenterConfigStatus defines the observed state
Expand Down
77 changes: 77 additions & 0 deletions pkg/api/v1alpha1/cloudstackdatacenterconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,23 @@ var cloudStackDatacenterConfigSpec1 = &CloudStackDatacenterConfigSpec{
ManagementApiEndpoint: "testEndpoint",
}

var cloudStackDatacenterConfigSpecAzs = &CloudStackDatacenterConfigSpec{
AvailabilityZones: []CloudStackAvailabilityZone{
{
Name: "Global",
Zone: CloudStackZone{
Name: "zone1",
Network: CloudStackResourceIdentifier{
Name: "net1",
},
},
Account: "admin",
Domain: "domain1",
ManagementApiEndpoint: "testEndpoint",
},
},
}

func TestCloudStackDatacenterConfigSpecEqual(t *testing.T) {
g := NewWithT(t)
cloudStackDatacenterConfigSpec2 := cloudStackDatacenterConfigSpec1.DeepCopy()
Expand Down Expand Up @@ -201,3 +218,63 @@ func TestCloudStackDatacenterConfigSpecNotEqualZonesNil(t *testing.T) {
cloudStackDatacenterConfigSpec2.Zones = nil
g.Expect(cloudStackDatacenterConfigSpec1.Equal(cloudStackDatacenterConfigSpec2)).To(BeFalse(), "Zones comparison in CloudStackDatacenterConfigSpec not detected")
}

func TestCloudStackDatacenterConfigSpecNotEqualAvailabilityZonesNil(t *testing.T) {
g := NewWithT(t)
g.Expect(cloudStackDatacenterConfigSpecAzs.AvailabilityZones[0].Equal(nil)).To(BeFalse(), "Zones comparison in CloudStackDatacenterConfigSpec not detected")
}

func TestCloudStackDatacenterConfigSpecNotEqualAvailabilityZonesEmpty(t *testing.T) {
g := NewWithT(t)
cloudStackDatacenterConfigSpec2 := cloudStackDatacenterConfigSpecAzs.DeepCopy()
cloudStackDatacenterConfigSpec2.AvailabilityZones = []CloudStackAvailabilityZone{}
g.Expect(cloudStackDatacenterConfigSpecAzs.Equal(cloudStackDatacenterConfigSpec2)).To(BeFalse(), "AvailabilityZones comparison in CloudStackDatacenterConfigSpec not detected")
}

func TestCloudStackDatacenterConfigSpecNotEqualAvailabilityZonesModified(t *testing.T) {
g := NewWithT(t)
cloudStackDatacenterConfigSpec2 := cloudStackDatacenterConfigSpecAzs.DeepCopy()
cloudStackDatacenterConfigSpec2.AvailabilityZones[0].Account = "differentAccount"
g.Expect(cloudStackDatacenterConfigSpec1.Equal(cloudStackDatacenterConfigSpec2)).To(BeFalse(), "AvailabilityZones comparison in CloudStackDatacenterConfigSpec not detected")
}

func TestCloudStackAvailabilityZonesEqual(t *testing.T) {
g := NewWithT(t)
cloudStackAvailabilityZoneSpec2 := cloudStackDatacenterConfigSpecAzs.AvailabilityZones[0].DeepCopy()
g.Expect(cloudStackDatacenterConfigSpecAzs.AvailabilityZones[0].Equal(cloudStackAvailabilityZoneSpec2)).To(BeTrue(), "AvailabilityZones comparison in CloudStackAvailabilityZoneSpec not detected")
}

func TestCloudStackAvailabilityZonesSame(t *testing.T) {
g := NewWithT(t)
g.Expect(cloudStackDatacenterConfigSpecAzs.AvailabilityZones[0].Equal(&cloudStackDatacenterConfigSpecAzs.AvailabilityZones[0])).To(BeTrue(), "AvailabilityZones comparison in CloudStackAvailabilityZoneSpec not detected")
}

func TestCloudStackDatacenterConfigSpecNotEqualAvailabilityZonesManagementApiEndpoint(t *testing.T) {
g := NewWithT(t)
cloudStackDatacenterConfigSpec2 := cloudStackDatacenterConfigSpecAzs.DeepCopy()
cloudStackDatacenterConfigSpec2.AvailabilityZones[0].ManagementApiEndpoint = "fake-endpoint"
g.Expect(cloudStackDatacenterConfigSpec1.Equal(cloudStackDatacenterConfigSpec2)).To(BeFalse(), "AvailabilityZones comparison in CloudStackDatacenterConfigSpec not detected")
}

func TestCloudStackDatacenterConfigSpecNotEqualAvailabilityZonesAccount(t *testing.T) {
g := NewWithT(t)
cloudStackDatacenterConfigSpec2 := cloudStackDatacenterConfigSpecAzs.DeepCopy()
cloudStackDatacenterConfigSpec2.AvailabilityZones[0].Account = "fake-acc"
g.Expect(cloudStackDatacenterConfigSpec1.Equal(cloudStackDatacenterConfigSpec2)).To(BeFalse(), "AvailabilityZones comparison in CloudStackDatacenterConfigSpec not detected")
}

func TestCloudStackDatacenterConfigSpecNotEqualAvailabilityZonesDomain(t *testing.T) {
g := NewWithT(t)
cloudStackDatacenterConfigSpec2 := cloudStackDatacenterConfigSpecAzs.DeepCopy()
cloudStackDatacenterConfigSpec2.AvailabilityZones[0].Domain = "fake-domain"
g.Expect(cloudStackDatacenterConfigSpec1.Equal(cloudStackDatacenterConfigSpec2)).To(BeFalse(), "AvailabilityZones comparison in CloudStackDatacenterConfigSpec not detected")
}

func TestCloudStackDatacenterConfigSetDefaults(t *testing.T) {
g := NewWithT(t)
cloudStackDatacenterConfig := CloudStackDatacenterConfig{
Spec: *cloudStackDatacenterConfigSpec1.DeepCopy(),
}
cloudStackDatacenterConfig.SetDefaults()
g.Expect(cloudStackDatacenterConfig.Spec.Equal(cloudStackDatacenterConfigSpecAzs)).To(BeTrue(), "AvailabilityZones comparison in CloudStackDatacenterConfigSpec not equal")
}
67 changes: 63 additions & 4 deletions pkg/api/v1alpha1/cloudstackdatacenterconfig_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,15 @@ type CloudStackDatacenterConfigSpec struct {
// Domain contains a grouping of accounts. Domains usually contain multiple accounts that have some logical relationship to each other and a set of delegated administrators with some authority over the domain and its subdomains
// This field is considered as a fully qualified domain name which is the same as the domain path without "ROOT/" prefix. For example, if "foo" is specified then a domain with "ROOT/foo" domain path is picked.
// The value "ROOT" is a special case that points to "the" ROOT domain of the CloudStack. That is, a domain with a path "ROOT/ROOT" is not allowed.
//
Domain string `json:"domain"`
Domain string `json:"domain,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

do we want to add the deprecated note to each field?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We will add that after we've migrated EKS-A internally to using the new fields. Prior to that, the linter throws a ton of errors for using deprecated fields which will slow down our reviews :(

// Zones is a list of one or more zones that are managed by a single CloudStack management endpoint.
Zones []CloudStackZone `json:"zones"`
Zones []CloudStackZone `json:"zones,omitempty"`
// Account typically represents a customer of the service provider or a department in a large organization. Multiple users can exist in an account, and all CloudStack resources belong to an account. Accounts have users and users have credentials to operate on resources within that account. If an account name is provided, a domain must also be provided.
Account string `json:"account,omitempty"`
// CloudStack Management API endpoint's IP. It is added to VM's noproxy list
ManagementApiEndpoint string `json:"managementApiEndpoint"`
ManagementApiEndpoint string `json:"managementApiEndpoint,omitempty"`
// List of AvailabilityZones to distribute VMs across - corresponds to a list of CAPI failure domains
maxdrib marked this conversation as resolved.
Show resolved Hide resolved
AvailabilityZones []CloudStackAvailabilityZone `json:"availabilityZones,omitempty"`
}

type CloudStackResourceIdentifier struct {
Expand Down Expand Up @@ -71,6 +72,22 @@ type CloudStackZone struct {
Network CloudStackResourceIdentifier `json:"network"`
}

// CloudStackAvailabilityZone maps to a CAPI failure domain to distribute machines across Cloudstack infrastructure
type CloudStackAvailabilityZone struct {
// Name is the name of the availability zone and should match with the input cloud-config credentials file
Name string `json:"name"`
// Zone represents the properties of the CloudStack zone in which clusters should be created, like the network.
Zone CloudStackZone `json:"zone"`
// Domain contains a grouping of accounts. Domains usually contain multiple accounts that have some logical relationship to each other and a set of delegated administrators with some authority over the domain and its subdomains
// This field is considered as a fully qualified domain name which is the same as the domain path without "ROOT/" prefix. For example, if "foo" is specified then a domain with "ROOT/foo" domain path is picked.
// The value "ROOT" is a special case that points to "the" ROOT domain of the CloudStack. That is, a domain with a path "ROOT/ROOT" is not allowed.
Domain string `json:"domain"`
// Account typically represents a customer of the service provider or a department in a large organization. Multiple users can exist in an account, and all CloudStack resources belong to an account. Accounts have users and users have credentials to operate on resources within that account. If an account name is provided, a domain must also be provided.
Account string `json:"account,omitempty"`
// CloudStack Management API endpoint's IP. It is added to VM's noproxy list
ManagementApiEndpoint string `json:"managementApiEndpoint"`
}

// CloudStackDatacenterConfigStatus defines the observed state of CloudStackDatacenterConfig
type CloudStackDatacenterConfigStatus struct { // INSERT ADDITIONAL STATUS FIELD - define observed state of cluster
// Important: Run "make" to regenerate code after modifying this file
Expand Down Expand Up @@ -142,6 +159,26 @@ func (v *CloudStackDatacenterConfig) Validate() error {
return nil
}

func (v *CloudStackDatacenterConfig) SetDefaults() {
if v.Spec.AvailabilityZones == nil || len(v.Spec.AvailabilityZones) == 0 {
maxdrib marked this conversation as resolved.
Show resolved Hide resolved
v.Spec.AvailabilityZones = []CloudStackAvailabilityZone{}
maxdrib marked this conversation as resolved.
Show resolved Hide resolved
for _, csZone := range v.Spec.Zones {
az := CloudStackAvailabilityZone{
Zone: csZone,
Account: v.Spec.Account,
Domain: v.Spec.Domain,
ManagementApiEndpoint: v.Spec.ManagementApiEndpoint,
Name: "Global",
}
v.Spec.AvailabilityZones = append(v.Spec.AvailabilityZones, az)
}
}
v.Spec.Zones = nil
v.Spec.Domain = ""
v.Spec.Account = ""
v.Spec.ManagementApiEndpoint = ""
}

func (s *CloudStackDatacenterConfigSpec) Equal(o *CloudStackDatacenterConfigSpec) bool {
if s == o {
return true
Expand All @@ -157,6 +194,14 @@ func (s *CloudStackDatacenterConfigSpec) Equal(o *CloudStackDatacenterConfigSpec
return false
}
}
if len(s.AvailabilityZones) != len(o.AvailabilityZones) {
return false
}
for i, az := range s.AvailabilityZones {
if !az.Equal(&o.AvailabilityZones[i]) {
maxdrib marked this conversation as resolved.
Show resolved Hide resolved
return false
}
}
return s.ManagementApiEndpoint == o.ManagementApiEndpoint &&
s.Domain == o.Domain &&
s.Account == o.Account
Expand All @@ -178,6 +223,20 @@ func (z *CloudStackZone) Equal(o *CloudStackZone) bool {
return false
}

func (az *CloudStackAvailabilityZone) Equal(o *CloudStackAvailabilityZone) bool {
if az == o {
return true
}
if az == nil || o == nil {
return false
}
return az.Zone.Equal(&o.Zone) &&
az.Name == o.Name &&
az.Account == o.Account &&
az.Domain == o.Domain &&
az.ManagementApiEndpoint == o.ManagementApiEndpoint
}

// +kubebuilder:object:generate=false

// Same as CloudStackDatacenterConfig except stripped down for generation of yaml file during generate clusterconfig
Expand Down

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

46 changes: 46 additions & 0 deletions pkg/api/v1alpha1/zz_generated.deepcopy.go

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