ROX-20757 scanner multi arch builds

[Stringy]

Builds on #1573, adding multi-arch builds (s390x, ppc64le, arm64) to scanner pipeline

[openshift-ci]

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

[tommartensen]

I see the pipeline passed, what other validation can we have?
Run the images on IBM arch VMs?

[msugakov]

In case of scanner and most other pipelines (except for collector), we have apply-tags task to satisfy the init's task image-url parameter. (We could have skipped that altogether because init does not do much and does even less in our case, but we wanted to stay close to the pipeline template and remain future-proof.)

We don't need per-arch konflux-$(params.revision)-amd64 tags. The only one we need is konflux-$(params.revision) which is naturally placed on the manifest image which you get in build-image-manifest.

Would it work if you delete build-image-manifest-konflux, delete apply-tags-<arch> tasks, and add only one apply-tags that works on a result of build-image-manifest?

[Molter73]

I've tried this, apply-tags will not retag the manifest, it will only retag the image for the architecture that matches the host running the pipeline (amd64 in this case). Alternatively, we could keep the build-image-manifest-konflux step, but have it use the stackrox images instead of applying the tags, WDYT?

[msugakov]

This is a weird limitation of apply-tags. Can you show me a pipelinerun please?

The only reason we have apply-tags in all pipelines but Collector is to satisfy the init task.

For satisfying the init task it should be sufficient to produce an image with a tag that's derived from information available at the pipeline level, for which we use git commit sha - $(params.output-image-repo):konflux-$(params.revision). Since the intent of init is to check if the image was previously built, we could either give amd64 one to it (i.e. keep only apply-tags for amd64 and tag the image as konflux-$(params.revision), no -amd64 suffix) or create a new manifest tagged as konflux-$(params.revision).

I think, that manifest creation is a cheap operation and it just feels cleaner (e.g. we could succeed building amd64 but fail P or Z) to check presence of builds for all arches. Therefore, I would suggest to try remove all apply-tags*, keep build-image-manifest-konflux but feed it from the same images as build-image-manifest.
Effectively, use build-image-manifest-konflux as a way to add tags.

Would this work?

For Collector, we not only create konflux-$(params.revision) tag for init but we also add $(tasks.determine-image-tag.results.IMAGE_TAG)-latest and $(tasks.determine-image-tag.results.IMAGE_TAG)-slim for StackRox deployment to succeed because this matches the convention. If tagging via build-image-manifest* works, these will be two extra tasks in the pipeline producing manifests tagged with -slim and -latest.

[Molter73]

Can you show me a pipelinerun please?

This was about 20 commits ago, doubt I can find the exact run but will try to run it again.

I agree that generating a proper manifest is cleaner than simply retagging a single arch.

[Molter73]

@msugakov, sorry for the delay, here is a pipeline for scanner-v4-db where I used apply tags on the manifest: https://console.redhat.com/preview/application-pipeline/workspaces/rh-acs/applications/acs/pipelineruns/scanner-v4-db-build-468sx

If you check in quay.io, you'll see the konflux image sha matches with the amd64 image and not the manifest. This is the same behaviour you'll get from doing podman/docker tag <MANIFEST> <IMAGE>, so I believe this is by design. I'll try creating a manifest from the stackrox images next, see if that yields the result we want.

[msugakov]

There's podman manifest inspect and called for both in scanner-v4-db-build-jzrb4, I get identical results.

$ diff -s <(podman manifest inspect quay.io/rhacs-eng/scanner-v4-db:4.6.x-107-g70adab264b-fast) <(podman manifest inspect quay.io/rhacs-eng/scanner-v4-db:konflux-70adab264b664d3f78ab3a84086bb4dbabb12963)
Files /proc/self/fd/11 and /proc/self/fd/15 are identical

Means we can use manifest generation as retagging, right?

[Stringy]

Would this mean we'd need separate manifest generation tasks in Collector for -latest and -slim?

[Stringy]

Actually I've just realised I can use a matrix for that. Ignore me 😄

[msugakov]

Yes, that's another matrix. The first one iterates over architectures, the second one iterates over manifest tags (yet each manifest includes image from each arch).

[Molter73]

Means we can use manifest generation as retagging, right?

That's my understanding, yes, I have that setup on the scanner-v4 and basic-component PRs already.

[Stringy]

Changes in this file are due to limitation with tekton matrices:

• "normal" params can't reference matrix params
• matrix params can't be arrays
• BUILD_ARGS must be an array and the tag variable needs to have the arch suffix in order to differentiate between multi-arch images

As a result, I've used the TARGETARCH variable within the Dockerfile to construct the tag here instead. I've done the same for main (stackrox/stackrox#12054) and collector (stackrox/collector#1751)

@tommartensen @msugakov I'd be keen to hear your thoughts on this workaround

[msugakov]

Commented in-place and also I wonder why did you decide this may be needed?

[msugakov]

Oh, it looks like there's a usability problem with matrix tasks.

https://console.redhat.com/preview/application-pipeline/workspaces/rh-acs/applications/acs/pipelineruns/scanner-build-hmj57 shows logs for just one run from the matrix (seems to be the first one defined, s390x) and does not allow to switch to other runs. I'm going to report this.

Have you seen one of matrix builds failing, would logs show the failing build or they always show just the first one?

[Stringy]

From what I've seen it's inconsistent in its reporting, and seemingly randomly picks a run from the list. I've only observed this while the task is running, however.

[msugakov]

Reported here https://redhat-internal.slack.com/archives/C04PZ7H0VA8/p1721997539610399

[msugakov]

@Stringy @Molter73 bad news: the recommendation from the Konflux team is not to use matrix for now. See https://redhat-internal.slack.com/archives/C04PZ7H0VA8/p1721997731935729?thread_ts=1717081428.417939&cid=C04PZ7H0VA8

I'm very sorry for me nudging you in that direction and your effort spent on it.

[msugakov]

Commented in-place and also I wonder why did you decide this may be needed?

[msugakov]

EC feels unhappy but I think not this change might be causing it.
Green light from me after you address these last findings.

[msugakov]

@tommartensen Could you PTAL?

[Stringy]

/retest

[Stringy]

/retest