Bug 1864677: operator/bootstrap: use correct key for generated cloud conf #1973
Conversation
pkg/operator/bootstrap.go
Outdated
There was a problem hiding this comment.
Adding info message when we are doing fallback will be nice for debugging.
|
Overall lgtm, one minor comment. Also, Verify test needs to be fixed. /retest |
|
adding hold since we are post-FF if this is a bug @abhinavdahiya please open and attach BZ /hold |
openshift-ci-robot
added
the
do-not-merge/hold
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
abhinavdahiya
force-pushed
the
bootstrap_fix_cloud_conf_key
branch
from
3e3a3b4 to
344fb9e
Compare
abhinavdahiya
changed the title
openshift-ci-robot
added
the
bugzilla/severity-high
|
@abhinavdahiya: This pull request references Bugzilla bug 1864677, 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
DetailsIn response to this:
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. |
openshift-ci-robot
added
the
bugzilla/valid-bug
added the BZ |
openshift-ci-robot
removed
the
do-not-merge/hold
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: abhinavdahiya, cgwalters The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
|
/retest Please review the full test history for this PR and help us cut down flakes. |
3 similar comments
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
@abhinavdahiya: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
@abhinavdahiya: Some pull requests linked via external trackers have merged: openshift/machine-config-operator#1973. The following pull requests linked via external trackers have not merged:
DetailsIn response to this:
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. |
7 participants
Previously the cloud provider config map was sourced from the
.spec.cloudConfof 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-configwiththe configuration in
cloud.confkey. We updated the in-cluster MCO to use the new API 4 but the MCO on bootstrap host was left untouched because the controllercould 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.
/cc @sinnykumari
since this is an extension to 4