Skip to content

commands: make build podman compatible #1207

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

Closed
wants to merge 1 commit into from

Conversation

delvin1933
Copy link

Description of the change:

When running 'operator-sdk' build with podman-docker the build fails
with a path error. Removing -f and setting the PATH argument to ./build
allows podman to build the container.

Has been tested against podman 1.12 on fedora 29 and docker-ce 5:18.09.33-0debian-stretch on debian stretch

Motivation for the change:

I want to be able to build the operator container with podman as I don't use docker

When running 'operator-sdk' build with podman-docker the build fails
with a path error. Removing -f and setting the PATH argument to ./build
allows podman to build the container.
@openshift-ci-robot openshift-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Mar 14, 2019
@openshift-ci-robot
Copy link

Hi @christophefarges. Thanks for your PR.

I'm waiting for a operator-framework or openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@joelanford
Copy link
Member

@christophefarges I don't think this change will work because the Dockerfile expects the working directory to be <operatorRootDir>, not <operatorRootDir>/build.

I tested your PR with docker-ce 18.06.1, and got the following error:

$ operator-sdk build go-operator
INFO[0000] Building Docker image go-operator            
Sending build context to Docker daemon  40.22MB
Step 1/7 : FROM registry.access.redhat.com/ubi7-dev-preview/ubi-minimal:7.6
 ---> 1f8526ca508d
Step 2/7 : ENV OPERATOR=/usr/local/bin/go-operator     USER_UID=1001     USER_NAME=go-operator
 ---> Using cache
 ---> 2027d56c12c5
Step 3/7 : COPY build/_output/bin/go-operator ${OPERATOR}
COPY failed: stat /var/lib/docker/tmp/docker-builder498450201/build/_output/bin/go-operator: no such file or directory
Error: failed to output build image go-operator: (failed to exec []string{"docker", "build", "build/", "-t", "go-operator"}: exit status 1)
Usage:
  operator-sdk build <image> [flags]

Flags:
      --docker-build-args string     Extra docker build arguments as one string such as "--build-arg https_proxy=$https_proxy"
      --enable-tests                 Enable in-cluster testing by adding test binary to the image
  -h, --help                         help for build
      --namespaced-manifest string   Path of namespaced resources manifest for tests (default "deploy/operator.yaml")
      --test-location string         Location of tests (default "./test/e2e")

Copy link
Member

@joelanford joelanford left a comment

Choose a reason for hiding this comment

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

@christophefarges Can you make this change and re-test with both docker and podman?

@@ -172,7 +172,7 @@ func buildFunc(cmd *cobra.Command, args []string) error {

log.Infof("Building Docker image %s", baseImageName)

dbArgs := []string{"build", ".", "-f", "build/Dockerfile", "-t", baseImageName}
dbArgs := []string{"build", "build/", "-t", baseImageName}
Copy link
Member

Choose a reason for hiding this comment

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

I think with this change, the build command will work with both docker and podman. It seems like podman is a bit more particular about the order of the arguments.

Suggested change
dbArgs := []string{"build", "build/", "-t", baseImageName}
dbArgs := []string{"build", "-f", "build/Dockerfile", "-t", baseImageName, "."}

@patrickeasters
Copy link

@christophefarges any traction on those changes? I'd very much like podman support as well.

@openshift-ci-robot
Copy link

@christophefarges: 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-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 13, 2019
@lilic
Copy link
Member

lilic commented Apr 16, 2019

@christophefarges Pinging again, if you are too busy one of us can also take over this PR. :)

@@ -172,7 +172,7 @@ func buildFunc(cmd *cobra.Command, args []string) error {

log.Infof("Building Docker image %s", baseImageName)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
log.Infof("Building Docker image %s", baseImageName)
log.Infof("Building container image %s", baseImageName)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this can be ignored after rebase: now that #1311 is merged, it's "OCI image".

There are likely other things that need to be changed now that that's merged, such as adding podman to the list of imageBuilder options

@jdoss
Copy link

jdoss commented May 19, 2019

Sorry for the enduser ping. I also do not use Docker and I would love to see better support for podman.

@patrickeasters
Copy link

I don't have an issue using podman in the master branch now. It looks like the work in #1311 fixed the order of the arguments. Granted, this does require the podman-docker package or some other sort of symlink, so it may still be worth adding podman explicitly as an option alongside docker and buildah.

@joelanford
Copy link
Member

Since the original intent of this PR seems to be resolved, I'll close this.

I've also submitted #1488 to add a podman option to the operator-sdk build --image-builder flag.

@joelanford joelanford closed this May 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants