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.

Sign up for GitHub

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

Jump to bottom

[CI:BUILD] Podman image: Mass cleanup + fix missing storage.conf #14437

Merged
merged 1 commit into from
Jun 3, 2022
Merged

[CI:BUILD] Podman image: Mass cleanup + fix missing storage.conf #14437

merged 1 commit into from
Jun 3, 2022

Conversation

cevich
Copy link
Member

@cevich cevich commented May 31, 2022 •
edited
Loading

As of Fedora 36, /etc/containers/storage.conf with defaults is
installed under /usr/share/containers/. This was causing builds
to fail in the necessary sed command that enables fuse-overlayfs.
Fix this by using sed on the new location with an output redirect
into the etc location.

Also, perform a mass-cleanup of the three files to make them easier
to read/maintain. Including renaming them to Containerfile,
since all native build tooling is now used to produce them.

Lastly, take advantage of the podman-next copr repository to install
the latest/greatest podman from main, rather than building it from
scratch. This will greatly speed up the image build speed.

Does this PR introduce a user-facing change?

None

@openshift-ci openshift-ci bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. do-not-merge/release-note-label-needed Enforce release-note requirement, even if just None release-note-none and removed do-not-merge/release-note-label-needed Enforce release-note requirement, even if just None labels May 31, 2022
@rhatdan
Copy link
Member

rhatdan commented May 31, 2022

Isn't /usr/share/containers/storage.conf there?

@cevich cevich mentioned this pull request May 31, 2022
Merged
@cevich cevich force-pushed the fix_podmanimage branch from e1a3301 to eccad7d Compare May 31, 2022 19:56
@cevich
Copy link
Member Author

cevich commented May 31, 2022

Isn't /usr/share/containers/storage.conf there?

Yeah, it's there. I thought the idea was for users to just set the non-default items and have /usr/share... for reference (like 'containers.conf'). So that;'s what I did here. Also I thought this was a simpler fix since it doesn't involve humans groking any regular expressions 😀 Is there a reason I'm not aware of to copy storage.conf and do "the sed thing"?

@rhatdan
Copy link
Member

rhatdan commented May 31, 2022

Nope copy/sed should be fine.

@cevich
Copy link
Member Author

cevich commented Jun 1, 2022

Nope copy/sed should be fine.

Okay, I'll make that change.

@cevich cevich force-pushed the fix_podmanimage branch from eccad7d to dd019cc Compare June 1, 2022 15:05
@cevich cevich marked this pull request as ready for review June 1, 2022 17:11
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 1, 2022
@TomSweeneyRedHat
Copy link
Member

Other than a question and 3 probable nits.
LGTM

@rhatdan
Copy link
Member

rhatdan commented Jun 1, 2022

LGTM other then Tom's changes.

@cevich cevich force-pushed the fix_podmanimage branch from dd019cc to 7fc8d22 Compare June 1, 2022 20:31
@cevich
Copy link
Member Author

cevich commented Jun 1, 2022

Force-push: removed file-name comments in Containerfiles.

@cevich cevich force-pushed the fix_podmanimage branch from 7fc8d22 to be185d4 Compare June 1, 2022 20:51
@cevich
Copy link
Member Author

cevich commented Jun 1, 2022

force-push: implemented @nalind suggestion.

@cevich
Podman image: Mass cleanup + fix missing storage.conf
fc95f83 
As of Fedora 36, `/etc/containers/storage.conf` with defaults is
installed under `/usr/share/containers/`.  This was causing builds
to fail in the necessary `sed` command that enables fuse-overlayfs.
Fix this by using sed on the new location with an output redirect
into the `etc` location.

Also, perform a mass-cleanup of the three files to make them easier
to read/maintain.  Including renaming them to `Containerfile`,
since all native build tooling is now used to produce them.

Lastly, take advantage of the `podman-next` copr repository to install
the latest/greatest podman from `main`, rather than building it from
scratch.  This will greatly speed up the image build speed.

Signed-off-by: Chris Evich <[email protected]>
@cevich cevich force-pushed the fix_podmanimage branch from be185d4 to fc95f83 Compare June 1, 2022 21:25
@cevich
Copy link
Member Author

cevich commented Jun 1, 2022

force-push: Added back in filename comments

This was referenced Jun 1, 2022
Closed
Closed
@cevich
Copy link
Member Author

cevich commented Jun 1, 2022

I just triggered the test-builds.

@cevich
Copy link
Member Author

cevich commented Jun 2, 2022

Okay, this should be ready to go.

@rhatdan
Copy link
Member

rhatdan commented Jun 3, 2022

/approve
/lgtm

@cevich
Copy link
Member Author

cevich commented Jun 3, 2022

/wakeup bot
/reload
/coffee
/refresh
/workdamnyou
/enable
/rerun

@rhatdan rhatdan added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jun 3, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 3, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cevich, rhatdan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-robot openshift-merge-robot merged commit 0dda468 into containers:main Jun 3, 2022
@cevich cevich mentioned this pull request Jun 6, 2022
Merged
@cevich cevich mentioned this pull request Jun 28, 2022
Merged
@cevich cevich deleted the fix_podmanimage branch April 18, 2023 14:44
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Aug 31, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@TomSweeneyRedHat TomSweeneyRedHat TomSweeneyRedHat left review comments

@nalind nalind nalind left review comments

Assignees
No one assigned
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. release-note-none
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@cevich @rhatdan @TomSweeneyRedHat @nalind @openshift-merge-robot