Skip to content

template.jinja: also clean up /var/lib/{rpm,dnf} for reproducibility#888

Merged
jmtd merged 7 commits into
cekit:developfrom
jmtd:adjust-cleanup
Feb 13, 2024
Merged

template.jinja: also clean up /var/lib/{rpm,dnf} for reproducibility#888
jmtd merged 7 commits into
cekit:developfrom
jmtd:adjust-cleanup

Conversation

@jmtd
Copy link
Copy Markdown
Collaborator

@jmtd jmtd commented Feb 7, 2024

hopefully addresses #886 .

@jmtd
Copy link
Copy Markdown
Collaborator Author

jmtd commented Feb 7, 2024

I have manually verified that a downstream image can do e.g. microdnf install... without problems. I should probably add some tests to this PR.

@jmtd jmtd force-pushed the adjust-cleanup branch 2 times, most recently from 9a787bd to 9b57d1b Compare February 8, 2024 12:42
Comment thread cekit/templates/template.jinja Outdated
Comment thread cekit/templates/template.jinja Outdated
Comment thread tests/test_dockerfile.py
jmtd added 3 commits February 8, 2024 15:07
Signed-off-by: Jonathan Dowland <jdowland@redhat.com>
Extend macro repo_clear_cache to also remove /var/lib/rpm
and /var/lib/dnf.

Don't bother checking for the existence of the directories:
rm -rf will return success even if they do not exist.

Additionally, merge two adjacent RUN rm -rf commands that
handle scripts and artifacts.

Fixes cekit#886.

Signed-off-by: Jonathan Dowland <jdowland@redhat.com>
These tests fail after adjustments were made to the template
to stop calling "pkg_manager clean all" and instead simply
remove the files that the package managers leave behind.

The test test_cleanup_rpm_dnf_default_pkg_manager handles checking
for the clean-up lines.

Signed-off-by: Jonathan Dowland <jdowland@redhat.com>
jmtd added 2 commits February 8, 2024 15:09
This path would be cleaned up by "apt-get clean" but we have stopped
calling it in the template.

Signed-off-by: Jonathan Dowland <jdowland@redhat.com>
dnf (full version) writes to /var/cache/dnf.

Signed-off-by: Jonathan Dowland <jdowland@redhat.com>
@jmtd
Copy link
Copy Markdown
Collaborator Author

jmtd commented Feb 8, 2024

Further test failures to adjust

tests/test_integ_builder_osbs.py::test_osbs_builder_with_rhpam_1: the test has an entire rendered Dockerfile embedded in it and checks for it byte-for-byte identical! This is going to break for any change we make to the template. I think that test (and any more like it) need refactoring to limit their comparisons to the topic of the test.

@rnc
Copy link
Copy Markdown
Contributor

rnc commented Feb 8, 2024

Further test failures to adjust

tests/test_integ_builder_osbs.py::test_osbs_builder_with_rhpam_1: the test has an entire rendered Dockerfile embedded in it and checks for it byte-for-byte identical! This is going to break for any change we make to the template. I think that test (and any more like it) need refactoring to limit their comparisons to the topic of the test.

I think at the time I did that to make sure the multistage was accurately building what the users wanted. While fiddly it was meant to catch mistakes in the generation.

jmtd added 2 commits February 9, 2024 16:05
The test was very sensitive to the template's generated post-install
cleanup code. Move to a more scoped regular-expression-based check.

regex_dockerfile copied from test_dockerfile; in future it might be
nice to move it to some common test library/utility class.

Signed-off-by: Jonathan Dowland <jdowland@redhat.com>
These tests started to fail with (otherwise unrelated) adjustments to
the template.

Longer term I think we should rework these tests to be less sensitive
(see cekit#890).

Signed-off-by: Jonathan Dowland <jdowland@redhat.com>
@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (4b6aaad) 90.16% compared to head (acee170) 90.16%.
Report is 1 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #888   +/-   ##
========================================
  Coverage    90.16%   90.16%           
========================================
  Files           42       42           
  Lines         3263     3263           
========================================
  Hits          2942     2942           
  Misses         321      321           

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

@jmtd
Copy link
Copy Markdown
Collaborator Author

jmtd commented Feb 13, 2024

Cheers! 🍻

@jmtd jmtd merged commit e393ff2 into cekit:develop Feb 13, 2024
@jmtd jmtd deleted the adjust-cleanup branch February 13, 2024 16:04
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