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

Remove overwrite_bootstrappolicy and pkg/cmd/server/admin/legacyetcd #17336

Merged
merged 2 commits into from
Nov 21, 2017
Merged

Remove overwrite_bootstrappolicy and pkg/cmd/server/admin/legacyetcd #17336

merged 2 commits into from
Nov 21, 2017

Conversation

juanvallejo
Copy link
Contributor

@juanvallejo juanvallejo commented Nov 15, 2017 •
edited
Loading

Fixes #15817

This patch solves a few of the items (currently checked) from #17309
Removes the file pkg/cmd/server/admin/overwrite_bootstrappolicy.go
which has been deprecated (and had some dependencies on pkg/oc.
It also removes the deprecated package pkg/cmd/server/admin/legacyetcd.

cc @deads2k @liggitt @openshift/cli-review

@openshift-ci-robot openshift-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Nov 15, 2017
@juanvallejo juanvallejo mentioned this pull request Nov 15, 2017
Closed
19 tasks
@liggitt
Copy link
Contributor

liggitt commented Nov 15, 2017

LGTM
@openshift/sig-security any concerns?

@enj
Copy link
Contributor

enj commented Nov 15, 2017

LGTM

@juanvallejo please confirm that you traced all of the dependencies, this should fix #15817

@deads2k
Copy link
Contributor

deads2k commented Nov 16, 2017

@liggitt will you allow it pre-rebase?

@deads2k
Copy link
Contributor

deads2k commented Nov 16, 2017

@mfojtik manually checked. Nothing crazy in the rebase regarding this. tagging.

/lgtm

@juanvallejo get it green.

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 16, 2017
@openshift-merge-robot openshift-merge-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 16, 2017
@simo5
Copy link
Contributor

simo5 commented Nov 16, 2017

@juanvallejo run make update and resubmit ?

@deads2k
Copy link
Contributor

deads2k commented Nov 16, 2017

And test-cmd failure is legit

@deads2k deads2k closed this Nov 16, 2017
@deads2k deads2k reopened this Nov 16, 2017
@juanvallejo juanvallejo force-pushed the jvallejo/rm-overwrite-bootstrap-policy branch from 22c53b2 to 0b198e8 Compare November 16, 2017 15:06
@openshift-merge-robot openshift-merge-robot removed the lgtm Indicates that a PR is ready to be merged. label Nov 16, 2017
@juanvallejo
Copy link
Contributor Author

/test extended_networking_minimal

@simo5
Copy link
Contributor

simo5 commented Nov 16, 2017

@juanvallejo you need to remove the test that explicitly tests the command you just removed I guess :-)

@juanvallejo
Copy link
Contributor Author

/test extended_conformance_gce

1 similar comment
@juanvallejo
Copy link
Contributor Author

/test extended_conformance_gce

@deads2k
Copy link
Contributor

deads2k commented Nov 16, 2017

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 16, 2017
@openshift-merge-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, juanvallejo

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

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@juanvallejo juanvallejo mentioned this pull request Nov 16, 2017
Merged
19 tasks
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@juanvallejo
Copy link
Contributor Author

/test extended_conformance_gce

@0xmichalis 0xmichalis removed their assignment Nov 17, 2017
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

1 similar comment
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

1 similar comment
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 20, 2017
@juanvallejo juanvallejo force-pushed the jvallejo/rm-overwrite-bootstrap-policy branch from e197a86 to 011b1ca Compare November 20, 2017 20:34
@openshift-merge-robot openshift-merge-robot removed lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Nov 20, 2017
@juanvallejo juanvallejo added the lgtm Indicates that a PR is ready to be merged. label Nov 20, 2017
@juanvallejo
Copy link
Contributor Author

/retest

@openshift-merge-robot
Copy link
Contributor

Automatic merge from submit-queue.

@openshift-merge-robot openshift-merge-robot merged commit 128e9f4 into openshift:master Nov 21, 2017
@openshift-ci-robot
Copy link

@juanvallejo: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
ci/openshift-jenkins/extended_conformance_install_update 011b1ca link /test extended_conformance_install_update
ci/openshift-jenkins/extended_conformance_gce 011b1ca link /test extended_conformance_gce

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.

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.

@juanvallejo juanvallejo deleted the jvallejo/rm-overwrite-bootstrap-policy branch November 27, 2017 15:26
openshift-merge-robot added a commit that referenced this pull request Nov 29, 2017
@openshift-merge-robot
Merge pull request #17356 from juanvallejo/jvallejo/move-clientcmd-pk…
f55c2b9 
…g-oc

Automatic merge from submit-queue.

move pkg/cmd/util/clientcmd -> pkg/oc/cli/util/clientcmd

This patch *partially* solves a few of the items (currently checked) from #17309 
Now that `clientcmd` (which includes printer factory methods) is moved into `pkg/oc`,
the following files outside of `pkg/oc` need to have their dependency on `clientcmd` broken (this will be done in a separate PR):

- [x] pkg/cmd/server/origin/controller/config.go (#17357)
- [x] pkg/cmd/server/admin/create_error_template.go   (#17357)
- [x] pkg/cmd/server/admin/create_bootstrap_project_template.go (#17357)
- [x] pkg/cmd/server/admin/overwrite_bootstrappolicy.go (*this file has been removed by* #17336)
- [x] pkg/cmd/server/admin/create_login_template.go (#17357)
- [x] pkg/cmd/server/admin/create_provider_selection_template.go (#17357)
- [x] **pkg/cmd/infra/router/template.go** (Wanted by: `pkg/cmd/openshift/openshift.go` (no other dependents)) (#17357)
- [x] **pkg/cmd/infra/router/f5.go** (Wanted by: `pkg/cmd/openshift/openshift.go` (no other dependents)) (#17357)
- [x] pkg/cmd/openshift/openshift.go (#17486 and #17482)
- [x] pkg/cmd/dockerregistry/dockerregistry.go (Wanted by: `cmd/dockerregistry/main.go` (depends on `clientcmd.Config`)) (#17357)
- [x] *pkg/diagnostics/networkpod/util/util.go* (Can  be addressed by moving `pkg/diagnostics` inside existing `pkg/oc/admin/diagnostics`) #17393
- [x] *pkg/diagnostics/client/config_contexts.go* (Can  be addressed by moving `pkg/diagnostics` inside existing `pkg/oc/admin/diagnostics`) #17393
- [x] *pkg/diagnostics/client/run_diagnostics_pod.go* (Can  be addressed by moving `pkg/diagnostics` inside existing `pkg/oc/admin/diagnostics`) #17393
- [x] *pkg/diagnostics/pod/auth.go* (Can  be addressed by moving `pkg/diagnostics` inside existing `pkg/oc/admin/diagnostics`) #17393
- [x] *pkg/diagnostics/network/run_pod.go* (Can be addressed by moving `pkg/diagnostics` inside existing `pkg/oc/admin/diagnostics`) #17393
- [x] pkg/ipfailover/keepalived/plugin.go (moved to `pkg/oc/experimental`) (#17357)
- [x] pkg/federation/kubefed/kubefed.go (#17357)
- [x] pkg/dockerregistry/server/client/client.go (#17357)
- [x] pkg/dockerregistry/server/auth_test.go (#17357)

**bold** = depends on `clientcmd.Config` (not sure what to do about this) AND only dependent is `pkg/cmd/openshift/openshift.go`

cc @deads2k @openshift/cli-review @liggitt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@mfojtik mfojtik

@deads2k deads2k

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/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@juanvallejo @liggitt @enj @deads2k @simo5 @openshift-merge-robot @openshift-bot @openshift-ci-robot @mfojtik @0xmichalis