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

installer: azure support all known cloud environments #321

Merged
merged 1 commit into from
Jul 24, 2020

Conversation

abhinavdahiya
Copy link
Contributor

@abhinavdahiya abhinavdahiya commented May 12, 2020

The enhancement details the necessary change for supporting installing to MAG like known Azure cloud environments.
known <=> known to Azure SDKs like the Azure SDK Go.

The enhancement specifically does outline the support for cloud environments on Azure that would require the user to provide a resource endpoint url to discover the environment details like AzureStack. That would require a lot of extra configuration on the installer internals side so that will be handles separately. This enhancement is still valuable as it unblocks MAG.

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 12, 2020
@abhinavdahiya
Copy link
Contributor Author

/cc @openshift/openshift-team-installer

@abhinavdahiya
Copy link
Contributor Author

/cc @sdodson @brenton @katherinedube


### Implementation Details/Notes/Constraints [optional]

#### Infrastructure global configuration
Copy link
Contributor Author

Choose a reason for hiding this comment

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

cc @openshift/api-reviewers

since this recommends a change to config/v1 object.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sdodson
Copy link
Member

sdodson commented May 13, 2020

/approve

staebler added a commit to staebler/api that referenced this pull request May 19, 2020
Add the `.status.azure.cloudName` field to the Infrastructure
type in config. This field informs in which Azure cloud environment
the cluster is installed and consequently which Azure API endpoint
should be used when communicating via the Azure SDK.

See openshift/enhancements#321

https://issues.redhat.com/browse/CORS-1441
staebler added a commit to staebler/api that referenced this pull request May 20, 2020
Add the `.status.platformStatus.azure.cloudName` field to the Infrastructure
type in config. This field informs in which Azure cloud environment
the cluster is installed and consequently which Azure API endpoint
should be used when communicating via the Azure SDK.

See openshift/enhancements#321

https://issues.redhat.com/browse/CORS-1441
staebler added a commit to staebler/api that referenced this pull request May 20, 2020
Add the `.status.platformStatus.azure.cloudName` field to the Infrastructure
type in config. This field informs in which Azure cloud environment
the cluster is installed and consequently which Azure API endpoint
should be used when communicating via the Azure SDK.

See openshift/enhancements#321

https://issues.redhat.com/browse/CORS-1441
staebler added a commit to staebler/cluster-config-operator that referenced this pull request May 20, 2020
Populate the `cloud` field in kube cloud.conf stored in the kube-cloud-config
ConfigMap in the openshift-config-managed namespace. This field informs in
which Azure cloud environment the cluster is installed and consequently which
Azure API endpoint should be used when communicating via the Azure SDK.

The value populated comes from the `.status.platformStatus.azure.cloudName`
field of the infrastructure.config.openshift.io resource. If the field is
empty or missing, then the default "AZUREPUBLICCLOUD" value is set for the
`cloud` field in the kube cloud config. If the field generated from the
infrastructure resource conflicts with the field in the user-provided kube
cloud config, then the controller will error on syncing the infrastructure
resource.

See openshift/enhancements#321

https://issues.redhat.com/browse/CORS-1444
staebler added a commit to staebler/cluster-config-operator that referenced this pull request May 20, 2020
Populate the `cloud` field in kube cloud.conf stored in the kube-cloud-config
ConfigMap in the openshift-config-managed namespace. This field informs in
which Azure cloud environment the cluster is installed and consequently which
Azure API endpoint should be used when communicating via the Azure SDK.

The value populated comes from the `.status.platformStatus.azure.cloudName`
field of the infrastructure.config.openshift.io resource. If the field is
empty or missing, then the default "AZUREPUBLICCLOUD" value is set for the
`cloud` field in the kube cloud config. If the field generated from the
infrastructure resource conflicts with the field in the user-provided kube
cloud config, then the controller will error on syncing the infrastructure
resource.

See openshift/enhancements#321

https://issues.redhat.com/browse/CORS-1444

#### Kube Cloud Config Controller

Since various kubernetes components like the kube-apiserver, kubelet (machine-config-operator), kube-controller-manager, cloud-controller-managers use the `.spec.cloudConfig` Config Map reference for cloud provider specific configurations, a new controller kube-cloud-config was introduced previously. The controller will perform the task of stitching configuration from Infrastructure object with the rest of the cloud config, such that all the kubernetes components can continue to directly consume a Config Map for configuration.
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel that I may be missing something regarding why the stitching is necessary. How is the cloud name different from, say, the resource group, with regards to how it is managed?

