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.

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

Jump to bottom

config/v1/types_infrastructure: Add AWS Region #300

Merged
merged 2 commits into from
May 7, 2019
Merged

config/v1/types_infrastructure: Add AWS Region #300

merged 2 commits into from
May 7, 2019

Commits on May 7, 2019

  1. 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]: openshift#300 (comment)
[6]: openshift#300 (comment)
    @wking
    wking committed May 7, 2019
    Configuration menu
    Copy the full SHA
    dedfb47 View commit details
    Browse the repository at this point in the history

  2. generated 

    With:

  $ make generate
    @wking
    wking committed May 7, 2019
    Configuration menu
    Copy the full SHA
    78e39eb View commit details
    Browse the repository at this point in the history