Skip to content

Conversation

@smoser
Copy link
Contributor

@smoser smoser commented Nov 13, 2023

This is basically the add of BUSYBOX image and then using that instead of centos image.

What type of PR is this?

Which issue does this PR fix:

What does this PR do / Why do we need it:

If an issue # is not available please add repro steps and logs showing the issue:

Testing done on this change:

Automation added to e2e:

Will this break upgrades or downgrades?

Does this PR introduce any user-facing change?:


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@smoser smoser force-pushed the test/busybox-images branch 4 times, most recently from e3bd7e2 to 20105b9 Compare November 13, 2023 19:19
@codecov
Copy link

codecov bot commented Nov 13, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6899344) 13.01% compared to head (4391f30) 13.01%.

❗ Current head 4391f30 differs from pull request most recent head 691ac17. Consider uploading reports for the commit 691ac17 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #545   +/-   ##
=======================================
  Coverage   13.01%   13.01%           
=======================================
  Files          40       40           
  Lines        6003     6003           
=======================================
  Hits          781      781           
  Misses       5094     5094           
  Partials      128      128           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@smoser
Copy link
Contributor Author

smoser commented Nov 13, 2023

Just for some reference here...
The last action to run against main here took 21m29s for unpriv 'Test' section and 24m39s for priv.

The action for this pr here took 4m14 seconds for unpriv and 8m21s for priv.

@smoser smoser force-pushed the test/busybox-images branch from 20105b9 to 4391f30 Compare November 13, 2023 19:59
This is basically the add of BUSYBOX image and then using
that instead of centos image.

Moving to busybox from centos required a few changed assumptions:

 * drop -P from grep - busybox's grep did not support -P. instead
   use posix groups ([:space:])
 * don't use long --format for 'stat' - busybox stat didn't like
   this, so just use '-c'.
 * mount - /usr/bin/mount -> /bin/mount - busybox image only has
   /bin, not /usr/bin/mount
 * /bin/bash - busybox image does not have bin/bash

The tests "squashfs empty change no layer" and
"tar empty change no layer" do not work with the busybox image.
There are layers created, and they have this content:

    $ tar tvf oci/blobs/sha256/acff571f8cf*b03f45
    tar: Removing leading `/' from member names
    drwxr-xr-x 0/0               0 2023-11-13 14:06 /
    drwxr-xr-x 0/0               0 2023-11-13 14:06 etc/
    ---------- 0/0               0 2023-11-13 14:06 etc/resolv.conf
    drwxr-xr-x 0/0               0 2023-11-13 14:06 proc/
    drwxr-xr-x 0/0               0 2023-11-13 14:06 sys/

The content there are results of stacker.  I don't have a good way
to avoid them at the moment.

I've also left the bom tests in place with the centos image.
Those would have had to have been adjusted and probably
would not work with busybox at the moment.

Signed-off-by: Scott Moser <smoser@brickies.net>
@smoser smoser force-pushed the test/busybox-images branch from 4391f30 to 691ac17 Compare November 14, 2023 13:33
@smoser
Copy link
Contributor Author

smoser commented Nov 14, 2023

@rchincha , do you have thoughts on this? From my perspective, it is an obvious huge win. It reduces the longest poll in the c-i tent (priv=priv workflow=ci) by ~ 40% (37m3s to 23m11s) and specifically the 'test' action of that run from 24m39s to 8m21s (~66%).

I'm not opposed to using centos (or some other image that is a more "real world" use case) for a set of tests, but it appears to be just wasted IO for the most part here.

@rchincha
Copy link
Contributor

@rchincha , do you have thoughts on this? From my perspective, it is an obvious huge win. It reduces the longest poll in the c-i tent (priv=priv workflow=ci) by ~ 40% (37m3s to 23m11s) and specifically the 'test' action of that run from 24m39s to 8m21s (~66%).

I'm not opposed to using centos (or some other image that is a more "real world" use case) for a set of tests, but it appears to be just wasted IO for the most part here.

Large multi-layer images cover some cases (such as sizes, compression, etc)
But we can move that into nightly tests.
When invoked from ci, maybe use the smaller images and nightly larger ones?

@smoser
Copy link
Contributor Author

smoser commented Nov 14, 2023

Large multi-layer images cover some cases (such as sizes, compression, etc)

docker://centos:latest is not a multi-layer image.

When invoked from ci, maybe use the smaller images and nightly larger ones?

I guess i'm not opposed to that. but I honestly have never heard of a bug in gzip that was size specific. Is there an example of an issue that you're thinking about?

I'd really rather not run significantly different things in c-i than in nightly. Its one thing to run some extra nightly jobs, another to entirely change what runs.

The change and results here show that ~60% of our c-i test time is spent doing unnecessary IO or CPU operations. Lets just drop that and spend time and IO doing useful things.

Copy link
Contributor

@rchincha rchincha left a comment

Choose a reason for hiding this comment

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

lgtm

@rchincha rchincha merged commit 8d233ed into project-stacker:main Nov 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants