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

Adding back in missing functionality of Custom Docker Image Builds #9756

Closed

Conversation

james-crowley
Copy link

SUMMARY

With the switch to 18.0, the removal of local docker install was removed. During this shuffle of code, it looks like some functionality was dropped. This PR aims to add back in that functionality. Plus add unofficial support for ppc64le like there is for arm64.

ISSUE TYPE
  • Feature/Bug Pull Request
COMPONENT NAME
  • Installer
AWX VERSION
awx: 18.0.0
ADDITIONAL INFORMATION

The functionality that was delete/shifted was:

  • Allowing for custom build images to be pushed to remote registry
  • Allowing for multi-arch builds via rendering the right download paths

New Stuff Added:

  • Unofficial support for build ppc64le
  • Support for custom base images(aka if a certain arch does not support centos:8, you could swap it for fedora or something similar)

…support for custom base images/containers. Added back in support for pushing to remote registry. Also added unofficial support in for ppc64le

Signed-off-by: James-Crowley <James.Crowley@ibm.com>
@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@james-crowley
Copy link
Author

@vielmetti If you get the chance, do you mind confirming that arm64 support is back in for custom built images in my PR? I think #9289 broke it in the reshuffling of code.

# Helper vars to construct the proper multi-arch configuration
tini_architecture: '{{ { "x86_64": "amd64", "aarch64": "arm64", "armv7": "arm", "ppc64le": "ppc64le" }[ansible_facts.architecture] }}'
kubectl_architecture: '{{ { "x86_64": "amd64", "aarch64": "arm64", "armv7": "arm", "ppc64le": "ppc64le" }[ansible_facts.architecture] }}'
base_container: '{{ { "x86_64": "centos:8", "aarch64": "centos:8", "armv7": "centos:8", "ppc64le": "centos:8" }[ansible_facts.architecture] }}'
Copy link
Contributor

Choose a reason for hiding this comment

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

For base_container, why use a mapping if all keys point to the same container image?

Copy link
Author

Choose a reason for hiding this comment

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

@jakemcdermott This is to allow for other architectures that do not have centos:8 available, notably s390x. In my testing, I was able to add an entry into the map here that looked something like:

"s390x": "fedora:32"

Which thus allowed me to build custom images for s390x. The reason I did not add s390x into the existing map is it requires other jinja edits in the Dockerfile template. I was going to do some more testing and do a PR later to add in unofficial support for s390x. But I thought adding in the base_container variable is useful right now for others.

Copy link
Author

Choose a reason for hiding this comment

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

You can see some of the smaller edits I backed ported into 17.1.0 to add s390x support here.

Copy link
Author

Choose a reason for hiding this comment

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

@jakemcdermott Are you fine with this addition?

Signed-off-by: James-Crowley <James.Crowley@ibm.com>
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@shanemcd
Copy link
Member

Hello. If you could please rebase and resolve the conflict, we can merge this.

@shanemcd
Copy link
Member

shanemcd commented Oct 2, 2021

Closing due to lack of response.

@shanemcd shanemcd closed this Oct 2, 2021
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.

4 participants