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

Retry job update after failure to prevent modification conflict #37077

Merged
merged 1 commit into from
Nov 19, 2016
Merged

Retry job update after failure to prevent modification conflict #37077

merged 1 commit into from
Nov 19, 2016
+32 −4

Conversation

soltysh
Copy link
Contributor

@soltysh soltysh commented Nov 18, 2016 •
edited by k8s-oncall
Loading

This fixes #34585 flake.

@janetkuo || @kubernetes/sig-apps ptal

I've been getting too many emails recently wrt to that issue, so I wanted to "clean" my inbox a bit 😉

This change is Reviewable

@soltysh
Copy link
Contributor Author

soltysh commented Nov 18, 2016

With this being a flake fix I'm tagging this as a 1.5 milestone, fyi @saad-ali @dims

@k8s-github-robot k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. release-note-label-needed labels Nov 18, 2016
@soltysh soltysh added this to the v1.5 milestone Nov 18, 2016
@soltysh soltysh added release-note-none Denotes a PR that doesn't merit a release note. and removed release-note-label-needed labels Nov 18, 2016
0xmichalis
0xmichalis reviewed Nov 18, 2016
View reviewed changes
test/e2e/framework/util.go
func UpdateJobWithRetries(c clientset.Interface, namespace, name string, applyUpdate updateJobFunc) (job *batch.Job, err error) {
jobs := c.Batch().Jobs(namespace)
var updateErr error
pollErr := wait.Poll(10*time.Millisecond, 1*time.Minute, func() (bool, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Use PollImmediate

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmm... shouldn't we use it for other *WithRetries here as well?

@0xmichalis
Copy link
Contributor

One comment, LGTM in general

@dims
Copy link
Member

dims commented Nov 19, 2016

@k8s-bot verify test this

@k8s-ci-robot
Copy link
Contributor

Jenkins verification failed for commit e3989c3. Full PR test history.

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

@soltysh soltysh assigned 0xmichalis and unassigned ixdy Nov 19, 2016
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 19, 2016
@soltysh
Copy link
Contributor Author

soltysh commented Nov 19, 2016

Nit addressed, applying label based on previous approval.

@soltysh soltysh added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 19, 2016
@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 19, 2016
@soltysh soltysh added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 19, 2016
@soltysh
Copy link
Contributor Author

soltysh commented Nov 19, 2016

Re-applying label back, I've updated bazel.

@k8s-ci-robot
Copy link
Contributor

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

The magic incantation to run this job again is @k8s-bot cri 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 kops AWS e2e failed for commit b253c20. Full PR test history.

The magic incantation to run this job again is @k8s-bot kops aws 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
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 2b86ed3 into kubernetes:master Nov 19, 2016
@soltysh soltysh deleted the issue34585 branch November 21, 2016 07:33
@soltysh soltysh mentioned this pull request Dec 3, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0xmichalis 0xmichalis 0xmichalis left review comments

Assignees

@0xmichalis 0xmichalis

Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. 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. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
v1.5
Development

Successfully merging this pull request may close these issues.

[k8s.io] Job should fail a job {Kubernetes e2e suite}
7 participants
@soltysh @0xmichalis @dims @k8s-ci-robot @k8s-github-robot @googlebot @ixdy