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

Only add some steps if EE_BUILDER_IMAGE is defined #450

Closed

Conversation

john-westcott-iv
Copy link
Member

I wanted to take a base image and then just add a couple of prepend steps to add some downloadable executables. So I did not define an EE_BUILDER_IAMGE (since nothing new was going to be built). When I did this I got errors because some of the steps in the context/Containerfile assumed I would have a build image. This change prevents these lines from making it into the file if EE_BUILDER_IMAGE is not defined.

To reproduce, create a builder file like:

---
version: 2

additional_build_steps:
  append:
    - RUN curl -L -o /usr/local/bin/operator-sdk https://github.com/operator-framework/operator-sdk/releases/download/v1.27.0/operator-sdk_linux_amd64
    - RUN chmod +x /usr/local/bin/operator-sdk
    - RUN curl -s "https://raw.githubusercontent.com/kubernetes-sigs/kustomize/master/hack/install_kustomize.sh"  | bash -s /usr/local/bin
    - RUN curl --output - https://mirror.openshift.com/pub/openshift-v4/x86_64/clients/ocp/latest-4.9/opm-linux.tar.gz | tar -xzv --directory /usr/local/bin

images:
  base_image:
    name: quay.io/ansible/awx-ee:latest

Run: ansible-builder build -f operator_hub_release.yml --tag quay.io/rhn_gps_jowestco/operator-hub-ee

At first you should get the following error because EE_BUILDER_IMAGE is None:

podman build -f context/Containerfile -t quay.io/rhn_gps_jowestco/operator-hub-ee context
[1/3] STEP 1/4: FROM quay.io/ansible/awx-ee:latest AS galaxy
[1/3] STEP 2/4: ARG ANSIBLE_GALAXY_CLI_COLLECTION_OPTS=
--> Using cache 9139fb655e1886da23b7a5c74d44d84fd55b5e1067d7753e3d54b3a7636a95b7
--> 9139fb655e1
[1/3] STEP 3/4: ARG ANSIBLE_GALAXY_CLI_ROLE_OPTS=
--> Using cache 9827f677dd161bb2ad88ed30602d4d8f3caa67cc023ce1e69408d57d7d28d838
--> 9827f677dd1
[1/3] STEP 4/4: USER root
--> Using cache 174abd59005ddd1bb6bcf60bdc03b517c8af64abb37bb8258b5d66a9158c2de6
--> 174abd59005
[2/3] STEP 1/1: FROM None AS builder
Error: error creating build container: repository name must be lowercase

An error occured (rc=125), see output line(s) above for details.

You can then add these if conditions one at a time until you can build the image successfully.

When I started going down the route of trying to add a EE_BUILD_IMAGE I was then getting errors because the dependency files were missing or empty.

@john-westcott-iv john-westcott-iv requested a review from a team as a code owner March 13, 2023 18:37
@github-actions github-actions bot added the needs_triage New item that needs to be triaged label Mar 13, 2023
@Shrews
Copy link
Contributor

Shrews commented Mar 13, 2023

Thanks for the PR.

The version 2 format is not really ready for prime time, especially in the devel branch as that is a very early release of the new format. A new version of builder is being actively developed in this PR that is going to greatly expand the changes to the version 2 format. And in fact, I think I already fixed this bug in one of the commits in that PR. I encourage you to give the code from that PR a try, but realize there are likely still bugs and more changes to come.

@Shrews Shrews removed the needs_triage New item that needs to be triaged label Mar 13, 2023
@Shrews
Copy link
Contributor

Shrews commented Mar 13, 2023

Going to close this as we aren't in a position to accept changes for builder 2.0 code in this branch quite yet and I'm fairly certain this bug is already fixed elsewhere anyway.

@Shrews Shrews closed this Mar 13, 2023
@Shrews
Copy link
Contributor

Shrews commented Mar 20, 2023

I think we'll have to fix this in ansible-builder version 1.2 so I'm going to re-open this. Sorry for closing it... I had forgotten this was released in that version, so thanks for this PR!

That being said, I think a better fix for this would be to have a check in Containerfile.write_containerfile() that skips the entire builder stage if no builder image is supplied. That way we don't missing any of those steps (looks like you missed a check for prepare_galaxy_copy_steps()). Also, we would need tests.

@Shrews Shrews reopened this Mar 20, 2023
@Shrews
Copy link
Contributor

Shrews commented Mar 20, 2023

After spending more time thinking about this, I think we cannot do my original suggestion. We must restore the ability for builder to default to the quay.io/ansible/ansible-builder:latest image, as the previous EE schema version does. The builder stage cannot be skipped because it does two things: builds a wheel cache for the python requirements, and generates a system dependency installation file for the install-from-bindep script to be called in a later stage. Allowing folks to skip this stage could cause a lot of confusion and breakage. As a side note, the next EE schema version will not require (nor allow) a builder image to be specified since it will use the base image.

Do you still want to work on this given the above info? If not, I'll take it upon myself to get this fixed for us.

@Shrews
Copy link
Contributor

Shrews commented Mar 21, 2023

Closing this in favor of PR #455

@Shrews Shrews closed this Mar 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants