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

Detect flakes in PR builder e2e runs #27898

Merged
merged 2 commits into from
Jul 18, 2016
Merged

Conversation

lavalamp
Copy link
Member

Won't be mergable until onsi/ginkgo#261 is agreed upon and merged.

Tossing a PR here to get the e2e test to run on it.

@k8s-github-robot k8s-github-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. release-note-label-needed labels Jun 22, 2016
@zmerlynn zmerlynn assigned lavalamp and unassigned zmerlynn Jun 22, 2016
@zmerlynn
Copy link
Member

(Feel free to kick back after WIP)

@lavalamp
Copy link
Member Author

@matchstick since you love godep related things: I made changes to vendored stuff here that certainly aren't reproducible, yet the verification test succeeded anyway...

@lavalamp
Copy link
Member Author

Need kubernetes/test-infra#204 to actually test this...

@k8s-github-robot k8s-github-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jun 23, 2016

# For pull jobs, keep a canonical ordering for tools that want to examine
# the output.
echo "aoeu debug: ${gcs_indirect_path}"
Copy link
Member Author

Choose a reason for hiding this comment

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

@ixdy this change doesn't seem to be affecting what the PR builder does. Does it run from head and not the PR?

Copy link
Member

Choose a reason for hiding this comment

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

yeah, it always downloads upload-to-gcs.sh from HEAD: https://github.com/kubernetes/test-infra/blob/b12293e6c4a49a968705775b06656d452f6db9e0/jenkins/job-configs/kubernetes-jenkins-pull/global.yaml#L7-L13

I think we discussed changing this to download only if necessary. cc @spxtr

Copy link
Member Author

Choose a reason for hiding this comment

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

How can I test my changes then?

On Fri, Jun 24, 2016 at 9:26 AM, Jeff Grafton notifications@github.com
wrote:

In hack/jenkins/upload-to-gcs.sh
#27898 (comment)
:

@@ -115,11 +121,26 @@ function upload_artifacts_and_build_result() {
echo "Uploading build log"
gsutil -q cp -Z -a "${gcs_acl}" "${WORKSPACE}/build-log.txt" "${gcs_build_path}"
fi
+

  • For pull jobs, keep a canonical ordering for tools that want to examine

  • the output.

  • echo "aoeu debug: ${gcs_indirect_path}"

yeah, it always downloads upload-to-gcs.sh from HEAD:
https://github.com/kubernetes/test-infra/blob/b12293e6c4a49a968705775b06656d452f6db9e0/jenkins/job-configs/kubernetes-jenkins-pull/global.yaml#L7-L13

I think we discussed changing this to download only if necessary. cc
@spxtr https://github.com/spxtr


You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
https://github.com/kubernetes/kubernetes/pull/27898/files/3f235d57378a8405d3fc05867aebd172af9b4980#r68423708,
or mute the thread
https://github.com/notifications/unsubscribe/AAnglv3AGA4EXUeOlUohlb1Q8g8CudM1ks5qPAU3gaJpZM4I8Noc
.

Copy link
Contributor

Choose a reason for hiding this comment

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

kubernetes/test-infra#217

Once that goes in it should test your changes.

Copy link
Member Author

Choose a reason for hiding this comment

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

@ixdy ping-- I thought you were making a change to make this possible on friday, but I can't find it now?

Copy link
Member

Choose a reason for hiding this comment

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

by the time I got an LG on that PR it was late Friday, and I didn't want to accidentally break Jenkins over the weekend. I'll merge it now.

@ixdy
Copy link
Member

ixdy commented Jun 27, 2016

@k8s-bot test this please, issue #IGNORE

@lavalamp
Copy link
Member Author

@ixdy hm, still didn't seem to work...

@ixdy
Copy link
Member

ixdy commented Jun 27, 2016

It didn't? Note that because of the way we save the build log, we don't get any output from upload-to-gcs.sh itself.

Looking on Jenkins I see

aoeu debug: gs://kubernetes-jenkins/pr-logs/directory/kubernetes-pull-build-test-e2e-gce
Writing gs://kubernetes-jenkins/pr-logs/pull/27898/kubernetes-pull-build-test-e2e-gce/46924 to gs://kubernetes-jenkins/pr-logs/directory/kubernetes-pull-build-test-e2e-gce/46924.txt
Marking build 46924 as the latest completed build for this PR
Marking build 46924 as the latest completed build

@lavalamp
Copy link
Member Author

@ixdy oh, awesome! thanks!

@k8s-github-robot
Copy link

@lavalamp
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.

@lavalamp
Copy link
Member Author

@k8s-bot test this issue: #IGNORE

Testing kubernetes/test-infra#246

@lavalamp
Copy link
Member Author

Awesome, it worked.

@lavalamp lavalamp changed the title WIP: detect flakes in PR builder e2e runs Detect flakes in PR builder e2e runs Jul 16, 2016
@lavalamp lavalamp assigned zmerlynn and unassigned lavalamp Jul 16, 2016
@lavalamp
Copy link
Member Author

Ginkgo change landed. This is the last piece needed to get automated systems to detect PR e2e flakes.

@zmerlynn zmerlynn added release-note Denotes a PR that will be considered when it comes time to generate release notes. lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed release-note-label-needed labels Jul 18, 2016
@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 18, 2016
@lavalamp lavalamp added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 18, 2016
@k8s-bot
Copy link

k8s-bot commented Jul 18, 2016

GCE e2e build/test passed for commit e91c5d0.

@lavalamp
Copy link
Member Author

adding retest-not-required since this just got tested three times in a row

@k8s-github-robot
Copy link

Automatic merge from submit-queue

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 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
Development

Successfully merging this pull request may close these issues.

7 participants