sclorg / container-common-scripts

Apache License 2.0
20 stars 45 forks source link

make squash work for podman, not for docker in the testsuite #277

Closed zmiklank closed 1 year ago

zmiklank commented 2 years ago

NOTE: podman is able to run "podman build --squash $context", but docker is not. In order to squash image during the build with docker, its daemon needs to have experimental features enabled:

echo '{"experimental": true}' >> /etc/docker/daemon.json

Followup: After some discussion within the team we decided, that it is not our goal to support squashing with docker, thus we will continue to test squash only with podman. This results in removing docker-squash functionality and introducing build --squash option, which is to be enabled if podman is used, and if SKIP_SQUASH is set to 0

zmiklank commented 2 years ago

I have reworked the PR based on @pkubatrh's review. Please re-review, if tests pass.

zmiklank commented 2 years ago

Unfortunately, they seem to fail. I will debug that.

zmiklank commented 2 years ago

[test-all]

zmiklank commented 2 years ago

Testing is not possible if fast-forward merge is not possible. As something was merged to master before last run of tests, they are all invalid. I have rebased on master and now am rerunning the tests. [test-all]

phracek commented 2 years ago

@zmiklank I would prefer before pushing execute make shellcheck. Sanity test failures:

./run-shellcheck.sh `git ls-files *.sh`

In build.sh line 186:
  pushd "${dir}" > /dev/null
  ^------------------------^ SC2164: Use 'pushd ... || exit' or 'pushd ... || return' in case pushd fails.

Did you mean: 
make: *** [Makefile:28: shellcheck] Error 123
  pushd "${dir}" > /dev/null || exit

In build.sh line 205:
  popd > /dev/null
  ^--------------^ SC2164: Use 'popd ... || exit' or 'popd ... || return' in case popd fails.

Did you mean: 
  popd > /dev/null || exit

In tag.sh line 14:
  pushd "${dir}" > /dev/null
  ^------------------------^ SC2164: Use 'pushd ... || exit' or 'pushd ... || return' in case pushd fails.

Did you mean: 
  pushd "${dir}" > /dev/null || exit

In tag.sh line 29:
  popd > /dev/null
  ^--------------^ SC2164: Use 'popd ... || exit' or 'popd ... || return' in case popd fails.

Did you mean: 
  popd > /dev/null || exit
zmiklank commented 2 years ago

@phracek, the error logs you have posted are not related to this PR. I did not add/adjust any of the mentioned lines.

This should be addressed elsewhere.

zmiklank commented 2 years ago

It seems, that tests error were caused by the impossibility of ff merge. Removing debugging options and retesting: [test-all]

zmiklank commented 2 years ago

This is stuck on the impossibility to build s2i-core-container because of an invalid reference to some s2i stuff in the image. This needs to be handled separately.

zmiklank commented 2 years ago

[test-all]

zmiklank commented 2 years ago

I like the solution. I've just proposed some possible naming improvements.

Thanks for reviewing, I have fixed the found issues in new commits. However, before merging I need to look at the tests again.

zmiklank commented 1 year ago

[test-all]

zmiklank commented 1 year ago

[test-all]

phracek commented 1 year ago

[test-all]