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

make containerized make targets support alternative container runtimes #906

Closed
wants to merge 22 commits into from

Conversation

tal-hason
Copy link
Contributor

as of #905

i have update the make file to work with an environment variable CONTAINER_RUNTIME, replaced it with the "docker" in the files

at the beginning of the make file I have added a bash script that checks if podman or docker exists and stores it in the CONTAINER_RUNTIME env. var

@netlify
Copy link

netlify bot commented Oct 2, 2023

Deploy Preview for docs-kargo-akuity-io ready!

Name Link
🔨 Latest commit 239a9ac
🔍 Latest deploy log https://app.netlify.com/sites/docs-kargo-akuity-io/deploys/652855b70b5402000818dc66
😎 Deploy Preview https://deploy-preview-906.kargo.akuity.io
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@tal-hason tal-hason force-pushed the #905/podman-support branch from 258d7e9 to b345320 Compare October 2, 2023 11:10
@krancour
Copy link
Member

krancour commented Oct 2, 2023

I'd prefer this just be an env var you can set, with docker as the default. This script will always prefer podman over docker if podman is found and I don't think it's reasonable to infer that everyone who has podman prefers it over docker.

@tal-hason
Copy link
Contributor Author

i can replace the order the docker will be the first and podman the second

or

make just as the env var with remarks

your call

@tal-hason tal-hason force-pushed the #905/podman-support branch from 98ddef2 to 125e26e Compare October 2, 2023 12:33
@krancour
Copy link
Member

krancour commented Oct 2, 2023

The env var... the thing is that I anticipate there will be plenty of people who prefer one or the other but happen to have both for... reasons.

@tal-hason
Copy link
Contributor Author

No prob, I will patch that later

Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@tal-hason tal-hason force-pushed the #905/podman-support branch from 69ed17e to e3b240e Compare October 2, 2023 20:25
@krancour krancour changed the title #905/podman support make containerized make targets support alternative container runtimes Oct 2, 2023
Comment on lines 21 to 27
> you can use podman by creating an environment variable
```bash
export CONTAINER_RUNTIME=podman
```

[How to config podman socket](https://docs.podman.ioen/latest/markdown/podman-system-service.1.html#examples)
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to be podman-specific here?

Copy link
Member

Choose a reason for hiding this comment

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

Do we need to be podman-specific here?

I realized my meaning might have been ambiguous.

The comment wasn't about the podman socket setup.

It was about the general idea of this PR adding prodman support.

I'm fine with the notion of supporting alternative container runtimes as long as they are "Docker compatible," but I'm less ok with presenting podman as a specially preferred/specially supported alternative container runtime... if that makes any sense.

I think the easiest thing to do here might be to just not mention this in the docs. The changes to the Makefile are good and I think anyone who really cares about using an alternative container runtime will find the way of doing it easily enough.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Signed-off-by: Tal Hason <thason@redhat.com>
Signed-off-by: Tal <thason@redhat.com>
Signed-off-by: Tal Hason <thason@redhat.com>
tal-hason and others added 19 commits October 6, 2023 17:31
Signed-off-by: Tal Hason <thason@redhat.com>
Signed-off-by: Tal Hason <thason@redhat.com>
…th podman instructions

Signed-off-by: Tal Hason <thason@redhat.com>
Signed-off-by: Tal Hason <thason@redhat.com>
Co-authored-by: Kent Rancourt <kent.rancourt@gmail.com>
Signed-off-by: Tal Hason <thason@redhat.com>
Signed-off-by: Kent <kent.rancourt@gmail.com>
Signed-off-by: Tal Hason <thason@redhat.com>
Co-authored-by: Kent Rancourt <kent.rancourt@gmail.com>
Signed-off-by: Tal Hason <thason@redhat.com>
Signed-off-by: Tal Hason <thason@redhat.com>
Signed-off-by: Tal Hason <thason@redhat.com>
Signed-off-by: Tal Hason <thason@redhat.com>
Signed-off-by: Tal Hason <thason@redhat.com>
Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>
Signed-off-by: Tal Hason <thason@redhat.com>
Signed-off-by: Tal Hason <thason@redhat.com>
Signed-off-by: dhanu <andreasdhanu@gmail.com>
Co-authored-by: Kent Rancourt <kent.rancourt@gmail.com>
Signed-off-by: Tal Hason <thason@redhat.com>
Signed-off-by: Tal Hason <thason@redhat.com>
Signed-off-by: Samuel Suter <samuel@suter.is>
Signed-off-by: Tal Hason <thason@redhat.com>
Signed-off-by: Tal Hason <thason@redhat.com>
Signed-off-by: Tal Hason <thason@redhat.com>
@tal-hason tal-hason force-pushed the #905/podman-support branch from 82f978f to 948c8db Compare October 6, 2023 14:31
@krancour
Copy link
Member

@tal-hason idk what happened with this PR, but it seems to include changes that are already in main. I'm not sure how that would have happened. Do you want to force push just the relevant commit or two to here? Alternatively, you could close this PR and open a new one having just the relevant changes.

Sorry... really no idea how it ended up in this odd state.

@tal-hason tal-hason closed this Oct 11, 2023
@tal-hason tal-hason reopened this Oct 11, 2023
@tal-hason
Copy link
Contributor Author

@tal-hason idk what happened with this PR, but it seems to include changes that are already in main. I'm not sure how that would have happened. Do you want to force push just the relevant commit or two to here? Alternatively, you could close this PR and open a new one having just the relevant changes.

Sorry... really no idea how it ended up in this odd state.

i can make a new branch from a fresh and just add the changes, maybe later on

@tal-hason
Copy link
Contributor Author

Opened #950 PR with a fresh branch

@tal-hason tal-hason closed this Oct 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants