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 17 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,81 @@ spec:
on resources within that account. If an account name is provided,
a domain must also be provided.
type: string
availabilityZones:
description: AvailabilityZones list of different partitions 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
credentialsRef:
description: CredentialsRef refers to the credentials in the
input ini file which contains api key/secret key for interacting
with this AZ
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
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:
- credentialsRef
- domain
- managementApiEndpoint
- 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 +173,6 @@ spec:
- network
type: object
type: array
required:
- domain
- managementApiEndpoint
- zones
type: object
status:
description: CloudStackDatacenterConfigStatus defines the observed state
Expand Down
79 changes: 75 additions & 4 deletions config/manifest/eksa-components.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3303,6 +3303,81 @@ spec:
on resources within that account. If an account name is provided,
a domain must also be provided.
type: string
availabilityZones:
description: AvailabilityZones list of different partitions 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
credentialsRef:
description: CredentialsRef refers to the credentials in the
input ini file which contains api key/secret key for interacting
with this AZ
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
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:
- credentialsRef
- domain
- managementApiEndpoint
- 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 @@ -3355,10 +3430,6 @@ spec:
- network
type: object
type: array
required:
- domain
- managementApiEndpoint
- zones
type: object
status:
description: CloudStackDatacenterConfigStatus defines the observed state
Expand Down
68 changes: 37 additions & 31 deletions designs/cloudstack-multiple-endpoints.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,50 +49,50 @@ will need to be available on *all* the Cloudstack API endpoints. We will validat
Currently, the CloudstackDataCenterConfig spec contains:
```
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
Domain string `json:"domain"`
// Zones is a list of one or more zones that are managed by a single CloudStack management endpoint.
Zones []CloudStackZone `json:"zones"`
// 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"`
// 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
Domain string `json:"domain"`
// Zones is a list of one or more zones that are managed by a single CloudStack management endpoint.
Zones []CloudStackZone `json:"zones"`
// 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"`
}
```

We would instead propose to gradually deprecate all the existing attributes and instead, simply include a list of AvailabilityZone objects like so

```
type CloudStackDatacenterConfigSpec struct {
// Deprecated
Domain string `json:"domain,omitempty"`
// Deprecated
Zones []CloudStackZone `json:"zones,omitempty"`
// Deprecated
Account string `json:"account,omitempty"`
// Deprecated
ManagementApiEndpoint string `json:"managementApiEndpoint,omitempty"`
// List of AvailabilityZones to distribute VMs across - corresponds to a list of CAPI failure domains
AvailabilityZones []CloudStackAvailabilityZone `json:"availabilityZones,omitempty"`
// Deprecated
Domain string `json:"domain,omitempty"`
// Deprecated
Zones []CloudStackZone `json:"zones,omitempty"`
// Deprecated
Account string `json:"account,omitempty"`
// Deprecated
ManagementApiEndpoint string `json:"managementApiEndpoint,omitempty"`
// List of AvailabilityZones to distribute VMs across - corresponds to a list of CAPI failure domains
AvailabilityZones []CloudStackAvailabilityZone `json:"availabilityZones,omitempty"`
}
```

where each AvailabilityZone object looks like

```
type CloudStackAvailabilityZone struct {
// Name would be used to match the availability zone defined in the datacenter config to the credentials passed in from the cloud-config ini file
Name string `json:"name"`
// 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"`
// Zones is a list of one or more zones that are managed by a single CloudStack management endpoint.
Zone CloudStackZone `json:"zone"`
// 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"`
// CredentialRef would be used to match the availability zone defined in the datacenter config to the credentials passed in from the cloud-config ini file
Copy link
Member

Choose a reason for hiding this comment

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

Instead of saying this matches the ini file, can we just say this references a secret in the eksa-namespace?

We could explain how the ini file works and maps to secrets in the "text docs" as opposed to the CRD API docs.

CredentialRef string `json:"credentialRef"`
// 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"`
// Zones is a list of one or more zones that are managed by a single CloudStack management endpoint.
Zone CloudStackZone `json:"zone"`
// 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"`
}
```

Expand All @@ -107,7 +107,13 @@ CAPC currently utilizes them to distribute machines across CloudStack Zones. How
2. Cloudstack domain
3. Cloudstack zone
4. Cloudstack account
5. A unique name
5. A reference to the customer-provider credentials

To add more detail to the credential ref, we will continue to accept a base64 encoded ini file as input from the customer when running eks-a. However,
in order to avoid taking a dependency on a meaningless section of a transient file, we will modify EKS-A to also create the secrets to be used by CAPC.
These secrets will be generated from the ini file provided by customer and applied to the cluster. CAPC will then proceed to take ownership of them as
Copy link
Member

Choose a reason for hiding this comment

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

Maybe worth to mention in which scenarios this file will be required? For example: when creating a management cluster, always. But what about upgrade? What about workload clusters?

Another interesting questions is, what happens if the user provides an ini file with a "secret" that already exists? (imagine for management cluster upgrades). Will the secret be updated or should they be immutable?

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 we would model all these decisions from what the vsphere provider currently has defined. As I understand, the secrets can be updated on every eks-a operation (create, upgrade) so I would expect the file will be required for every operation. VSphere seems to apply the generated secrets from the input credentials on every operation and ignores old ones. I suppose this isn't the cleanest approach but old secrets could be cleaned up manually in the worst case.

Copy link
Member

Choose a reason for hiding this comment

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

I would recommend not following what vsphere does just because that's the status quo. Some of the logic in the vsphere provider was intentional, some other logic was just built over time. Also, vSphere might have other constraints different than cloudstack (for starters, it doesn't support multiendpoint).

If you want to overwrite secrets: are those secrets gonna be unique per cluster? Or will different clusters share the same secrets as long as they share the same availability zones?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated doc to reflect our offline discussion - EKS-A will create secrets but not modify existing ones to prevent unintentional overwrites. If users want to modify credentials, they can do so manually, see below

Copy link
Member

Choose a reason for hiding this comment

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

does it mean every time we pass an updated ini and run upgrade cmd, eks-a creates a new secret and update the CredentialRef to point to the new secret? what happens to the old unused secrets?

currently done in [CAPV](https://github.com/kubernetes-sigs/cluster-api-provider-vsphere/blob/fae6ef88467e608665e2902e2bb0aaeb4cee67ed/docs/identity_management.md#identity-types).
We will refer to this set of credentials in the CloudstackCluster resource as well so CAPC knows which ones to use.

You can find more information about these Cloudstack resources [here](http://docs.cloudstack.apache.org/en/latest/conceptsandterminology/concepts.html#cloudstack-terminology)

Expand Down
Loading