Skip to content

fix: make deploy-olm on ARM clusters#1478

Merged
openshift-merge-bot[bot] merged 2 commits into
openshift:masterfrom
mateusoliveira43:fix/make-deploy-olm-arm
Aug 7, 2024
Merged

fix: make deploy-olm on ARM clusters#1478
openshift-merge-bot[bot] merged 2 commits into
openshift:masterfrom
mateusoliveira43:fix/make-deploy-olm-arm

Conversation

@mateusoliveira43
Copy link
Copy Markdown

@mateusoliveira43 mateusoliveira43 commented Jul 26, 2024

Why the changes were made

Allow make deploy-olm to also work in ARM clusters.

Related to openshift/release#54877

How to test the changes made

Run make deploy-olm on ARM and non ARM clusters and create DPA (to test quay images). Both should succeed.

Note: you will need to use unsupported override, since quay images are still not available in ARM architecture.

Signed-off-by: Mateus Oliveira <msouzaol@redhat.com>
@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 26, 2024
@mateusoliveira43

This comment was marked as outdated.

@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented Jul 26, 2024

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 26, 2024
Signed-off-by: Mateus Oliveira <msouzaol@redhat.com>
Copy link
Copy Markdown
Contributor

@weshayutin weshayutin left a comment

Choose a reason for hiding this comment

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

whayutin@thinkdoe:/OPENSHIFT/git/OADP/oadp-operator$ oc get node -o jsonpath='{.items[0].status.nodeInfo.operatingSystem}'
linuxwhayutin@thinkdoe:
/OPENSHIFT/git/OADP/oadp-operatorget node -o jsonpath='{.items[0].status.nodeInfo.architecture}'ure}'
amd64whayutin@thinkdoe:~/OPENSHIFT/git/OADP/oadp-operator$

Comment thread Makefile
ifeq ($(shell $(ENVTEST) list | grep $(ENVTEST_K8S_VERSION)),)
# TODO what --arch=amd64 does here?
ENVTESTPATH = $(shell $(ENVTEST) --arch=amd64 use $(ENVTEST_K8S_VERSION) -p path)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In some past version there was an envtest_k8s_version value that would only support amd64 since there's no arm64 artifacts available to download.

Comment thread Makefile
# - podman machine ssh sudo rpm-ostree install qemu-user-static && sudo systemctl reboot
# from: https://github.com/containers/podman/issues/12144#issuecomment-955760527
# related enhancements that may remove the need to manually install qemu-user-static https://bugzilla.redhat.com/show_bug.cgi?id=2061584
DOCKER_BUILD_ARGS ?= --platform=linux/amd64
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

we could pass --platform=linux/amd64,linux/arm64 and have simplified logic, but builds take longer

so, I think is best to use the fastest solution for development environment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1

@mateusoliveira43 mateusoliveira43 marked this pull request as ready for review August 5, 2024 16:03
@openshift-ci openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 5, 2024
@openshift-ci openshift-ci Bot requested review from kaovilai and mrnold August 5, 2024 16:04
Comment thread Dockerfile
@@ -1,5 +1,7 @@
# Build the manager binary
FROM quay.io/konveyor/builder:ubi9-latest AS builder
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
FROM quay.io/konveyor/builder:ubi9-latest AS builder
FROM --platform=$BUILDPLATFORM quay.io/konveyor/builder:ubi9-latest AS builder
ARG BUILDPLATFORM

Comment thread Makefile
Comment on lines +40 to +41
CLUSTER_OS = $(shell $(OC_CLI) get node -o jsonpath='{.items[0].status.nodeInfo.operatingSystem}' 2> /dev/null)
CLUSTER_ARCH = $(shell $(OC_CLI) get node -o jsonpath='{.items[0].status.nodeInfo.architecture}' 2> /dev/null)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Followup

Suggested change
CLUSTER_OS = $(shell $(OC_CLI) get node -o jsonpath='{.items[0].status.nodeInfo.operatingSystem}' 2> /dev/null)
CLUSTER_ARCH = $(shell $(OC_CLI) get node -o jsonpath='{.items[0].status.nodeInfo.architecture}' 2> /dev/null)
CLUSTER_OS ?= $(shell $(OC_CLI) get node -o jsonpath='{.items[0].status.nodeInfo.operatingSystem}' 2> /dev/null)
CLUSTER_ARCH ?= $(shell $(OC_CLI) get node -o jsonpath='{.items[0].status.nodeInfo.architecture}' 2> /dev/null)

@kaovilai
Copy link
Copy Markdown
Member

kaovilai commented Aug 6, 2024

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Aug 6, 2024
@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented Aug 6, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kaovilai, mateusoliveira43, shubham-pampattiwar, weshayutin

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [kaovilai,mateusoliveira43,shubham-pampattiwar]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot
Copy link
Copy Markdown

/retest-required

Remaining retests: 0 against base HEAD 0a8b70f and 2 for PR HEAD 0fca95f in total

@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented Aug 7, 2024

@mateusoliveira43: all tests passed!

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

@openshift-merge-bot openshift-merge-bot Bot merged commit 0532206 into openshift:master Aug 7, 2024
@mateusoliveira43
Copy link
Copy Markdown
Author

/cherry-pick oadp-1.4

@openshift-cherrypick-robot
Copy link
Copy Markdown
Contributor

@mateusoliveira43: new pull request created: #1485

Details

In response to this:

/cherry-pick oadp-1.4

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants