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

Unify in_podman container packaging & VM packaging #5853

Merged
merged 2 commits into from
Apr 29, 2020
Merged

Unify in_podman container packaging & VM packaging #5853

merged 2 commits into from
Apr 29, 2020

Conversation

cevich
Copy link
Member

@cevich cevich commented Apr 16, 2020 •
edited
Loading

Depends on optional: #5737

Significant changes in this PR:

  • Separate out package update + install from the VM Image setup process, into dedicated scripts.
  • Reuse VM Image packaging scripts, for in_podman container image (Dockerfile + Dockerfile.ubuntu).
  • Add test-builds of the container images to the VM Image build process, to provide feedback upon future VM Image setup changes.

Note: Both the VM Images and in_podman container image are utilized outside of the libpod repository. For example,

@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 16, 2020
@cevich cevich force-pushed the unify_in_podman_build branch from 0df19cf to 8f6954f Compare April 16, 2020 20:47
@rh-atomic-bot
Copy link
Collaborator

☔ The latest upstream changes (presumably #5852) made this pull request unmergeable. Please resolve the merge conflicts.

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 17, 2020
@cevich cevich force-pushed the unify_in_podman_build branch from 8f6954f to ea25a04 Compare April 20, 2020 18:25
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 20, 2020
@cevich cevich force-pushed the unify_in_podman_build branch 3 times, most recently from 2426812 to f3490ba Compare April 20, 2020 18:42
@cevich cevich changed the title WIP: Unify in_podman container packaging & VM packaging WIP: CI:IMG: Unify in_podman container packaging & VM packaging Apr 20, 2020
@cevich cevich force-pushed the unify_in_podman_build branch 6 times, most recently from 59959c0 to 86851c7 Compare April 20, 2020 18:57
jwhonce
jwhonce reviewed Apr 20, 2020
View reviewed changes
contrib/cirrus/lib.sh Show resolved Hide resolved
contrib/cirrus/lib.sh
_finalize
}

ubuntu_finalize() {
set +e # Don't fail at the very end
echo "Resetting to fresh-state for usage as cloud-image."
$LILTO $SUDOAPTGET autoremove
sudo rm -rf /var/cache/apt
$SUDO rm -rf /var/cache/apt
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
$SUDO rm -rf /var/cache/apt
$SUDO rm -r /var/cache/apt

If you expect the cache to really be empty drop the -f.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure I follow the suggestion. This is another VM Image build-time thing where it's probably okay to ignore a 'does not exist' error. Am I missing something?

contrib/cirrus/packer/fedora_packaging.sh Outdated Show resolved Hide resolved
contrib/cirrus/packer/fedora_setup.sh Outdated Show resolved Hide resolved
contrib/cirrus/setup_environment.sh Outdated Show resolved Hide resolved
@cevich cevich force-pushed the unify_in_podman_build branch from 86851c7 to 362fc3c Compare April 21, 2020 13:12
@cevich cevich mentioned this pull request Apr 21, 2020
Merged
@cevich cevich force-pushed the unify_in_podman_build branch 4 times, most recently from 9c1cd7c to 7eadab9 Compare April 22, 2020 13:47
@cevich cevich mentioned this pull request Apr 22, 2020
Merged
@cevich cevich changed the title WIP: CI:IMG: Unify in_podman container packaging & VM packaging WIP: Unify in_podman container packaging & VM packaging Apr 22, 2020
@cevich cevich force-pushed the unify_in_podman_build branch 2 times, most recently from 3fb75b6 to 953508c Compare April 23, 2020 15:39
@cevich cevich changed the title WIP: Unify in_podman container packaging & VM packaging Unify in_podman container packaging & VM packaging Apr 24, 2020
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 24, 2020
cevich added 2 commits April 24, 2020 08:22
@cevich
Cirrus: Unify package installation
97ecd21 
Also, test-build critical container images depended upon for
CI-purposes.

Signed-off-by: Chris Evich <[email protected]>
@cevich
Cirrus: Utilize new VM images
62accbc 
Signed-off-by: Chris Evich <[email protected]>
@cevich cevich force-pushed the unify_in_podman_build branch from 953508c to 62accbc Compare April 24, 2020 12:22
@cevich
Copy link
Member Author

cevich commented Apr 24, 2020

@baude do you mind taking a quick peek at the not-required but running tests, just to verify they appear to be operating as expected? In other words, I'm not sure what tests are expected to fail and pass or error.

@cevich
Copy link
Member Author

cevich commented Apr 29, 2020

@baude @mheon ping. Can we get this PR merged soon-ish? I've got another followup PR rolling, to update us to F32 proper and fix an SELinux-related SNAFU affecting buildah..

@rhatdan
Copy link
Member

rhatdan commented Apr 29, 2020

/approve

@openshift-ci-robot
Copy link
Collaborator

[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-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 29, 2020
@TomSweeneyRedHat
Copy link
Member

It would be nice to rename all the "Dockerfile." files to "Containerfile." if possible.

@baude
Copy link
Member

baude commented Apr 29, 2020

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 29, 2020
@openshift-merge-robot openshift-merge-robot merged commit 620baaf into containers:master Apr 29, 2020
@cevich
Copy link
Member Author

cevich commented Apr 29, 2020

@TomSweeneyRedHat I agree, but also/first we need to convince all the docker-centric build services (like quay.io) to recognize it too :S

(I'm assuming it doesn't, maybe I'm wrong?)

@TomSweeneyRedHat
Copy link
Member

@cevich, that's the caveat. Many (all?) of the docker-centric build services don't recognize a Containerfile, I know for a fact that quay.io does not, but I hope to at least change that.

So if there's some reliance on the file being named "Dockerfile" so it can be found by some system expecting that name, then I'd leave it as is. I'm guessing, however, that the Dockerfile.{extension} files can be changed to "Containerfile.{extension}" as they are not likely used by a build system.

@cevich cevich deleted the unify_in_podman_build branch June 30, 2021 18:12
@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 Sep 22, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jwhonce jwhonce jwhonce left review comments

@rhatdan rhatdan Awaiting requested review from rhatdan

Assignees

@baude baude

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.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@cevich @rh-atomic-bot @rhatdan @openshift-ci-robot @TomSweeneyRedHat @baude @jwhonce @openshift-merge-robot