In the case of the resource group, the installer sets the resource group in both the cloud-provider-config ConfigMap and the infrastructure resource. The kube-cloud-config controller does not bother with stitching the resource group into the kube cloud config. I am assuming that, for the cloud name, the installer would do the same with setting the cloud in both the cloud-provider-config ConfigMap and the infrastructure resource. If so, why is the subsequent stitching needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The stitching is required using kube_cloud_config when the user can edit the option as day-2, so that any change the user makes to the infra object after installation reflects in the cloud config.

Given that cloudName does not fall into this category as the users are not allowed to change these yet. But we have near time goal to allow users to install to custom cloud like when using Azure stack hub or change gov cloud to custom endpoints, and that would allow the users to modify the cloudname as part of spec and then we would require this stitching.

Therefore i has added the requirement.

staebler added a commit to staebler/cluster-config-operator that referenced this pull request May 20, 2020
Populate the `cloud` field in kube cloud.conf stored in the kube-cloud-config
ConfigMap in the openshift-config-managed namespace. This field informs in
which Azure cloud environment the cluster is installed and consequently which
Azure API endpoint should be used when communicating via the Azure SDK.

The value populated comes from the `.status.platformStatus.azure.cloudName`
field of the infrastructure.config.openshift.io resource. If the field is
empty or missing, then the default "AZUREPUBLICCLOUD" value is set for the
`cloud` field in the kube cloud config. If the field generated from the
infrastructure resource conflicts with the field in the user-provided kube
cloud config, then the controller will error on syncing the infrastructure
resource.

See openshift/enhancements#321

https://issues.redhat.com/browse/CORS-1444

### Version Skew Strategy

The changes to the API objects are backwards compatible and therefore there shouldn't be any specific concerns w.r.t to upgrades or downgrades.
Copy link
Member

Choose a reason for hiding this comment

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

Should we point out that growing new clouds is not an issue here, because the Infrastructure property is immutable status?

staebler added a commit to staebler/installer that referenced this pull request Jun 3, 2020
Add the `azure.cloudName` field to the installconfig. This field
directs which Azure cloud environment is used for the cluster.

The `status.platformStatus.azure.cloudName` field is set in the
infrastructure.config.openshift.io resource to match the cloud name
configured in the installconfig.

The `cloud` field is set in the cloud-provider-config ConfigMap
to match the cloud name configured in the installconfig.

See openshift/enhancements#321

https://issues.redhat.com/browse/CORS-1442
staebler added a commit to staebler/cluster-config-operator that referenced this pull request Jun 15, 2020
Populate the `cloud` field in kube cloud.conf stored in the kube-cloud-config
ConfigMap in the openshift-config-managed namespace. This field informs in
which Azure cloud environment the cluster is installed and consequently which
Azure API endpoint should be used when communicating via the Azure SDK.

The value populated comes from the `.status.platformStatus.azure.cloudName`
field of the infrastructure.config.openshift.io resource. If the field is
empty or missing, then the default "AZUREPUBLICCLOUD" value is set for the
`cloud` field in the kube cloud config. If the field generated from the
infrastructure resource conflicts with the field in the user-provided kube
cloud config, then the controller will error on syncing the infrastructure
resource.

See openshift/enhancements#321

https://issues.redhat.com/browse/CORS-1444
staebler added a commit to staebler/cloud-credential-operator that referenced this pull request Jun 15, 2020
Use the Azure cloud environment specified by the
`status.platformStatus.azure.cloudName` field of the
infrastructure.config.openshift.io resource.

See openshift/enhancements#321
staebler added a commit to staebler/cloud-credential-operator that referenced this pull request Jun 15, 2020
Bump to the latest of github.com/openshift/api to pull in the
addition of the `status.platformStatus.azure.cloudName`
field of the infrastructure.config.openshift.io in order to
support other Azure cloud environments.

Repalced github.com/jteeuwen/go-bindata with
github.com/go-bindata/go-bindata.

Manually bumped the following.
  k8s.io/client-go v0.18.2
  sigs.k8s.io/controller-runtime v0.6.0

Changes from openshift/api#650

See openshift/enhancements#321
staebler added a commit to staebler/cluster-ingress-operator that referenced this pull request Jun 15, 2020
Use the Azure cloud environment specified by the
`status.platformStatus.azure.cloudName` field of the
infrastructure.config.openshift.io resource.

See openshift/enhancements#321
staebler added a commit to staebler/cluster-ingress-operator that referenced this pull request Jun 15, 2020
Bump to the latest of github.com/openshift/api to pull in the
addition of the `status.platformStatus.azure.cloudName`
field of the infrastructure.config.openshift.io in order to
support other Azure cloud environments.

Changes from openshift/api#650

See openshift/enhancements#321
staebler added a commit to staebler/cluster-config-operator that referenced this pull request Jun 16, 2020
Populate the `cloud` field in kube cloud.conf stored in the kube-cloud-config
ConfigMap in the openshift-config-managed namespace. This field informs in
which Azure cloud environment the cluster is installed and consequently which
Azure API endpoint should be used when communicating via the Azure SDK.

The value populated comes from the `.status.platformStatus.azure.cloudName`
field of the infrastructure.config.openshift.io resource. If the field is
empty or missing, then the default "AzurePublicCloud" value is set for the
`cloud` field in the kube cloud config. If the field generated from the
infrastructure resource conflicts with the field in the user-provided kube
cloud config, then the controller will error on syncing the infrastructure
resource.

See openshift/enhancements#321

https://issues.redhat.com/browse/CORS-1444
staebler added a commit to staebler/cluster-config-operator that referenced this pull request Jun 16, 2020
Populate the `cloud` field in kube cloud.conf stored in the kube-cloud-config
ConfigMap in the openshift-config-managed namespace. This field informs in
which Azure cloud environment the cluster is installed and consequently which
Azure API endpoint should be used when communicating via the Azure SDK.

The value populated comes from the `.status.platformStatus.azure.cloudName`
field of the infrastructure.config.openshift.io resource. If the field is
empty or missing, then the default "AzurePublicCloud" value is set for the
`cloud` field in the kube cloud config. If the field generated from the
infrastructure resource conflicts with the field in the user-provided kube
cloud config, then the controller will error on syncing the infrastructure
resource.

See openshift/enhancements#321

https://issues.redhat.com/browse/CORS-1444
patrickdillon pushed a commit to patrickdillon/installer that referenced this pull request Jun 17, 2020
Add the `azure.cloudName` field to the installconfig. This field
directs which Azure cloud environment is used for the cluster.

The `status.platformStatus.azure.cloudName` field is set in the
infrastructure.config.openshift.io resource to match the cloud name
configured in the installconfig.

The `cloud` field is set in the cloud-provider-config ConfigMap
to match the cloud name configured in the installconfig.

See openshift/enhancements#321

https://issues.redhat.com/browse/CORS-1442
staebler added a commit to staebler/cluster-config-operator that referenced this pull request Jun 23, 2020
Populate the `cloud` field in kube cloud.conf stored in the kube-cloud-config
ConfigMap in the openshift-config-managed namespace. This field informs in
which Azure cloud environment the cluster is installed and consequently which
Azure API endpoint should be used when communicating via the Azure SDK.

The value populated comes from the `.status.platformStatus.azure.cloudName`
field of the infrastructure.config.openshift.io resource. If the field is
empty or missing, then the default "AzurePublicCloud" value is set for the
`cloud` field in the kube cloud config. If the field generated from the
infrastructure resource conflicts with the field in the user-provided kube
cloud config, then the controller will error on syncing the infrastructure
resource.

See openshift/enhancements#321

https://issues.redhat.com/browse/CORS-1444
staebler added a commit to staebler/cluster-ingress-operator that referenced this pull request Jul 12, 2020
Use the Azure cloud environment specified by the
`status.platformStatus.azure.cloudName` field of the
infrastructure.config.openshift.io resource.

See openshift/enhancements#321
staebler added a commit to staebler/cluster-ingress-operator that referenced this pull request Jul 12, 2020
Bump to the latest of github.com/openshift/api to pull in the
addition of the `status.platformStatus.azure.cloudName`
field of the infrastructure.config.openshift.io in order to
support other Azure cloud environments.

Changes from openshift/api#650

See openshift/enhancements#321
@crawford
Copy link
Contributor

This approach looks good to me. There are a few pending comments, so I won't leave the /lgtm just yet.

@abhinavdahiya
Copy link
Contributor Author

@crawford @sdodson updated the enhancement to fix all the outstanding comments.

staebler added a commit to staebler/cluster-ingress-operator that referenced this pull request Jul 22, 2020
Use the Azure cloud environment specified by the
`status.platformStatus.azure.cloudName` field of the
infrastructure.config.openshift.io resource.

See openshift/enhancements#321
staebler added a commit to staebler/cluster-ingress-operator that referenced this pull request Jul 22, 2020
Bump to the latest of github.com/openshift/api to pull in the
addition of the `status.platformStatus.azure.cloudName`
field of the infrastructure.config.openshift.io in order to
support other Azure cloud environments.

Changes from openshift/api#650

See openshift/enhancements#321
@crawford
Copy link
Contributor

Looks good to me.

@sdodson
Copy link
Member

sdodson commented Jul 24, 2020

/lgtm
I didn't see any questions unanswered or any dissenting opinions.

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 24, 2020
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinavdahiya, sdodson

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [abhinavdahiya,sdodson]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-robot openshift-merge-robot merged commit cf57cd4 into openshift:master Jul 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet