From dedfb47b1ff14bbd03727fa5720f63d8dc7bea5b Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Fri, 26 Apr 2019 11:49:00 -0700 Subject: [PATCH 1/2] config/v1/types_infrastructure: Add AWS Region The image registry needs this so it can figure out where to create an S3 bucket for installs with cluster-provisioned infrastructure [1]. We need the registry to be happy after: $ openshift-install create cluster so we don't want to rely on the installer pushing this config in as a day-2 operation. You can extract "what AWS region am I in?" from the cloud provider, but the registry operator is not authorized to do so. With this addition, we provide a way for the cluster creator to declare a default region for new infrastructure. This field is in Status and not Spec, because we're not going to support transitioning clusters between regions by reconciling this. The cluster admin can write it once (in the manifest that goes up with the bootstrap Ignition config), and then it's read-only reality forever. I'd prefer an +optional Region property, because you only need to set it if you want the cluster to provision resources for you. For user-provided infrastructure flows, you could leave this unset. However, the registry needs the region for its S3 client [2], even if it's just to manage data in a user-provided bucket. So folks who leave the property off will have an Available:false registry until they set region or regionEndpoint via the configs.imageregistry.operator.openshift.io custom resource [3,4]. But Adam wanted it required [5], and going from required -> optional later is a non-breaking change while optional -> required would be breaking, so it's required with this commit. I've pushed the type specifier down into platformStatus, deprecating the old property, because discriminator fields need to live inside the struct whose properties they discriminate [6]. It would be nice to be able to drop the old type property and rename platformStatus to platform, but it's too close to API freeze to be able to do that, so we're stuck with the old name. We'll drop the old type in v2, if we ever have a v2 of this type. [1]: https://github.com/openshift/cluster-image-registry-operator/blob/670539cda460d97e0a3e23f077fdf9dd00d2ccdc/pkg/clusterconfig/clusterconfig.go#L44 [2]: https://github.com/openshift/cluster-image-registry-operator/blob/670539cda460d97e0a3e23f077fdf9dd00d2ccdc/pkg/storage/s3/s3.go#L53-L65 [3]: https://github.com/openshift/cluster-image-registry-operator/blob/670539cda460d97e0a3e23f077fdf9dd00d2ccdc/manifests/00-crd.yaml [4]: https://github.com/openshift/cluster-image-registry-operator/blob/670539cda460d97e0a3e23f077fdf9dd00d2ccdc/pkg/apis/imageregistry/v1/types.go#L179-L184 [5]: https://github.com/openshift/api/pull/300#discussion_r280575549 [6]: https://github.com/openshift/api/pull/300#discussion_r280795398 --- config/v1/types_infrastructure.go | 41 +++++++++++++++++++++++++------ 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/config/v1/types_infrastructure.go b/config/v1/types_infrastructure.go index 786e81a9a48..c59dc39c1ec 100644 --- a/config/v1/types_infrastructure.go +++ b/config/v1/types_infrastructure.go @@ -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"` + // 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 @@ -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 { + // 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 From 78e39ebaa7addc5cddd06a97b61a473dc1f051ee Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Fri, 26 Apr 2019 11:51:51 -0700 Subject: [PATCH 2/2] generated With: $ make generate --- config/v1/zz_generated.deepcopy.go | 44 ++++++++++++++++++- .../v1/zz_generated.swagger_doc_generated.go | 22 +++++++++- 2 files changed, 64 insertions(+), 2 deletions(-) diff --git a/config/v1/zz_generated.deepcopy.go b/config/v1/zz_generated.deepcopy.go index 3ad614c5e31..bb944cd89f5 100644 --- a/config/v1/zz_generated.deepcopy.go +++ b/config/v1/zz_generated.deepcopy.go @@ -150,6 +150,22 @@ func (in *APIServerStatus) DeepCopy() *APIServerStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AWSPlatformStatus) DeepCopyInto(out *AWSPlatformStatus) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AWSPlatformStatus. +func (in *AWSPlatformStatus) DeepCopy() *AWSPlatformStatus { + if in == nil { + return nil + } + out := new(AWSPlatformStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *AdmissionConfig) DeepCopyInto(out *AdmissionConfig) { *out = *in @@ -1613,7 +1629,7 @@ func (in *Infrastructure) DeepCopyInto(out *Infrastructure) { out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) out.Spec = in.Spec - out.Status = in.Status + in.Status.DeepCopyInto(&out.Status) return } @@ -1688,6 +1704,11 @@ func (in *InfrastructureSpec) DeepCopy() *InfrastructureSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *InfrastructureStatus) DeepCopyInto(out *InfrastructureStatus) { *out = *in + if in.PlatformStatus != nil { + in, out := &in.PlatformStatus, &out.PlatformStatus + *out = new(PlatformStatus) + (*in).DeepCopyInto(*out) + } return } @@ -2271,6 +2292,27 @@ func (in *OperandVersion) DeepCopy() *OperandVersion { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PlatformStatus) DeepCopyInto(out *PlatformStatus) { + *out = *in + if in.AWS != nil { + in, out := &in.AWS, &out.AWS + *out = new(AWSPlatformStatus) + **out = **in + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PlatformStatus. +func (in *PlatformStatus) DeepCopy() *PlatformStatus { + if in == nil { + return nil + } + out := new(PlatformStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Project) DeepCopyInto(out *Project) { *out = *in diff --git a/config/v1/zz_generated.swagger_doc_generated.go b/config/v1/zz_generated.swagger_doc_generated.go index c81304477e6..18e6fb47b9d 100644 --- a/config/v1/zz_generated.swagger_doc_generated.go +++ b/config/v1/zz_generated.swagger_doc_generated.go @@ -699,6 +699,15 @@ func (RegistrySources) SwaggerDoc() map[string]string { return map_RegistrySources } +var map_AWSPlatformStatus = map[string]string{ + "": "AWSPlatformStatus holds the current status of the Amazon Web Services infrastructure provider.", + "region": "region holds the default AWS region for new AWS resources created by the cluster.", +} + +func (AWSPlatformStatus) SwaggerDoc() map[string]string { + return map_AWSPlatformStatus +} + var map_Infrastructure = map[string]string{ "": "Infrastructure holds cluster-wide information about Infrastructure. The canonical name is `cluster`", "metadata": "Standard object's metadata.", @@ -731,7 +740,8 @@ func (InfrastructureSpec) SwaggerDoc() map[string]string { var map_InfrastructureStatus = map[string]string{ "": "InfrastructureStatus describes the infrastructure the cluster is leveraging.", "infrastructureName": "infrastructureName uniquely identifies a cluster with a human friendly name. Once set it should not be changed. Must be of max length 27 and must have only alphanumeric or hyphen characters.", - "platform": "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": "platform is the underlying infrastructure provider for the cluster.\n\nDeprecated: Use platformStatus.type instead.", + "platformStatus": "platformStatus holds status information specific to the underlying infrastructure provider.", "etcdDiscoveryDomain": "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", "apiServerURL": "apiServerURL is a valid URI with scheme(http/https), address and port. apiServerURL can be used by components like the web console to tell users where to find the Kubernetes API.", "apiServerInternalURI": "apiServerInternalURL is a valid URI with scheme(http/https), address and port. apiServerInternalURL can be used by components like kubelets, to contact the Kubernetes API server using the infrastructure provider rather than Kubernetes networking.", @@ -741,6 +751,16 @@ func (InfrastructureStatus) SwaggerDoc() map[string]string { return map_InfrastructureStatus } +var map_PlatformStatus = map[string]string{ + "": "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": "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.", + "aws": "AWS contains settings specific to the Amazon Web Services infrastructure provider.", +} + +func (PlatformStatus) SwaggerDoc() map[string]string { + return map_PlatformStatus +} + var map_Ingress = map[string]string{ "": "Ingress holds cluster-wide information about Ingress. The canonical name is `cluster`", "metadata": "Standard object's metadata.",