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

Remove duplicated code in secret e2e tests #33978

Merged
merged 1 commit into from
Oct 7, 2016

Conversation

rata
Copy link
Member

@rata rata commented Oct 3, 2016

What this PR does / why we need it: This come up when writing another PR: #28936 as a comment from @thockin.

Which issue this PR fixes (optional, in fixes #<issue number>(, #<issue_number>, ...) format, will close that issue when PR gets merged):

Special notes for your reviewer:

Release note:

This patch just removes duplicated code in secret e2e tests.


This change is Reviewable

@k8s-ci-robot
Copy link
Contributor

Can a kubernetes member verify that this patch is reasonable to test? If so, please reply with "@k8s-bot ok to test" on its own line.

Regular contributors should join the org to skip this step.

@rata rata changed the title [DO NOT MERGE] WIP: Remove duplicated code in secret e2e tests Remove duplicated code in secret e2e tests Oct 3, 2016
@rata
Copy link
Member Author

rata commented Oct 3, 2016

This may fail a few times as it's not easy for me to test it in my old hardware. But if you can authorize it to test, it would be great :)

Also, configmap e2e are way more extensive than secrets. It's easy to add similar tests on secrets while doing this functions, so let me know if it's ok to do it

@k8s-github-robot k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. release-note-label-needed labels Oct 3, 2016
@fejta fejta added the sig/storage Categorizes an issue or PR as relevant to SIG Storage. label Oct 6, 2016
@fejta fejta assigned saad-ali and jingxu97 and unassigned fejta Oct 6, 2016
@fejta
Copy link
Contributor

fejta commented Oct 6, 2016

Nice improvement @rata! Will someone from @kubernetes/sig-storage give assign this to themselves and give it a quick review?

@rata
Copy link
Member Author

rata commented Oct 6, 2016

@fejta thanks! Do you think it's "ok to test" so we can actually see it works fine?

@saad-ali
Copy link
Member

saad-ali commented Oct 6, 2016

@k8s-bot ok to test

@saad-ali
Copy link
Member

saad-ali commented Oct 6, 2016

LGTM

@saad-ali saad-ali added lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. and removed release-note-label-needed labels Oct 6, 2016
@rata
Copy link
Member Author

rata commented Oct 6, 2016

@saad-ali thanks! :)

@saad-ali
Copy link
Member

saad-ali commented Oct 6, 2016

No problem! Thanks for the cleanup

@k8s-ci-robot
Copy link
Contributor

Jenkins GCE Node e2e failed for commit a7b405930cc488d4a0f0d77b27222940eaa9885a. Full PR test history.

The magic incantation to run this job again is @k8s-bot node e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-ci-robot
Copy link
Contributor

Jenkins GCI GCE e2e failed for commit a7b405930cc488d4a0f0d77b27222940eaa9885a. Full PR test history.

The magic incantation to run this job again is @k8s-bot gci gce e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-ci-robot
Copy link
Contributor

Jenkins GCE e2e failed for commit a7b405930cc488d4a0f0d77b27222940eaa9885a. Full PR test history.

The magic incantation to run this job again is @k8s-bot gce e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-ci-robot
Copy link
Contributor

Jenkins Kubemark GCE e2e failed for commit a7b405930cc488d4a0f0d77b27222940eaa9885a. Full PR test history.

The magic incantation to run this job again is @k8s-bot kubemark e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@rata rata force-pushed the simplify-e2e-secret branch from a7b4059 to 84985b0 Compare October 6, 2016 03:39
@k8s-ci-robot
Copy link
Contributor

Jenkins GCI GKE smoke e2e failed for commit 84985b0440bf84b13d086c345073d6c301db301b. Full PR test history.

The magic incantation to run this job again is @k8s-bot gci gke e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 6, 2016
This patch just removes duplicated code in secret e2e tests.
@rata rata force-pushed the simplify-e2e-secret branch from 84985b0 to 0ff2f08 Compare October 6, 2016 13:05
@k8s-ci-robot
Copy link
Contributor

Jenkins GKE smoke e2e failed for commit 0ff2f08. Full PR test history.

The magic incantation to run this job again is @k8s-bot gke e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@rata
Copy link
Member Author

rata commented Oct 6, 2016

@saad-ali I've just rebased and pushed today, because I thought the last 3 faild were flaky. And it seems they are, as now only one fails.

Should I just retry until this passes? Or what should I do in this case? (looking at the error, it seems unrelated)

@saad-ali saad-ali added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 6, 2016
@saad-ali
Copy link
Member

saad-ali commented Oct 6, 2016

Should I just retry until this passes? Or what should I do in this case? (looking at the error, it seems unrelated)

Yes, but you don't have to do a rebase/push to trigger retest.

@saad-ali
Copy link
Member

saad-ali commented Oct 6, 2016

@k8s-bot gke e2e test this, Issue: #33175

@rata
Copy link
Member Author

rata commented Oct 6, 2016

@saad-ali thanks! I do rebase/amend and push because I think I don't have permissions to send that command to the bot :-)

@saad-ali
Copy link
Member

saad-ali commented Oct 7, 2016

Right, sorry about that

@k8s-github-robot
Copy link

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

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit f352e67 into kubernetes:master Oct 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. sig/storage Categorizes an issue or PR as relevant to SIG Storage. 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.

7 participants