-
Notifications
You must be signed in to change notification settings - Fork 18.7k
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
Windows CI: Make sure that CI fails on any error #40599
Conversation
3c9e30a
to
a9e53d2
Compare
a9e53d2
to
23262ff
Compare
23262ff
to
893100a
Compare
7ecbd7e
to
27edce9
Compare
6b7c71c
to
c799853
Compare
Looks that I got this one working @thaJeztah @StefanScherer PTAL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much!! Not near my computer, but had an initial look at the changes; left some questions inline
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SGTM
Thanks
c799853
to
f6a8d35
Compare
Hmm. After update RS5 CI failed to error which have nothing to do with this change (except that now we do not ignore those anymore):
Will try what it says on next round... |
- If unit tests fails - If intergration tests fails Signed-off-by: Olli Janatuinen <[email protected]>
f6a8d35
to
82b5ff8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤔 Looks like something is missing in the uploaded artifacts. Here's from another PR; https://ci-next.docker.com/public/job/moby/job/PR-41460/1/, and that contains a windowsRS5-integration-bundles.zip
, and looks like that's missing in this PR; https://ci-next.docker.com/public/blue/organizations/jenkins/moby/detail/PR-40599/22/artifacts
That is weird. Looks like that Jenkins is not running correct version of definition. End of test phase those are created to correct place:
But bundle creation is looking from them from incorrect folder:
and those was collected correctly earlier before I changed those |
Oh yes, I think the Jenkinsfile is protected and never run the modified version directly from a PR build. |
Yep. @thaJeztah is this good now? CI looks to be 💚 |
🤦 ah, of course |
I was considering opening a draft PR to test the Jenkinsfile changes, but looking again, the changes in the Jenkinsfile are all inside |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Let's merge 👍 |
Yea. Looks that it exactly what happened... Some reason it does not find bundles/CIDUT.out file https://ci-next.docker.com/public/blue/organizations/jenkins/moby/detail/master/647/pipeline/231 Do you have some way to login to server and actually look where those files are? |
@olljanat It‘s a bit tricky, the Windows agents are ephemeral and get destroyed after idle time. |
@StefanScherer well those Windows integration tests have been disabled, broken, etc... very long time so I guess that missing artifacts is not big thing. However as it is now merged to master you probably can check those from any pull request or master build server after those are finished. |
@StefanScherer Weird. Looks that https://ci-next.docker.com/public/blue/organizations/jenkins/moby/detail/PR-38469/23/artifacts/ created artifacts correctly. So either Jenkins/Jenkins nodes are someway out of sync or for some reason bundles\CIDUT.out does not get generated all the times... |
@olljanat Thanks for the two example jobs. The failing job that got the artifacts failed in line 885. If this step doesn't fail, then the script changes directory in line 891. https://github.com/moby/moby/blob/master/hack/ci/windows.ps1#L885-L891 Maybe something like this might be the issue copying files to a relative target |
@olljanat Confirmed, sometimes the script is in the wrong directory. And the |
Updated PR #41463 to go to the correct directory to collect the logs. |
- What I did
On Windows CI tests are called using path:
Jenkins -> PowerShell -> gotestsum -> go test
gotestsum automatically return any exit code coming from go test and these changes make sure that if gotestsum exit code is not zero we exit using 1 to Jenkins too.
Also changed gotestsum to use "standard-verbose" format which is same than
-test.v
go test.Also make sure that all outputs ends up to artifact.
- How I did it
Fixed PowerShell script and Jenkinsfile.
- How to verify it
CI passed on here and you can see that on #38469 where I also included this one broken integration test made CI failing https://ci-next.docker.com/public/blue/organizations/jenkins/moby/detail/PR-38469/19/pipeline/252 and it fails fast without need wait all tests to run.
- A picture of a cute animal (not mandatory but encouraged)
Fixes #39576
Fixes #40069