Adding multi-arch matrix pipeline #1236

arewm · 2024-08-02T02:30:55Z

This is a draft PR for a multi-arch pipeline with matrix builds. It cannot be merged yet as there are still a couple of outstanding issues with running a matrix build including:

Multi-platform support for running workloads on the local cluster: feat(KFLUXINFRA-741): PROD: MPC w/ "local" support redhat-appstudio/infra-deployments#4328
Local platforms added for running workloads: chore(KFLUXINFRA-743) Prod local platforms redhat-appstudio/infra-deployments#4329
Support in the buildah task for localhosts to be run in cluster instead of remotely: Enable remote tasks to be run in cluster #1216
Automatically appending an architecture suffix to the image tags: Add an architecture suffix to images pushed for multi-platform if missing #1185
Support for referencing single-element matrix results by array: SRVKP-5882

Before you complete this pull request ...

Look for any open pull requests in the repository with the title "e2e-tests update" and
see if there are recent e2e-tests updates that will be applicable to your change.

zregvart

I like this

pipelines/docker-build-multi-arch-oci-ta/README.md

pipelines/docker-build-multi-arch-oci-ta/kustomization.yaml

zregvart · 2024-08-02T10:52:05Z

pipelines/docker-build-multi-arch-oci-ta/patch.yaml

+# add matrix to run based on build-platforms
+# change build-image-index to use results from build-containers as well as run after it
+# change build-image-index to build image index
+# change JAVA_COMMUNITY_DEPENDENCIES value to "$(tasks.build-containers.results.JAVA_COMMUNITY_DEPENDENCIES[0])"


I guess we don't care from which image build the JAVA_COMMUNITY_DEPENDENCIES resulted, all architectures will generate the same?

This has been the pattern that we have maintained when exploring multi-arch pipelines. It can be revisited when the java build pipeline is reassessed for multi-arch support.

pipelines/docker-build-multi-arch-oci-ta/patch.yaml

zregvart · 2024-08-02T10:52:54Z

pipelines/docker-build-multi-arch-oci-ta/patch.yaml

+  value: build-containers
+- op: replace
+  path: /spec/tasks/3/taskRef/name
+  value: buildah-remote-oci-ta


We build everything remotely, pragmatic approach...

Well, we use the same task for everything. The intention behind some of the linked issues in the beginning is to allow these to be a "pass through" to an in-cluster build without changing the task used.

pipelines/docker-build-multi-arch-oci-ta/patch.yaml

pipelines/docker-build-oci-ta/patch.yaml

pipelines/docker-build-rhtap/patch.yaml

arewm · 2024-08-02T17:55:13Z

/retest build-definitions-pull-request

ifireball · 2024-08-04T06:30:26Z

Is this an implementation of STONEBLD-2567?

arewm · 2024-08-05T11:22:28Z

@ifireball, yes. As is #1226.

simonbaird · 2024-08-05T13:57:28Z

IIUC, this is semi-blocked until tektoncd/chains#1164 becomes generally available within Konflux. Is that accurate?

simonbaird · 2024-08-05T13:57:44Z

See also #1226 which may have some overlap.

arewm · 2024-08-05T14:31:57Z

@simonbaird, This is not blocked on the Chains bug fix. I have worked around that with #1204

simonbaird · 2024-08-05T15:42:35Z

@simonbaird, This is not blocked on the Chains bug fix. I have worked around that with #1204

Ack, so I think we're likely to abandon #1226, which (AIUI) tries to implement a multi-arch pipeline without matrix support. Do you agree @joejstuart ?

openshift-ci

@arewm: 1 invalid OWNERS file

In response to this:

This is a draft PR for a multi-arch pipeline with matrix builds. It cannot be merged yet as there are still a couple of outstanding issues with running a matrix build including:

Multi-platform support for running workloads on the local cluster: feat(KFLUXINFRA-741): PROD: MPC w/ "local" support redhat-appstudio/infra-deployments#4328 chore(KFLUXINFRA-743) Prod local platforms redhat-appstudio/infra-deployments#4329

Support in the buildah task for localhosts to be run in cluster instead of remotely: Enable remote tasks to be run in cluster #1216

Automatically appending an architecture suffix to the image tags: Add an architecture suffix to images pushed for multi-platform if missing #1185

Support for referencing single-element matrix results by array: SRVKP-5882

Before you complete this pull request ...

Look for any open pull requests in the repository with the title "e2e-tests update" and
see if there are recent e2e-tests updates that will be applicable to your change.

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.

task/build-image-index/OWNERS

arewm · 2024-08-20T17:27:23Z

/retest

chmeliik · 2024-08-21T07:35:40Z

task/buildah/0.2/buildah.yaml

@@ -107,7 +107,7 @@ spec:
  results:
  - description: Digest of the image just built
    name: IMAGE_DIGEST
-  - description: Image repository where the built image was pushed
+  - description: Image repository and tag where the built image was pushed


The "and tag" part is technically up to the user; if they use a tag-less image as IMAGE, then IMAGE_URL won't have a tag either.

But this should be rare if not non-existent in practice

If no tag is set than an implicit tag of latest will be used.

Used yes, but not returned in IMAGE_URL

chmeliik · 2024-08-21T08:56:36Z

task/build-image-index/0.1/build-image-index.yaml

-        if [[ $(echo $i | tr -cd ":" | wc -c) == 2 ]]; then
+        TOADD_REPOSITORY="$(echo "$i" | cut -d@ -f1)"
+        TOADD_DIGEST="$(echo "$i" | cut -d@ -f2)"
+        TOADD_TAG="$(echo "$IMAGE" | cut -d: -f2)"


Shouldn't we be extracting the tag from $i rather than $IMAGE? Do we want to ignore the actual tag in IMAGES for some reason?

