Skip to content

Switch test image to a non rate-limited manifest list#5216

Merged
crosbymichael merged 1 commit intocontainerd:masterfrom
estesp:testimage-429
Mar 19, 2021
Merged

Switch test image to a non rate-limited manifest list#5216
crosbymichael merged 1 commit intocontainerd:masterfrom
estesp:testimage-429

Conversation

@estesp
Copy link
Copy Markdown
Member

@estesp estesp commented Mar 17, 2021

Builds on changes from #5189 to default to a non rate-limited testImage used across many of the client tests since we are still having a lot of rate-limiting on the macos nodes running the SELinux/Cgroupsv2 tests.

Signed-off-by: Phil Estes [email protected]

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Mar 17, 2021

Build succeeded.

dims
dims approved these changes Mar 17, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@dims dims left a comment

Choose a reason for hiding this comment

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

LGTM

@estesp
Copy link
Copy Markdown
Member Author

estesp commented Mar 17, 2021

Well, it was a fun exercise; to make this work we need an image with at least top, sleep and need to change the "pre-pulled" image for tests that just expect the default test image to be in the content store.

@estesp estesp marked this pull request as draft March 17, 2021 15:03
@estesp estesp changed the title Switch test image to a non rate-limited manifest list WIP: Switch test image to a non rate-limited manifest list Mar 17, 2021
@mikebrow
Copy link
Copy Markdown
Member

mikebrow commented Mar 17, 2021

well pause vs alpine would probably improve our benchmark scoring :-)

@dims
Copy link
Copy Markdown
Member

dims commented Mar 17, 2021

LOL on top sleep :)

@crosbymichael
Copy link
Copy Markdown
Member

crosbymichael commented Mar 18, 2021

I guess an alpine image from gcr will work

@estesp estesp force-pushed the testimage-429 branch 2 times, most recently from 3e0e979 to 20c95d6 Compare March 18, 2021 17:48
@estesp estesp changed the title WIP: Switch test image to a non rate-limited manifest list Switch test image to a non rate-limited manifest list Mar 18, 2021
@estesp estesp marked this pull request as ready for review March 18, 2021 18:55
AkihiroSuda
AkihiroSuda reviewed Mar 19, 2021
View reviewed changes
Comment thread integration/client/client_unix_test.go

var (
testImage string
testImage = "mirror.gcr.io/library/busybox:latest"
Copy link
Copy Markdown
Member

@AkihiroSuda AkihiroSuda Mar 19, 2021

Choose a reason for hiding this comment

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

nit: s/var/const/

Copy link
Copy Markdown
Member

@AkihiroSuda AkihiroSuda Mar 19, 2021

Choose a reason for hiding this comment

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

Can we also have const testImageWWWDataUID = 33 in this file?

Copy link
Copy Markdown
Member

@crosbymichael crosbymichael Mar 19, 2021

Choose a reason for hiding this comment

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

Lets go ahead and merge this so the CI doesn't keep failing on us. We can address this in a followup

@crosbymichael crosbymichael merged commit f7bd43c into containerd:master Mar 19, 2021
@estesp
Copy link
Copy Markdown
Member Author

estesp commented Mar 19, 2021

uh oh.. I was still looking into random CI failures (I can't figure out why not consistent) after this switch to busybox; 1 in 10 runs (or so) of the integration suite were failing mount of /proc with the user namespaces test + readonly root fs. Was tempted just to give up and go back to alpine, but was tempted to spend time figuring out the random /proc mount failure

@crosbymichael
Copy link
Copy Markdown
Member

crosbymichael commented Mar 19, 2021

oops. you wanna revert of fix forward?

@estesp estesp mentioned this pull request Mar 20, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AkihiroSuda AkihiroSuda AkihiroSuda left review comments

@dims dims dims approved these changes

@crosbymichael crosbymichael crosbymichael approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@estesp @mikebrow @dims @crosbymichael @AkihiroSuda