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

Drain pods created from ReplicaSets in 'kubectl drain' #23689

Merged
merged 1 commit into from
Apr 16, 2016
Merged

Drain pods created from ReplicaSets in 'kubectl drain' #23689

merged 1 commit into from
Apr 16, 2016

Conversation

maclof
Copy link
Contributor

@maclof maclof commented Mar 31, 2016

Refer to my issue here: #23531

@maclof maclof mentioned this pull request Mar 31, 2016
Closed
@k8s-bot
Copy link

k8s-bot commented Mar 31, 2016

Can one of the admins verify that this patch is reasonable to test? (reply "ok to test", or if you trust the user, reply "add to whitelist")

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

2 similar comments
@k8s-bot
Copy link

k8s-bot commented Mar 31, 2016

Can one of the admins verify that this patch is reasonable to test? (reply "ok to test", or if you trust the user, reply "add to whitelist")

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

@k8s-bot
Copy link

k8s-bot commented Mar 31, 2016

Can one of the admins verify that this patch is reasonable to test? (reply "ok to test", or if you trust the user, reply "add to whitelist")

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

@maclof
Copy link
Contributor Author

maclof commented Mar 31, 2016

Having some trouble getting the tests working, PTAL and let me know if I'm missing something

@k8s-github-robot k8s-github-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Mar 31, 2016
@janetkuo
Copy link
Member

@k8s-bot ok to test

@k8s-bot
Copy link

k8s-bot commented Apr 1, 2016

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in hack/jenkins/job-configs/kubernetes-jenkins-pull/ instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

@janetkuo
Copy link
Member

janetkuo commented Apr 1, 2016

@k8s-bot ok to test

@janetkuo
Copy link
Member

janetkuo commented Apr 1, 2016

@ixdy k8s-bot isn't responding to me. Can you add me to the admin list?

@ixdy
Copy link
Member

ixdy commented Apr 1, 2016

@janetkuo you're in the admin list. Can you try commenting "ok to test" without the @k8s-bot at the beginning of the line?

@k8s-bot
Copy link

k8s-bot commented Apr 1, 2016

GCE e2e build/test passed for commit 36fc6836797ac3bf5dbe3a7de1c68f420c80f77f.

@ixdy
Copy link
Member

ixdy commented Apr 1, 2016

Hm, of course that triggered it. I noticed that in the config your name is listed as JanetKuo, so maybe it cares about capitalization? I'll change it.

This was referenced Apr 1, 2016
Closed
Closed
@davidopp davidopp assigned mml and unassigned janetkuo Apr 5, 2016
@davidopp
Copy link
Member

davidopp commented Apr 5, 2016

@mml

@mml
Copy link
Contributor

mml commented Apr 5, 2016

@maclof can you please update the docs to match?

@mml
Copy link
Contributor

mml commented Apr 5, 2016

@ixdy The Jenkins unit/integration link is giving me 404 (after redirects). Any idea what's going on?

@maclof
Copy link
Contributor Author

maclof commented Apr 5, 2016

I've updated docs in this commit: maclof@cf5bc68 but for some reason it isn't showing up in this PR? o.o

@mml
Copy link
Contributor

mml commented Apr 5, 2016

@maclof also looks like the test failed.

--- FAIL: TestDrain (0.08s)
    drain_test.go:502: RS-managed pod: pod never deleted

mml
mml reviewed Apr 5, 2016
View reviewed changes
pkg/kubectl/cmd/drain_test.go
@@ -396,6 +436,8 @@ func TestDrain(t *testing.T) {
return &http.Response{StatusCode: 200, Body: objBody(testapi.Extensions.Codec(), &ds)}, nil
case m.isFor("GET", "/namespaces/default/jobs/job"):
return &http.Response{StatusCode: 200, Body: objBody(testapi.Extensions.Codec(), &job)}, nil
case m.isFor("GET", "/namespaces/default/replicasets/rs"):
return &http.Response{StatusCode: 200, Body: objBody(testapi.Extensions.Codec(), &test.rcs[0])}, nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you want &test.replicaSets[0]

@maclof
Copy link
Contributor Author

maclof commented Apr 5, 2016

@mml thanks, no idea how I missed that one.

@maclof maclof force-pushed the drain-replicaset-pods branch from cf5bc68 to 97267e3 Compare April 5, 2016 21:54
@k8s-bot
Copy link

k8s-bot commented Apr 5, 2016

GCE e2e build/test failed for commit 10c05ccb2ec13761fbb7931dfc812437c055be04.

Please reference the list of currently known flakes when examining this failure. If you request a re-test, you must reference the issue describing the flake.

@k8s-github-robot k8s-github-robot removed lgtm "Looks good to me", 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 Apr 15, 2016
@k8s-bot
Copy link

k8s-bot commented Apr 15, 2016

GCE e2e build/test passed for commit f2fd9aee08fb9f7a331157311b0e5ed822d1e0f7.

@maclof maclof force-pushed the drain-replicaset-pods branch from f2fd9ae to fdf4098 Compare April 15, 2016 07:51
@k8s-bot
Copy link

k8s-bot commented Apr 15, 2016

GCE e2e build/test passed for commit fdf4098.

@maclof
Copy link
Contributor Author

maclof commented Apr 15, 2016

@mml I rebased this morning, but something weird seems to be going on with the GCE Node e2e?

@mml mml added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 15, 2016
@mml
Copy link
Contributor

mml commented Apr 15, 2016

@k8s-bot re-test please #24297

@k8s-github-robot
Copy link

@mml
You must link to the test flake issue which caused you to request this manual re-test.
Re-test requests should be in the form of: k8s-bot test this issue: #<number>
Here is the list of open test flakes.

@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 15, 2016
@mml
Copy link
Contributor

mml commented Apr 15, 2016

k8s-bot test this issue: #24297

@mml mml added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 15, 2016
@k8s-bot
Copy link

k8s-bot commented Apr 15, 2016

GCE e2e build/test passed for commit fdf4098.

@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot
Copy link

k8s-bot commented Apr 16, 2016

GCE e2e build/test passed for commit fdf4098.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit fe6a7a2 into kubernetes:master Apr 16, 2016
@chancez
Copy link
Member

chancez commented May 2, 2016

Is it possible to get this cherry-picked into 1.2.x?

This seems like a pretty big oversight, and pretty much makes using drain impossible in 1.2 since many things have now moved to Deployments/ReplicaSets by default. Ex: heapster/eventer in upstream, but I'm sure other users are now using Deployments for their own things. Now we basically need to choose between having drain, or using deployments, which is quite unfortunate.

CC @mml

@davidopp davidopp added this to the v1.2 milestone May 4, 2016
@davidopp davidopp changed the title Drain pods created from ReplicaSets Drain pods created from ReplicaSets in ;kubectl drain' May 4, 2016
@davidopp davidopp changed the title Drain pods created from ReplicaSets in ;kubectl drain' Drain pods created from ReplicaSets in 'kubectl drain' May 4, 2016
@davidopp davidopp mentioned this pull request May 4, 2016
Merged
@zmerlynn zmerlynn added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label May 4, 2016
roberthbailey referenced this pull request May 5, 2016
@roberthbailey
Merge pull request #25157 from davidopp/automated-cherry-pick-of-#236…
032ed06 
…89-upstream-release-1.2

Drain pods created from ReplicaSets in 'kubectl drain'
@k8s-cherrypick-bot
Copy link

Commit found in the "release-1.2" branch appears to be this PR. Removing the "cherrypick-candidate" label. If this is an error find help to get your PR picked.

shyamjvs referenced this pull request in shyamjvs/kubernetes Dec 1, 2016
@roberthbailey
Merge pull request kubernetes#25157 from davidopp/automated-cherry-pi…
fab32d2 
…ck-of-#23689-upstream-release-1.2

Drain pods created from ReplicaSets in 'kubectl drain'
shouhong referenced this pull request in shouhong/kubernetes Feb 14, 2017
@roberthbailey
Merge pull request kubernetes#25157 from davidopp/automated-cherry-pi…
3de3be7 
…ck-of-#23689-upstream-release-1.2

Drain pods created from ReplicaSets in 'kubectl drain'
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@mml mml

Labels
cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
v1.2
Development

Successfully merging this pull request may close these issues.