ci: gha test workflow for integration and unit test by crazy-max · Pull Request #44034 · moby/moby

crazy-max · 2022-08-25T13:23:43Z

follow-up #43479

- What I did

Adds gha test workflow to run integration and unit test. Will ease testing #43529 and also moving out some bits from Jenkins.

I use the same matrix strategy to distribute integration-cli tests as windows workflow. ~~Looking at the overall number of jobs running in the pipeline now I think we could remove tests against windows-2019 so queue size would be reduce.~~

Some tests look broken on GitHub Runner, probably linked to the runner environment so skipping them on gha env for now.

integration (rootless): https://github.com/crazy-max/moby/runs/8010866754?check_suite_focus=true#step:7:1032

=== Failed
=== FAIL: amd64.integration.container TestNetworkLoopbackNat (1.48s)
    nat_test.go:91: assertion failed: it works (msg string) !=  (string)

related #41561

ìntegration-cli: https://github.com/crazy-max/moby/runs/8010867286?check_suite_focus=true#step:6:662

=== Failed
=== FAIL: amd64.integration-cli TestDockerCLIEventSuite/TestEventsOOMDisableTrue (0.03s)
    docker_cli_events_unix_test.go:99: assertion failed: error is not nil: error executing docker inspect: Error response from daemon: invalid reference format: repository name must be lowercase

related #36541

ìntegration-cli: https://github.com/crazy-max/moby/runs/8010867670?check_suite_focus=true#step:6:1328

=== Failed
=== FAIL: amd64.integration-cli TestDockerPluginSuite/TestPluginInstallImage (15.21s)
    docker_cli_plugins_test.go:232: assertion failed: expression is false: strings.Contains(out, `Encountered remote "application/vnd.docker.container.image.v1+json"(image) when fetching`)
    --- FAIL: TestDockerPluginSuite/TestPluginInstallImage (15.21s)

related #41559 #43996

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

crazy-max · 2022-08-25T14:08:17Z

https://github.com/moby/moby/runs/8017622176?check_suite_focus=true#step:6:1856

=== Failed
=== FAIL: libnetwork/networkdb TestNetworkDBCRUDMediumCluster (22.12s)
    networkdb_test.go:426: timeout hit after 20s: node2:Waiting for cluster peers to be established

related #42484

Makefile

thaJeztah

LGTM, thanks!

integration/container/nat_test.go

.github/workflows/test.yml

AkihiroSuda · 2022-08-25T18:06:25Z

integration-cli/requirements_test.go

 }

+func NotGitHubActions() bool {
+	return os.Getenv("GITHUB_ACTIONS") != "true"


Better to use strconv.ParseBool

Yes, I think we can either go with "allow the variable to both enable or explicitly disable", in which case we would have something like gzDecompress()

Or, we could keep it simple and consider any non-empty value to be "enabled", and empty to be "disabled";

moby/distribution/push_v2.go

Line 198 in 9184f0b

if os.Getenv("DOCKER_ALLOW_SCHEMA1_PUSH_DONOTUSE") == "" {

Given that this is only for CI, and not a user-facing option, I'd be ok with "non-empty" means "enabled";

Suggested change

return os.Getenv("GITHUB_ACTIONS") != "true"

return os.Getenv("GITHUB_ACTIONS") == ""

Or is it expected for this var to have GITHUB_ACTIONS=false ? I see various project just checking for "empty", but there's some that have "false" conditions as well; https://grep.app/search?q=GITHUB_ACTIONS&case=true&filter[lang][0]=Shell

Non empty sgtm

AkihiroSuda

Cgroup2 mode seems misconfigured

.github/workflows/test.yml

thaJeztah · 2022-08-25T19:42:25Z

Cgroup2 mode seems misconfigured

Good catch, thanks Akihiro!

thaJeztah · 2022-08-25T20:18:37Z

LOL; looks like we need to hash the paths;

mkdir: cannot create directory ‘/tmp/reports/dockercliimagessuitedockercliimportsuitedockercliinfosuitedockercliinspectsuitedockerclilinkssuitedockercliloginsuitedockerclilogssuitedockerclinetmodesuitedockerclinetworksuitedockerclipluginlogdriversuitedockerclipluginssuitedockercliportsuitedockercliproxysuitedockercliprunesuitedockerclipssuitedockerclipullsuitedockerclipushsuite’: 
File name too long

.github/workflows/test.yml

Signed-off-by: CrazyMax <[email protected]>

thaJeztah · 2022-09-07T22:58:32Z

@AkihiroSuda @tianon PTAL 🤗

AkihiroSuda · 2022-09-07T23:36:04Z

integration-cli/requirements_test.go

 }

+func GitHubActions() bool {
+	return os.Getenv("GITHUB_ACTIONS") == "true"


strconv.ParseBool

Ah, dang; I recall I commented on this to use "non-empty" (as I don't think we'll ever gonna use GITHUB_ACTIONS=false); #44034 (comment)

Looks like it was inadvertently marked as "resolved"; does that suggestion work for you, @AkihiroSuda ?

non-empty also sgtm

Ah I missed this one. Done with non-empty.

Signed-off-by: CrazyMax <[email protected]>

crazy-max · 2022-09-08T11:01:48Z

This one again 😞: https://github.com/moby/moby/runs/8246861287?check_suite_focus=true#step:6:225

#18 https://download.opensuse.org/repositories/devel:/tools:/criu/Debian_11/Release.key
#18 ERROR: Get "https://download.opensuse.org/repositories/devel:/tools:/criu/Debian_11/Release.key": net/http: TLS handshake timeout
------
 > https://download.opensuse.org/repositories/devel:/tools:/criu/Debian_11/Release.key:
------
ERROR: failed to solve: failed to load cache key: Get "https://download.opensuse.org/repositories/devel:/tools:/criu/Debian_11/Release.key": net/http: TLS handshake timeout
Error: buildx bake failed with: ERROR: failed to solve: failed to load cache key: Get "https://download.opensuse.org/repositories/devel:/tools:/criu/Debian_11/Release.key": net/http: TLS handshake timeout

@thaJeztah Could we merge #44086 first? This is really frustrating and seems to happen quite often.

crazy-max · 2022-09-08T11:47:32Z

Thanks for the re-run @thaJeztah, good to merge?

thaJeztah · 2022-09-08T13:56:00Z

Thanks! I think that should address @AkihiroSuda's comment; let me merge this to unblock the other work, but @AkihiroSuda if there's anything to fix up after this, let me know ❤️

See #44034 (comment)

thaJeztah

LGTM

crazy-max requested a review from tianon as a code owner August 25, 2022 13:23

thaJeztah added status/2-code-review area/testing process/cherry-pick kind/refactor PR's that refactor, or clean-up code labels Aug 25, 2022

crazy-max force-pushed the gha-test branch from c0838c1 to 3103475 Compare August 25, 2022 13:55

crazy-max force-pushed the gha-test branch from 3103475 to 3345031 Compare August 25, 2022 14:20

thaJeztah reviewed Aug 25, 2022

View reviewed changes

Makefile Show resolved Hide resolved

thaJeztah reviewed Aug 25, 2022

View reviewed changes

Makefile Show resolved Hide resolved

thaJeztah approved these changes Aug 25, 2022

View reviewed changes

thaJeztah reviewed Aug 25, 2022

View reviewed changes

integration/container/nat_test.go Outdated Show resolved Hide resolved

AkihiroSuda reviewed Aug 25, 2022

View reviewed changes

.github/workflows/test.yml Outdated Show resolved Hide resolved

AkihiroSuda reviewed Aug 25, 2022

View reviewed changes

AkihiroSuda previously requested changes Aug 25, 2022

View reviewed changes

crazy-max force-pushed the gha-test branch from 3345031 to 9c16f8c Compare August 25, 2022 18:24

crazy-max mentioned this pull request Aug 25, 2022

ci(windows): move windows-2019 to another workflow #44035

Merged

crazy-max requested review from AkihiroSuda and removed request for tianon August 25, 2022 18:40

AkihiroSuda reviewed Aug 25, 2022

View reviewed changes

.github/workflows/test.yml Outdated Show resolved Hide resolved

AkihiroSuda reviewed Aug 25, 2022

View reviewed changes

.github/workflows/test.yml Outdated Show resolved Hide resolved

crazy-max force-pushed the gha-test branch from 9c16f8c to 63e8b02 Compare August 25, 2022 19:55

crazy-max force-pushed the gha-test branch 6 times, most recently from 7c9071e to 1aa0970 Compare August 25, 2022 22:02

thaJeztah reviewed Sep 7, 2022

View reviewed changes

.github/workflows/test.yml Outdated Show resolved Hide resolved

crazy-max added 2 commits September 7, 2022 22:52

ci: gha test workflow for integration and unit test

2e04be3

Signed-off-by: CrazyMax <[email protected]>

integration: TestNetworkLoopbackNat is broken on GitHub Runner

df731c7

Signed-off-by: CrazyMax <[email protected]>

crazy-max force-pushed the gha-test branch from 80ea8b6 to 47305e8 Compare September 7, 2022 20:52

This comment was marked as outdated.

Sign in to view

crazy-max mentioned this pull request Sep 7, 2022

[22.06 backport] ci: gha test workflow for integration and unit test #44112

Merged

AkihiroSuda reviewed Sep 7, 2022

View reviewed changes

crazy-max added 5 commits September 8, 2022 12:12

integration-cli: TestEventsOOM* broken on GitHub Runner

b8bf60c

Signed-off-by: CrazyMax <[email protected]>

integration-cli: TestPluginInstallImage broken on GitHub Runner

6086422

Signed-off-by: CrazyMax <[email protected]>

ci(test): upload reports

4c8af0e

Signed-off-by: CrazyMax <[email protected]>

ci(test): send coverage to codecov

807c849

Signed-off-by: CrazyMax <[email protected]>

ci: fix .windows workflow name

3668849

Signed-off-by: CrazyMax <[email protected]>

crazy-max force-pushed the gha-test branch from 47305e8 to 3668849 Compare September 8, 2022 10:12

crazy-max mentioned this pull request Sep 8, 2022

Dockerfile: variant support in frozen-images stage #44114

Merged

crazy-max requested review from AkihiroSuda and thaJeztah and removed request for AkihiroSuda and thaJeztah September 8, 2022 13:55

thaJeztah approved these changes Sep 8, 2022

View reviewed changes

thaJeztah merged commit dba0819 into moby:master Sep 8, 2022

crazy-max deleted the gha-test branch September 8, 2022 13:56

thaJeztah added process/cherry-picked and removed process/cherry-pick labels Sep 8, 2022

neersighted added this to the v-next milestone Jan 10, 2023

	return os.Getenv("GITHUB_ACTIONS") != "true"
	return os.Getenv("GITHUB_ACTIONS") == ""

Conversation

crazy-max commented Aug 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

crazy-max commented Aug 25, 2022

Uh oh!

Uh oh!

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

AkihiroSuda Aug 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thaJeztah Aug 30, 2022

Choose a reason for hiding this comment

Uh oh!

crazy-max Aug 30, 2022

Choose a reason for hiding this comment

Uh oh!

AkihiroSuda left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

thaJeztah commented Aug 25, 2022

Uh oh!

thaJeztah commented Aug 25, 2022

Uh oh!

Uh oh!

This comment was marked as outdated.

thaJeztah commented Sep 7, 2022

Uh oh!

AkihiroSuda Sep 7, 2022

Choose a reason for hiding this comment

Uh oh!

thaJeztah Sep 8, 2022

Choose a reason for hiding this comment

Uh oh!

AkihiroSuda Sep 8, 2022

Choose a reason for hiding this comment

Uh oh!

crazy-max Sep 8, 2022

Choose a reason for hiding this comment

Uh oh!

crazy-max commented Sep 8, 2022

Uh oh!

crazy-max commented Sep 8, 2022

Uh oh!

thaJeztah commented Sep 8, 2022

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

crazy-max commented Aug 25, 2022 •

edited

Loading

AkihiroSuda Aug 25, 2022 •

edited

Loading