Skip to content

Conversation

@sinnykumari
Copy link
Contributor

cloudConfig is now generated by kube_cloud_config controller for all supported
platforms. Controller generates kube-cloud-config ConfigMap in openshift-config-managed
namespace where cloud.conf key is stored.

Links:

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

/cc sdodson

@sinnykumari
Copy link
Contributor Author

/cc @sdodson

@sinnykumari sinnykumari force-pushed the cloudconfig branch 3 times, most recently from ca365ee to ac77ea8 Compare April 16, 2020 17:05
@sinnykumari
Copy link
Contributor Author

/retest

1 similar comment
@sinnykumari
Copy link
Contributor Author

/retest

@sdodson
Copy link
Member

sdodson commented Apr 16, 2020

/assign @runcom @abhinavdahiya

@sinnykumari
Copy link
Contributor Author

/skip
/test e2e-gcp-op
/test e2e-gcp-upgrade

@runcom
Copy link
Member

runcom commented Apr 17, 2020

/retest

@sinnykumari sinnykumari force-pushed the cloudconfig branch 2 times, most recently from cb5a911 to 31799e4 Compare April 17, 2020 09:59
@sinnykumari
Copy link
Contributor Author

interesting, since 30 mins jobs are at waiting for importing pipeline:base ...

@sinnykumari
Copy link
Contributor Author

/retest

1 similar comment
@sinnykumari
Copy link
Contributor Author

/retest

@LorbusChris
Copy link
Contributor

@sinnykumari @runcom this might be a good opportunity to move to using properly typed PlatformType identifiers instead of the untyped strings we're using (and that are already marked as wrong by the TODO comment).

This commit here from another PR migrates the Controller over to use them, it might serve you as a reference: 27502d5

The PlatformType fields already defined in ControllerConfig.Infra.Status.PlatformStatus.Type, it's just that the Controller doesn't process them, and looks up that info in the ControllerConfig.Platform strings instead.

@sinnykumari
Copy link
Contributor Author

/retest

@abhinavdahiya
Copy link
Contributor

abhinavdahiya commented Apr 17, 2020

https://github.com/openshift/api/pull/621/files#diff-cee9360e003319719376a6c9189c2e48R29-R37

so the MCO should be reading from openshift-managed-config/kube-cloud-config configmap cloud.conf key. that's the expectation of the API.

so, therefore, don't look at the spec.cloudConfig.name

so personally this should be

get configmap openshift-managed-config/kube-cloud-config
if err 
    if err is NotFound
        if cloud config required for platform
            return err
use the cloud.conf key to configure the kubelet.
is cloud config required for platform:
if spec.cloudConfig.name != "" return true
if platform in list [] return true
return false

@sinnykumari
Copy link
Contributor Author

Thanks Abhinav for the feedback, made changes accordingly.
Let me know if I missed something :)

@sinnykumari
Copy link
Contributor Author

/bugzilla refresh

@openshift-ci-robot openshift-ci-robot added bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. and removed bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Apr 20, 2020
@openshift-ci-robot
Copy link
Contributor

@sinnykumari: This pull request references Bugzilla bug 1825948, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.5.0) matches configured target release for branch (4.5.0)
  • bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)
Details

In response to this:

/bugzilla refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@kikisdeliveryservice
Copy link
Contributor

/skip

@sinnykumari
Copy link
Contributor Author

sinnykumari commented Apr 21, 2020

PR referencing bug https://bugzilla.redhat.com/show_bug.cgi?id=1825944 has been merged.
/retest

@sinnykumari
Copy link
Contributor Author

/retest

@LorbusChris
Copy link
Contributor

/retest

@LorbusChris
Copy link
Contributor

/test e2e-gcp-op

@sinnykumari
Copy link
Contributor Author

Yay, tests are passing now!

@runcom
Copy link
Member

runcom commented Apr 23, 2020

@abhinavdahiya ptal

…ud-config ConfigMap

cloudConfig is now generated by kube_cloud_config controller for all supported
platforms. Controller generates kube-cloud-config ConfigMap in openshift-config-managed
namespace where cloud.conf key is stored.

Links:
- https://github.com/openshift/enhancements/blob/master/enhancements/installer/aws-custom-region-and-endpoints.md
- openshift/api#599
- openshift/api#621
@openshift-ci-robot
Copy link
Contributor

@sinnykumari: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/e2e-aws-scaleup-rhel7 e7455dc link /test e2e-aws-scaleup-rhel7

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@runcom
Copy link
Member

runcom commented Apr 24, 2020

/skip

@abhinavdahiya
Copy link
Contributor

/lgtm

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

[APPROVALNOTIFIER] This PR is APPROVED

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

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

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 5230988 into openshift:master Apr 24, 2020
@openshift-ci-robot
Copy link
Contributor

@sinnykumari: All pull requests linked via external trackers have merged: openshift/machine-config-operator#1658. Bugzilla bug 1825948 has been moved to the MODIFIED state.

Details

In response to this:

Bug 1825948: cloudConfig: read cloud Config from openshift-config-managed/kube-cloud-config ConfigMap

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

abhinavdahiya added a commit to abhinavdahiya/machine-config-operator that referenced this pull request Aug 3, 2020
Previously the cloud provider config map was sourced from the `.spec.cloudConf` of the infrastructure object.
but to enable user input and stitching explained in [1] and [2], we added a controller to generate a cloud configuraration using the spec
for use by kubelet and kube cloud controller manager. The new API defined in [3] creates a config map `openshift-config-managed/kube-cloud-config` with
the configuration in `cloud.conf` key. We updated the in-cluster MCO to use the new API [4] but the MCO on bootstrap host was left untouched because the controller
could not generated on the bootstrap host yet.

But [5] and [6] intend to provide the same generated config map on the bootstrap host for MCO, and therefore we need the bootstrap MCO to read the key defined by
the generated API but fallback to he spec key (old behavior) for backward compatibility.

[1]: https://github.com/openshift/enhancements/blob/master/enhancements/installer/aws-custom-region-and-endpoints.md
[2]: https://github.com/openshift/enhancements/blob/master/enhancements/installer/azure-support-known-cloud-environments.md
[3]: https://github.com/openshift/api/blob/e21882127f24772e3b5388fe1fdc37669e8e1d04/config/v1/types_infrastructure.go#L26-L40
[4]: openshift#1658
[5]: openshift/cluster-config-operator#140
[6]: openshift/installer#3831
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. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants