| CARVIEW |
Navigation Menu
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Run CRI integration tests in GitHub Actions (Windows) #6626
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
Run CRI integration tests in GitHub Actions (Windows) #6626
Conversation
There's no specific need mentioned at the points it was added, and it makes the Windows-hosted test run setup slightly weird. Signed-off-by: Paul "TBBle" Hampson <Paul.Hampson@Pobox.com>
Signed-off-by: Paul "TBBle" Hampson <Paul.Hampson@Pobox.com>
|
Hi @TBBle. Thanks for your PR. I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
Build succeeded.
|
|
The two Linux-side test failures in the test run seem likely to be flakes. Both failed in the same two tests, These failures didn't occur on the run for the same code in my fork. The Windows Server 2022 failure is probably also a flake, as it was a bunch of failures in the cri-integration tests that I hadn't seen in either the above run on my fork, or the earlier run before I added critest. That said, I was curious to see if I was going to see a failure I did see in my fork's last couple of runs, but it happens in cri-tools and so is later that the failure I hit here. I will push a minor change (I forgot to bump the job timeout when I added critest) and see if run comes out differently. Edit: And it passed on the second attempt Huzzah.. |
Signed-off-by: Paul "TBBle" Hampson <Paul.Hampson@Pobox.com>
Apart from crictl and go-junit-report, this script is just making the remote test VMs look like GitHub Actions VMs, i.e. git, make-mingw32, golang. And we don't use go-junit-report, so we can save a lot of time (about five minutes) by just extracting the interesting part. Signed-off-by: Paul "TBBle" Hampson <Paul.Hampson@Pobox.com>
TBBle
force-pushed
the
run-cri_integration-tests-in-GitHub_Actions
branch
from
e7703ae to
48b4783
Compare
|
Build succeeded.
|
| @@ -31,7 +31,7 @@ import ( | |||
| ) | |||
|
|
|||
| func TestAdditionalGids(t *testing.T) { | |||
| testPodLogDir, err := os.MkdirTemp("/tmp", "additional-gids") | |||
| testPodLogDir, err := os.MkdirTemp("", "additional-gids") | |||
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.
t.TempDir is more preferable, but can be another PR
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.
Sure. This at-least lines them up with the other os.MkdirTemp calls that didn't have /tmp hardcoded.
AkihiroSuda
changed the title
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
As suggested at #4878 (comment), and enhancing #5039.
I figured this was less work and less-costly than actually getting AKS and GCP accounts set up to let me run the CRI tests via the periodic job, and I don't have a regular Windows Server machine at-hand to run the tests locally.