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

configurable engine for container images #6400

Closed
wants to merge 1 commit into from

Conversation

itroyano
Copy link

@itroyano itroyano commented Apr 20, 2023

Description of the change:

Allow the container engine to be configurable in the generated Makefile.

Motivation for the change:
For users running Podman or other container runtimes, this adds the capability to run make commands with a simple variable specifying their runtime.

Checklist

If the pull request includes user-facing changes, extra documentation is required:

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 24, 2023
Copy link
Contributor

@everettraven everettraven left a comment

Choose a reason for hiding this comment

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

@itroyano Thanks for the contribution - I apologize for the long delay in receiving a review. This PR seems to have gotten lost in the ocean of other PRs we had (I've been going through and cleaning them up).

If you don't mind rebasing and resolving any conflicts I'd be happy to approve this PR and work on getting it merged in.

@itroyano itroyano force-pushed the imageBuilders branch 2 times, most recently from 954735e to 3f8e0e3 Compare July 22, 2023 18:00
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 22, 2023
@itroyano
Copy link
Author

Hi @everettraven,
Thanks for taking a look. I've rebased now.

@everettraven
Copy link
Contributor

@itroyano Looks like the DCO check is now failing - if you look at the details of the failing check it should tell you how to resolve it, but the gist is that your commit is missing the sign-off

@itroyano itroyano temporarily deployed to deploy July 24, 2023 13:23 — with GitHub Actions Inactive
@itroyano itroyano temporarily deployed to deploy July 24, 2023 13:23 — with GitHub Actions Inactive
@itroyano itroyano temporarily deployed to deploy July 24, 2023 13:23 — with GitHub Actions Inactive
@itroyano itroyano temporarily deployed to deploy July 24, 2023 13:23 — with GitHub Actions Inactive
@itroyano
Copy link
Author

Added.

@itroyano itroyano temporarily deployed to deploy July 24, 2023 13:52 — with GitHub Actions Inactive
@itroyano itroyano temporarily deployed to deploy July 24, 2023 13:52 — with GitHub Actions Inactive
@itroyano itroyano temporarily deployed to deploy July 24, 2023 13:52 — with GitHub Actions Inactive
@itroyano itroyano temporarily deployed to deploy July 24, 2023 13:52 — with GitHub Actions Inactive
@itroyano itroyano temporarily deployed to deploy July 24, 2023 13:52 — with GitHub Actions Inactive
@itroyano itroyano temporarily deployed to deploy July 24, 2023 13:52 — with GitHub Actions Inactive
@@ -392,6 +392,8 @@ Note that:
- `operator-sdk generate crds` is replaced with `make manifests`, which generates CRDs and RBAC rules.
- `operator-sdk build` is replaced with `make docker-build IMG=<some-registry>/<project-name>:<tag>`.

`IMAGE_BUILDER` can be used here to specify your container engine. default is `docker`.
Copy link
Member

Choose a reason for hiding this comment

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

Have we already pulled this in from Kubebuilder in golang, since we are modifying its tutorial here?

Copy link
Author

Choose a reason for hiding this comment

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

Truth be told I just did a search and replace on all the docs that mention build.
I can remove if it doesn't belong here.

Signed-off-by: Igor Troyanovsky <itroyano@redhat.com>
@itroyano itroyano temporarily deployed to deploy July 25, 2023 13:35 — with GitHub Actions Inactive
@itroyano itroyano temporarily deployed to deploy July 25, 2023 13:35 — with GitHub Actions Inactive
@itroyano itroyano temporarily deployed to deploy July 25, 2023 13:36 — with GitHub Actions Inactive
@itroyano itroyano temporarily deployed to deploy July 25, 2023 13:36 — with GitHub Actions Inactive
@itroyano itroyano temporarily deployed to deploy July 25, 2023 13:43 — with GitHub Actions Inactive
@itroyano itroyano temporarily deployed to deploy July 25, 2023 13:43 — with GitHub Actions Inactive
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 28, 2023
@openshift-merge-robot
Copy link

PR needs rebase.

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/test-infra repository.

@openshift-bot
Copy link

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci openshift-ci bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 28, 2023
@itroyano
Copy link
Author

itroyano commented Jan 4, 2024

I see the latest version added a CONTAINER_TOOL option. Thanks!
This PR can be closed now.

@itroyano itroyano closed this Jan 4, 2024
@itroyano itroyano deleted the imageBuilders branch January 4, 2024 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants