Skip to content

config/v1/types_infrastructure: Add AWS Region #300

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 2 commits into from
May 7, 2019
Merged

config/v1/types_infrastructure: Add AWS Region #300

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dedfb47
config/v1/types_infrastructure: Add AWS Region
wking Apr 26, 2019
78e39eb
generated
wking Apr 26, 2019
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
41 changes: 33 additions & 8 deletions config/v1/types_infrastructure.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,16 @@ type InfrastructureStatus struct {
// alphanumeric or hyphen characters.
InfrastructureName string `json:"infrastructureName"`

// platform is the underlying infrastructure provider for the cluster. This
// value controls whether infrastructure automation such as service load
// balancers, dynamic volume provisioning, machine creation and deletion, and
// other integrations are enabled. If None, no infrastructure automation is
// enabled. Allowed values are "AWS", "Azure", "BareMetal", "GCP", "Libvirt",
// "OpenStack", "VSphere", and "None". Individual components may not support
// all platforms, and must handle unrecognized platforms as None if they do
// not support that platform.
// platform is the underlying infrastructure provider for the cluster.
//
// Deprecated: Use platformStatus.type instead.
Platform PlatformType `json:"platform,omitempty"`

// platformStatus holds status information specific to the underlying
// infrastructure provider.
// +optional
PlatformStatus *PlatformStatus `json:"platformStatus,omitempty"`
Comment thread
wking marked this conversation as resolved.

// etcdDiscoveryDomain is the domain used to fetch the SRV records for discovering
// etcd servers and clients.
// For more info: https://github.com/etcd-io/etcd/blob/329be66e8b3f9e2e6af83c123ff89297e49ebd15/Documentation/op-guide/clustering.md#dns-discovery
Expand Down Expand Up @@ -93,6 +93,31 @@ const (
VSpherePlatformType PlatformType = "VSphere"
)

// PlatformStatus holds the current status specific to the underlying infrastructure provider
// of the current cluster. Since these are used at status-level for the underlying cluster, it
// is supposed that only one of the status structs is set.
type PlatformStatus struct {
Copy link
Copy Markdown
Contributor

@deads2k deads2k May 3, 2019

Choose a reason for hiding this comment

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

This needs a discriminator field which indicates which one of the many values is going to be set. CRD discriminators are assumed to be in the same struct as the subfields I think. @sttts

Copy link
Copy Markdown
Contributor

@sttts sttts May 3, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

in the CRD it must be in the same JSON object (which in case of struct embedding might differ from "same struct"). Generators like crd-gen from controller-tools will probably require the same struct.

Copy link
Copy Markdown
Member Author

@wking wking May 3, 2019

Choose a reason for hiding this comment

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

So bring

api/config/v1/types_infrastructure.go

Line 48 in a1f3bdc

Platform PlatformType `json:"platform,omitempty"`
down into here? And deprecate the old property?

Copy link
Copy Markdown
Member Author

@wking wking May 3, 2019

Choose a reason for hiding this comment

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

...deprecate for removal in v2 in four years ;)

Copy link
Copy Markdown
Contributor

@sttts sttts May 3, 2019

Choose a reason for hiding this comment

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

and that's not only a restriction for CRDs, but will be for native types as well.

Copy link
Copy Markdown
Contributor

@deads2k deads2k May 3, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

...deprecate for removal in v2 in four years ;)

I think you're left with no other choice. Duplicate the value

Copy link
Copy Markdown
Contributor

@abhinavdahiya abhinavdahiya May 3, 2019

Choose a reason for hiding this comment

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

I'm not sure if native API adds discriminators like
https://godoc.org/k8s.io/api/core/v1#VolumeSource

Copy link
Copy Markdown
Member Author

@wking wking May 3, 2019

Choose a reason for hiding this comment

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

Ok, pushed the discriminator down into platformStatus deprecating the old property with a1f3bdc -> 81ef6b4.

// type is the underlying infrastructure provider for the cluster. This
// value controls whether infrastructure automation such as service load
// balancers, dynamic volume provisioning, machine creation and deletion, and
// other integrations are enabled. If None, no infrastructure automation is
// enabled. Allowed values are "AWS", "Azure", "BareMetal", "GCP", "Libvirt",
// "OpenStack", "VSphere", and "None". Individual components may not support
// all platforms, and must handle unrecognized platforms as None if they do
// not support that platform.
Type PlatformType `json:"type"`

// AWS contains settings specific to the Amazon Web Services infrastructure provider.
// +optional
AWS *AWSPlatformStatus `json:"aws,omitempty"`
}

// AWSPlatformStatus holds the current status of the Amazon Web Services infrastructure provider.
type AWSPlatformStatus struct {
// region holds the default AWS region for new AWS resources created by the cluster.
Region string `json:"region"`
}

// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

// InfrastructureList is
Expand Down
44 changes: 43 additions & 1 deletion config/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.

22 changes: 21 additions & 1 deletion config/v1/zz_generated.swagger_doc_generated.go
View file
Open in desktop

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