Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve build of the ISO creating containers #5052

Conversation

jkonecny12
Copy link
Member

@jkonecny12 jkonecny12 commented Aug 17, 2023

Change the ownership of the artifacts build from the ISO builds

The ISO builds are required to be done under root. That means that the resulting artifacts are also root owned. That makes troubles with git clean or similar calls. To solve that copy the owner and group from the input Anaconda RPMs to output artifacts. That should be correct most of the time.

Also remove requirements to run make commands as root

The sudo make ... is just wrong and should not be used anymore. Instead sudo calls are abstracted in the Makefile.

Improve usage of the container build cache

Cache could save a lot of time during development but could cause outdated images because container don't know if the DNF call is outdated. To resolve this with a compromise let's set the cache live to 24 6 hours. When cache is older than 24 6 hours it will be ignored otherwise it's used.
This is significant improvement to container development!

Moved to #5073

Reduce size of the images

For all the DNF calls disable weak dependencies.
This is harder to test but should be easily to fix potential issues.
However, the saving should be nice. In my simple testing:

anaconda-ci on master branch:
original size: 2.68 GiB
new size: 2.22 GiB

Moved to #5072

Improve the documentation of the ISO builds

Reflect new changes.
Improve the state based on the feedback I've got.

@jkonecny12 jkonecny12 added infrastructure Changes affecting mostly infrastructure f39 f40 labels Aug 17, 2023
Copy link
Contributor

@VladimirSlavik VladimirSlavik left a comment

Choose a reason for hiding this comment

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

Thank you, this looks great!

Do you want to change also the workflows together with this, or leave it for later?

I wonder about the first commit. What are the risks of that change?

@jkonecny12 jkonecny12 force-pushed the master-fix-root-owning-files-after-iso-build branch from 61f2e83 to b51b643 Compare August 21, 2023 15:31
@github-actions
Copy link

Infrastructure check failed on these files. Please do a double check of these files before merge!

CONTRIBUTING.rst
Makefile.am

@VladimirSlavik
Copy link
Contributor

Please split the PR according to what you are fixing or changing. Owner after image build = one thing. Caching = second thing. Dependencies = third thing.

@jkonecny12 jkonecny12 changed the title Fix artifacts ownership after ISO builds Tweak Anaconda containers Aug 22, 2023
@VladimirSlavik
Copy link
Contributor

Looks like without weak deps, the tests don't succeed.

@jkonecny12
Copy link
Member Author

/kickstart-test --testtype smoke

Do not require people to use any Makefile commands as sudo. The sudo
calls should be solved inside of the Makefile.
Because the build of an ISO is done as root the output artifacts are
root too. To solve this copy the ownership from the input anaconda RPMs
to the output artifacts.
@jkonecny12 jkonecny12 force-pushed the master-fix-root-owning-files-after-iso-build branch from b51b643 to 2cb2b61 Compare August 23, 2023 15:21
@github-actions
Copy link

Infrastructure check failed on these files. Please do a double check of these files before merge!

CONTRIBUTING.rst
Makefile.am

@jkonecny12
Copy link
Member Author

Moved the parts of the PR to separate PRs:
#5072
#5073

@jkonecny12
Copy link
Member Author

/kickstart-test --testtype smoke

@jkonecny12 jkonecny12 changed the title Tweak Anaconda containers Improve build of the ISO creating containers Aug 23, 2023
Copy link
Contributor

@VladimirSlavik VladimirSlavik left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thank you!

@VladimirSlavik
Copy link
Contributor

Should be good to merge, I'll change the build action in #5071 as needed.

@jkonecny12 jkonecny12 merged commit 3d84202 into rhinstaller:master Aug 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
f39 f40 infrastructure Changes affecting mostly infrastructure
Development

Successfully merging this pull request may close these issues.

2 participants