-
Notifications
You must be signed in to change notification settings - Fork 13
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
ROX-24040: Set Podman instead of Docker to fix RHEL8 builders #1917
Conversation
@@ -36,6 +36,9 @@ IMAGE_REPOSITORY="${QUAY_IMAGE_REPOSITORY:-app-sre/acs-fleet-manager}" | |||
PROBE_IMAGE_REPOSITORY="${PROBE_QUAY_IMAGE_REPOSITORY:-app-sre/acscs-probe}" | |||
EMAILSENDER_IMAGE_REPOSITORY="${PROBE_QUAY_IMAGE_REPOSITORY:-app-sre/acscs-emailsender}" | |||
|
|||
# Support RHEL8 build images for App-Interface ci-ext | |||
DOCKER=podman | |||
|
|||
source ./scripts/build_setup.sh | |||
|
|||
# Push the image: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment for the lines 47, 57:
Do we need remove this for podman or use an alternative?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure which lines /flags you mean, can you please specify?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DOCKER_CONFIG=${DOCKER_CONFIG}
We used it only on app-interface and I'm happy to get rid of it.
I don't think it makes any difference to podman.
I'm not sure if |
377236d
to
53e3b34
Compare
53e3b34
to
c3508bc
Compare
c3508bc
to
1b58eca
Compare
1b58eca
to
c9de2dd
Compare
c9de2dd
to
0df3ecb
Compare
@@ -497,12 +497,12 @@ docker/login: docker/login/fleet-manager | |||
.PHONY: docker/login | |||
|
|||
docker/login/fleet-manager: | |||
@docker logout quay.io | |||
$(DOCKER) logout quay.io || true # Swallog podman error if not logged in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: How about renaming DOCKER
to CONTAINER_ENGINE
?
0df3ecb
to
f910aa3
Compare
71d8187
to
feba6e3
Compare
feba6e3
to
26bb920
Compare
26bb920
to
2d07a64
Compare
@kovayur PTAL |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ebensh, ludydoo The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
2d07a64
to
bbcafce
Compare
New changes are detected. LGTM label has been removed. |
@@ -66,10 +66,6 @@ else | |||
GOBIN=$(shell $(GO) env GOBIN) | |||
endif | |||
|
|||
ifeq ($(IMAGE_PLATFORM),) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will break multi-arch builds.
- We build arm images on Github actions
- We can build x86 images for openshift internal registry (image/push/internal) on arm based machines
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-sigs/prow repository. |
buildx build --push is not yet supported by Podman: containers/buildah#4677
So we switch to single-arch build, then build and push separately to match the other deployments.
Tested: