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

Jump to bottom

Add --selector, --pod-selector flags oc adm drain #17616

Merged
merged 2 commits into from Dec 21, 2017
Merged

Add --selector, --pod-selector flags oc adm drain #17616

merged 2 commits into from Dec 21, 2017

Conversation

juanvallejo
Copy link
Contributor

@juanvallejo juanvallejo commented Dec 5, 2017 •
edited

Fixes #17554
Fixes #17563
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1525340

Picks kubernetes/kubernetes#52917, kubernetes/kubernetes#54083, and kubernetes/kubernetes#56864 to bring in --selector and --pod-selector flag support to oc adm drain.

cc @openshift/cli-review @deads2k @dustymabe

@openshift-merge-robot openshift-merge-robot added the vendor-update Touching vendor dir or related files label Dec 5, 2017
@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Dec 5, 2017
@soltysh
Copy link
Member

soltysh commented Dec 6, 2017

The changes itself lgtm, but there's a rebase in flight (#17576) which will pick the first 2 commits. The 3rd one is not accepted upstream yet and will wait till next week when queue is open. I'm holding this until after the rebase lands, in that case.
/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 6, 2017
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 14, 2017
@openshift-ci-robot openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 15, 2017
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 15, 2017
@deads2k
Copy link
Contributor

deads2k commented Dec 15, 2017

@juanvallejo still needed?

@juanvallejo
Copy link
Contributor Author

@deads2k yeah the pod-selector changes are the only thing new that this PR brings in post-rebase. Updated to be on par with kubernetes/kubernetes#56864

@mfojtik
Copy link
Member

mfojtik commented Dec 18, 2017

@juanvallejo this will need to be picked for 3.8 (see the BZ I assigned you today).

juanvallejo added a commit to juanvallejo/origin that referenced this pull request Dec 18, 2017
@juanvallejo
UPSTREAM: 56864: pick pod-selector changes from #56864: pick of origin …
08a0928 
…openshift#17616
@juanvallejo juanvallejo mentioned this pull request Dec 18, 2017
Merged
@juanvallejo
Copy link
Contributor Author

@mfojtik opened pick of these changes for 3.8 here: #17868

@juanvallejo
Copy link
Contributor Author

/test unit

@juanvallejo juanvallejo reopened this Dec 18, 2017
@deads2k
Copy link
Contributor

deads2k commented Dec 19, 2017

looks up to date

/hold cancel
/lgtm

@openshift-ci-robot openshift-ci-robot added lgtm Indicates that a PR is ready to be merged. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Dec 19, 2017
@openshift-ci-robot
Copy link

[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

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 19, 2017
@juanvallejo
Copy link
Contributor Author

/test extended_conformance_gce

2 similar comments
@juanvallejo
Copy link
Contributor Author

/test extended_conformance_gce

@juanvallejo
Copy link
Contributor Author

/test extended_conformance_gce

@juanvallejo
Copy link
Contributor Author

/test extended_conformance_install

@soltysh
Copy link
Member

soltysh commented Dec 20, 2017

/retest

@juanvallejo
Copy link
Contributor Author

/test extended_conformance_install

@dustymabe
Copy link
Member

I must say this is really hard to watch

@juanvallejo
Copy link
Contributor Author

@dustymabe test flakes. test flakes everywhere

/retest

@soltysh
Copy link
Member

soltysh commented Dec 21, 2017

/retest

@openshift-merge-robot
Copy link
Contributor

Automatic merge from submit-queue (batch tested with PRs 17072, 17616).

openshift-merge-robot added a commit that referenced this pull request Dec 21, 2017
@openshift-merge-robot
Merge pull request #17616 from juanvallejo/jvallejo/add-selector-oc-d…
d8c46e4 
…rain

Automatic merge from submit-queue (batch tested with PRs 17072, 17616).

Add --selector, --pod-selector flags `oc adm drain`

Fixes #17554
Fixes #17563
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1525340

Picks kubernetes/kubernetes#52917, kubernetes/kubernetes#54083, and kubernetes/kubernetes#56864 to bring in `--selector` and `--pod-selector` flag support to `oc adm drain`.

cc @openshift/cli-review @deads2k @dustymabe
@openshift-merge-robot openshift-merge-robot merged commit d8c46e4 into openshift:master Dec 21, 2017
@juanvallejo juanvallejo deleted the jvallejo/add-selector-oc-drain branch December 21, 2017 15:03
openshift-merge-robot added a commit that referenced this pull request Jan 8, 2018
@openshift-merge-robot
Merge pull request #17868 from juanvallejo/jvallejo/pick-17616-pod-se…
91f500a 
…lector

Automatic merge from submit-queue.

Pick 17616: Add --selector, --pod-selector flags `oc adm drain`

UPSTREAM kubernetes/kubernetes#56864
Pick of #17616
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1525340

cc @mfojtik @deads2k @soltysh
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smarterclayton smarterclayton Awaiting requested review from smarterclayton

@soltysh soltysh Awaiting requested review from soltysh

Assignees

@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/M Denotes a PR that changes 30-99 lines, ignoring generated files. vendor-update Touching vendor dir or related files
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@juanvallejo @soltysh @deads2k @mfojtik @openshift-ci-robot @dustymabe @openshift-merge-robot