Note: this code wouldn't work as is, the tag extraction doesn't handle digests (and port numbers in the registry host, but that's a problem across almost every task in the pipeline)

I added this tag logic because the e2e tests were failing since no tag was getting set. After looking at the pipeline definitions again, this is because I was passing the IMAGE_REF from the container build instead of IMAGE_URL. The result of this is that the IMAGE_URL that was returned ended up stripping off the tag.

For a matrix build, we need to pass IMAGE_REF, but for non-matrix builds, however, $(tasks.build-container.results.IMAGE_URL)@$(tasks.build-container.results.IMAGE_DIGEST) should be used to define the input IMAGES to allow the pass-through functionality.

I'm not sure I understood that, but it would be great if the task worked in general, not only for some possible configurations of a pipeline

chmeliik

I think this works, though the build-image-index code is a bit mind-bending in its handling of tags

openshift-merge-robot · 2024-08-21T12:30:59Z

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.

* Use of Image Index * Inclusion of image tag in IMAGE_URL result Signed-off-by: arewm <[email protected]>

* Created a new task that has an appropriate name (build-image-index) * Changed the build-image-manifest task to be a rebuild of the new build-image-index task * Enabled the image index task to run and skip the generation of an index image. This lets the task be added into "single arch" pipelines as well as there are still situations where an image index should be created Signed-off-by: arewm <[email protected]>

The build-image-index task is added to all pipelines but the generation of an image index is disabled by default. Signed-off-by: arewm <[email protected]>

Signed-off-by: arewm <[email protected]>

With konflux-ci/build-definitions#1236, we started building a multi-arch pipeline. This change adds it to the config map so that it can be used for configuring components. Signed-off-by: arewm <[email protected]>

chmeliik mentioned this pull request Aug 2, 2024

Adding trusted artifacts multi arch docker build pipeline #1226

Closed

zregvart approved these changes Aug 2, 2024

View reviewed changes

arewm force-pushed the multi-arch-pipeline branch from 5878a49 to 3345b3b Compare August 2, 2024 15:23

arewm commented Aug 2, 2024

View reviewed changes

pipelines/docker-build-rhtap/patch.yaml Show resolved Hide resolved

arewm force-pushed the multi-arch-pipeline branch from 3345b3b to 6fb9c01 Compare August 2, 2024 15:36

arewm force-pushed the multi-arch-pipeline branch 2 times, most recently from e3a97f4 to b86405f Compare August 2, 2024 18:04

openshift-merge-robot added the needs-rebase label Aug 5, 2024

arewm mentioned this pull request Aug 14, 2024

buildah-remote task: concurrency problems with source workspace #1275

Open

arewm force-pushed the multi-arch-pipeline branch from b86405f to 84e3df9 Compare August 14, 2024 18:17

openshift-ci bot added the do-not-merge/invalid-owners-file label Aug 14, 2024

openshift-ci bot reviewed Aug 14, 2024

View reviewed changes

task/build-image-index/OWNERS Outdated Show resolved Hide resolved

openshift-merge-robot removed the needs-rebase label Aug 14, 2024

arewm force-pushed the multi-arch-pipeline branch from 84e3df9 to b7e6e9e Compare August 14, 2024 18:20

openshift-ci bot removed the do-not-merge/invalid-owners-file label Aug 14, 2024

arewm force-pushed the multi-arch-pipeline branch 3 times, most recently from fdbe24f to de741da Compare August 15, 2024 15:05

arewm changed the title ~~DO NOT MERGE: Adding multi-arch pipeline~~ Adding multi-arch matrix pipeline Aug 15, 2024

arewm marked this pull request as ready for review August 15, 2024 15:05

openshift-ci bot requested review from chmeliik and Josh-Everett August 15, 2024 15:05

arewm requested a review from a team August 15, 2024 15:05

arewm force-pushed the multi-arch-pipeline branch 3 times, most recently from 59a8c06 to 089a8a1 Compare August 20, 2024 17:24

chmeliik reviewed Aug 21, 2024

View reviewed changes

chmeliik approved these changes Aug 21, 2024

View reviewed changes

arewm force-pushed the multi-arch-pipeline branch from 089a8a1 to eb17d6d Compare August 21, 2024 12:30

openshift-merge-robot added the needs-rebase label Aug 21, 2024

arewm force-pushed the multi-arch-pipeline branch from eb17d6d to 17c8fff Compare August 21, 2024 13:41

arewm added 3 commits August 21, 2024 09:54

unify on language

83e61a9

* Use of Image Index * Inclusion of image tag in IMAGE_URL result Signed-off-by: arewm <[email protected]>

Add image index task to pipelines

47cb196

The build-image-index task is added to all pipelines but the generation of an image index is disabled by default. Signed-off-by: arewm <[email protected]>

arewm force-pushed the multi-arch-pipeline branch from 17c8fff to b8615a5 Compare August 21, 2024 13:54

chmeliik approved these changes Aug 21, 2024

View reviewed changes

arewm enabled auto-merge August 21, 2024 13:59

Add a matrixed multi-arch pipeline

462f97e

Signed-off-by: arewm <[email protected]>

arewm force-pushed the multi-arch-pipeline branch from b8615a5 to 462f97e Compare August 21, 2024 14:05

arewm added this pull request to the merge queue Aug 21, 2024

Merged via the queue into konflux-ci:main with commit c251fc3 Aug 21, 2024
13 checks passed

arewm deleted the multi-arch-pipeline branch August 21, 2024 15:07

arewm mentioned this pull request Aug 21, 2024

Add the multi-arch pipeline to the pipeline config redhat-appstudio/infra-deployments#4379

Merged

arewm mentioned this pull request Aug 21, 2024

Add the multi-arch pipeline to the pipeline config openshift/hac-dev#977

Merged

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding multi-arch matrix pipeline #1236

Adding multi-arch matrix pipeline #1236

arewm commented Aug 2, 2024 •

edited

Loading

zregvart left a comment

zregvart Aug 2, 2024

arewm Aug 2, 2024

zregvart Aug 2, 2024

arewm Aug 2, 2024

arewm commented Aug 2, 2024

ifireball commented Aug 4, 2024

arewm commented Aug 5, 2024

simonbaird commented Aug 5, 2024

simonbaird commented Aug 5, 2024

arewm commented Aug 5, 2024

simonbaird commented Aug 5, 2024

openshift-ci bot left a comment

arewm commented Aug 20, 2024

chmeliik Aug 21, 2024

arewm Aug 21, 2024

chmeliik Aug 21, 2024

chmeliik Aug 21, 2024

arewm Aug 21, 2024

chmeliik Aug 21, 2024

chmeliik left a comment

openshift-merge-robot commented Aug 21, 2024

Adding multi-arch matrix pipeline #1236

Adding multi-arch matrix pipeline #1236

Conversation

arewm commented Aug 2, 2024 • edited Loading

Before you complete this pull request ...

zregvart left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arewm commented Aug 2, 2024

ifireball commented Aug 4, 2024

arewm commented Aug 5, 2024

simonbaird commented Aug 5, 2024

simonbaird commented Aug 5, 2024

arewm commented Aug 5, 2024

simonbaird commented Aug 5, 2024

openshift-ci bot left a comment

Choose a reason for hiding this comment

Before you complete this pull request ...

arewm commented Aug 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chmeliik left a comment

Choose a reason for hiding this comment

openshift-merge-robot commented Aug 21, 2024

arewm commented Aug 2, 2024 •

edited

Loading