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

RHOAIENG-16076: tests(gha): change to using rootful podman, so that we can share containers/storage with cri-o later #782

Conversation

jiridanek
Copy link
Member

@jiridanek jiridanek commented Nov 24, 2024

https://issues.redhat.com/browse/RHOAIENG-16076

Another chunk of the

changes to be reviewed and merged.

Description

I need to save the disk space, so I can't just copy the built images into KinD or some such local instance of kubernetes where I can run the Makefile tests. Therefore, I need to build the images in a rootful Podman, and then (subsequent PR) I'll be able to reuse it's containers/storage directory in cri-o backed kubernetes.

https://podman-desktop.io/blog/sharing-podman-images-with-kubernetes-cluster#introduction

How Has This Been Tested?

GHA

Merge criteria:

  • The commits are squashed in a cohesive manner and have meaningful messages.
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has manually tested the changes and verified that the changes work

@openshift-ci openshift-ci bot requested review from dibryant and harshad16 November 24, 2024 17:28

[Socket]
ListenStream=%t/podman/podman.sock
SocketMode=0666
Copy link
Member Author

Choose a reason for hiding this comment

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

@jiridanek jiridanek added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Nov 24, 2024
@jiridanek jiridanek changed the title RHOAIENG-16076: tests(gha): change to using rootful podman, so that we can share containers/storage with cri-o later RHOAIENG-16076: tests(gha): change to using rootful podman, so that we can share containers/storage with cri-o later Nov 24, 2024
@jiridanek
Copy link
Member Author

/override ci/prow/images

Copy link
Contributor

openshift-ci bot commented Nov 26, 2024

@jiridanek: Overrode contexts on behalf of jiridanek: ci/prow/images

In response to this:

/override ci/prow/images

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

…e can share containers/storage with cri-o later
@jiridanek jiridanek force-pushed the jd_notebooks_gha_make/rootfulpodman branch from acb4aa5 to 0237d4a Compare November 26, 2024 07:15
@jiridanek
Copy link
Member Author

/override ci/prow/images

Copy link
Contributor

openshift-ci bot commented Nov 26, 2024

@jiridanek: Overrode contexts on behalf of jiridanek: ci/prow/images

In response to this:

/override ci/prow/images

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@jstourac
Copy link
Member

I have to admit I don't fully follow all of the configuration changes here, but LGTM in general overview and if it works and helps to reduce the disk-space utilized, I'm all for this.

Thank you, Jiri!

/lgtm

# Use the rootful instance of podman for sharing images with cri-o
# https://podman-desktop.io/blog/sharing-podman-images-with-kubernetes-cluster#introduction
# https://access.redhat.com/solutions/6986565
CONTAINER_HOST: unix:///var/run/podman/podman.sock
Copy link
Member Author

Choose a reason for hiding this comment

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

when podman cli is started, it consults this env variable; when it is present, then it connects (as a client) to this socket, so things work pretty much as they would in the docker world

the server can be a systemd service running as root, and if current user has permissions on the socket (it has, because of 0666), then we run rootful podman

Comment on lines +116 to +123
# podman from brew has its own /etc (was giving me Failed to obtain podman configuration: runroot must be set)
# the (default) config location is also where cri-o gets its storage defaults (that can be overriden in crio.conf)
sudo cp ci/cached-builds/containers.conf /etc/containers.conf
sudo cp ci/cached-builds/containers.conf /home/linuxbrew/.linuxbrew/opt/podman/etc/containers.conf
sudo cp ci/cached-builds/storage.conf /etc/containers/storage.conf
sudo cp ci/cached-builds/storage.conf /home/linuxbrew/.linuxbrew/opt/podman/etc/containers/storage.conf
sudo cp ci/cached-builds/registries.conf /etc/containers/registries.conf
sudo cp ci/cached-builds/registries.conf /home/linuxbrew/.linuxbrew/opt/podman/etc/containers/registries.conf
Copy link
Member Author

Choose a reason for hiding this comment

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

these are just the default config files, but I need to have some nondefault values in them, mostly the path to containers/storage; it's pretty much the same as it was before, but i'm putting them into the systemwide location, not just into user directory, because now we run rootful podman

also, I am putting it into the systemwide locations for crio as well as podman, and because podman is installed from homebrew, it lives under /home/linuxbrew/.linuxbrew/opt/podman/etc; weird but so is life

Comment on lines +4 to +6
# Failed to pull image "bitnami/kubectl:1.26.4": reading manifest 1.26.4 in quay.io/bitnami/kubectl: unauthorized: access to the requested resource is not authorized
unqualified-search-registries = [ "docker.io" ]
short-name-mode = "enforcing"
Copy link
Member Author

Choose a reason for hiding this comment

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

i only saw this when I was doing something with kyverno (I don't intend to push that), but it looks like a good default; kyverno only has images in docker.io, but with the previous defaults somehow an attempt to pull from quay.io was made; let's stop that

@jiridanek
Copy link
Member Author

thanks for the review; I added some comments, and I'd be happy to discuss this more; it is possible I am doing something overly complicated here; it's always best to have simple infra as possible

/approve

Copy link
Contributor

openshift-ci bot commented Nov 26, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jiridanek

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-bot openshift-merge-bot bot merged commit 7650cd8 into opendatahub-io:main Nov 26, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved lgtm tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants