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

cmd/openshift-install/waitfor: Rename from user-provided-infrastructure #1552

Merged
merged 1 commit into from
Apr 8, 2019

Conversation

wking
Copy link
Member

@wking wking commented Apr 8, 2019

There is no hard line between installer- and user-provided infrastructure. Rename these commands to focus on what they'll do instead of the work-flow into which we expect them to fit.

We're still working out how we can drop the router-CA injection to avoid wait-for cluster-ready surprising users my editing their on-disk kubeconfig (#1541). But that's mitigated somewhat by the fact that addRouterCAToClusterCA is idempotent, because AppendCertsFromPEM wraps AddCert and AddCert checks to avoid duplicate certificates.

I haven't provided backwards-compat shims because the UPI workflow is so young that I don't expect users who rely on its stability.

@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 8, 2019
@wking
Copy link
Member Author

wking commented Apr 8, 2019

/assign @abhinavdahiya

@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 8, 2019
@abhinavdahiya
Copy link
Contributor

@wking can you update other users of upi like abhinavdahiya@16b7301

There is no hard line between installer- and user-provided
infrastructure.  Rename these commands to focus on what they'll do
instead of the work-flow into which we expect them to fit.

We're still working out how we can drop the router-CA injection to
avoid 'wait-for cluster-ready' surprising users my editing their
on-disk kubeconfig [1].  But that's mitigated somewhat by the fact
that addRouterCAToClusterCA is idempotent, because AppendCertsFromPEM
wraps AddCert [2] and AddCert checks to avoid duplicate certificates
[3].

[1]: openshift#1541
[2]: https://github.com/golang/go/blob/go1.12/src/crypto/x509/cert_pool.go#L144
[3]: https://github.com/golang/go/blob/go1.12/src/crypto/x509/cert_pool.go#L106-L109
@wking
Copy link
Member Author

wking commented Apr 8, 2019

... can you update other users of upi...

That's what you get for using the convenient alias instead of the full subcommand name ;). But yeah, done with 2a39d8b -> 8cd13c0.

@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 8, 2019
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

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

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,wking]

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

@wking
Copy link
Member Author

wking commented Apr 8, 2019

e2e-aws:

Flaky tests:

[Feature:Platform][Smoke] Managed cluster should start all core operators [Suite:openshift/conformance/parallel]
[Feature:Prometheus][Conformance] Prometheus when installed on the cluster should start and expose a secured proxy and unsecured metrics [Suite:openshift/conformance/parallel/minimal]

Failing tests:

[sig-storage] In-tree Volumes [Driver: hostPath] [Testpattern: Dynamic PV (default fs)] provisioning should provision storage with mount options [Suite:openshift/conformance/parallel] [Suite:k8s]

/retest

@openshift-merge-robot openshift-merge-robot merged commit d427396 into openshift:master Apr 8, 2019
@wking wking deleted the wait-for-renames branch April 10, 2019 20:30
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. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants