Added suport for building multi-arch images #3314
Conversation
tekton-robot
added
do-not-merge/work-in-progress
|
|
tekton-robot
added
the
size/XS
|
Hi @AverageMarcus. Thanks for your PR. I'm waiting for a tektoncd member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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/test-infra repository. |
tekton-robot
added
the
needs-ok-to-test
AverageMarcus
force-pushed
the
multi_arch
branch
from
579a85e
to
c9a0520
Compare
tekton/publish.yaml
Outdated
| @@ -150,11 +150,11 @@ spec: | |||
| OUTPUT_BUCKET_RELEASE_DIR="/workspace/output/bucket/previous/$(inputs.params.versionTag)" | |||
|
|
|||
| # Publish images and create release.yaml | |||
| ko resolve --preserve-import-paths -t $(inputs.params.versionTag) -f /workspace/go/src/github.com/tektoncd/pipeline/config/ > $OUTPUT_BUCKET_RELEASE_DIR/release.yaml | |||
| ko resolve --platform=all --preserve-import-paths -t $(inputs.params.versionTag) -f /workspace/go/src/github.com/tektoncd/pipeline/config/ > $OUTPUT_BUCKET_RELEASE_DIR/release.yaml | |||
There was a problem hiding this comment.
For this to work we'll need to make sure the ko image we use is updated with a recent enough ko (> version 0.6.0).
That's configured here, in the plumbing repo: https://github.com/tektoncd/plumbing/blob/master/tekton/images/ko/Dockerfile#L18
And after we make that change we'll need to rebuild and publish the image, so it can be used here.
Do you want to make the plumbing change? If not, I can to unblock you.
|
We might also want to change the Makefile: Line 73 in 5005e81 And test/e2e-common.sh: Line 23 in 5005e81 And DEVELOPMENT.md: |
|
I've been speaking with @barthy1 on Slack and there needs to be changes made to the base images first for this to work. I'm going to see what I can do with those too. Hopefully I can get the E2E running against my ARM cluster but will leave this PR in draft until I have an update :) |
|
@imjasonh Regarding updating the Makefile - I wasn't sure about this. I'd estimate that in most instances when people are running these task they're only interested in targeting a single architecture (usually the one it's running on but not always) and building the multi-arch images can drastically slow things down. I wonder if introducing an env var to control the target architecture would be better? |
Yeah that's fair, let's skip that one. |
|
Update: It turns out |
Yeah, I'd warn against going too deep on distroless if you can avoid it for now. 😆 The discussion in tektoncd/community#211 is (I think) settling on a distinction between "available architectures" (arm64 and amd64, with s390x coming soon with changes in distroless 🤞 ), and "supported architectures" (currently just amd64, with s390x coming soon, see tektoncd/community#212) with automated tests on clusters with those architectures. This change makes it so we can make arm64 images "available", but I don't think we need to block anything on having it be "supported", since that's a bigger commitment. This PR is still blocked on the ko image change in plumbing, and a rebuild/release of that image AFAIK. cc @barthy1 |
yep, working on that one. Latest |
tekton-robot
added
needs-rebase
|
I've just tested this change on an Arm64 server and can confirm the image builds and deploys correctly. |
|
Distroless added s390x recently, and the ko image has been updated to 0.6.0, is there anything else we need to finish this work? |
|
/kind feature |
tekton-robot
added
the
kind/feature
|
@imjasonh we also need to build |
|
That base image should only be necessary if using git-init or creds-init. We can still merge this PR to get multiarch support for controller components and the entrypoint binary in the meantime. When build-base gets multiarch we'll pick that up automatically with this PR. |
|
I agree with you that merging this PR can provid already some Tekton components for multi-arch usage, which is great. |
Yeah that's a good point. While we have partial multi-arch support, we could simply not document multi-arch support, and expect users to continue to assume/require amd64, since that's all that's worked historically. We don't currently make any mention of CPU architecture support anywhere afaik, so maybe the first step is deciding where to document that, then adding to that, "...and some arm64 and s390x support, work in progress, you might see weird failures 😄 " |
|
/lgtm cancel |
|
For some reason, prow doesn't pick up that the branch has conflict… and it blocks the merge queue 😓 |
|
(wait.. and now the message from |
|
/lgtm |
|
/lgtm cancel |
|
It does need a rebase, it conflicts with alpha -> beta changes, so |
This avoids JSON marshaling the same object every time a PipelineRun is cancelled, and marshals the object once at startup. If for some reason marshaling that object fails (which it should never do), the controller will exit and restart. Failure to marshal this object should only be caused by a bug in our code, since the object isn't derived from anything related to user requests. If we have a bug in our cancel patch generation, we'd rather find out by having the controller crashloop than find out by having PipelineRuns be uncancellable, which might be harder to detect.
We've just added support for optional workspaces but I forgot to include an example showing how to use them as part of a When Expression. This commit adds a PipelineRun example YAML showing use of a workspaces.<name>.bound variable in a when expression. I've also updated pipelines.md to mention that you can use a when expression to evaluate whether an optional workspace was bound or not and to provide a short example snippet of using the bound variables in a when expression.
This extracts the migration to v1beta1 part from tektoncd#2897. Release tasks and pipelines from the `tekton/` folder are migrated to use v1beta1 APIs. Signed-off-by: Vincent Demeester <[email protected]>
Introducing PipelineRunFacts as a combination of PipelineRunState, DAG Graph, and finally Graph. This simplifies function definitions without having to pass graphs to PipelineRunState attributes.
…re not configurable when launching the Tekton controller. This meant that Tekton deployments could not be tuned to the environments it was deployed in. This commit makes DefaultThreadsPerController, QPS, and Burst configurable via flags. This helps tune a deployment depending on expected concurrency in production.
A while ago we added Script block support to Sidecars but forgot to perform variable replacement on them. We perform variable replacement on pretty much every other important field in a Sidecar so we should probably also do it in Scripts. This commit applies variables used in Sidecar Script blocks and adds a few tests for it (copied from the existing step_replacements func).
The taskrun controller now emits a warning event if the affinity specified by pod template will be overwritten with affinity assistant. Closes tektoncd#2678
Prior to this commit the volumes that were generated for workspace bindings did not have names that matched the values injected through workspaces.<name>.volume variables. This occurred because we called the GetVolumes workspace function twice during TaskRun Pod initialization and each execution generated different random names. This commit updates the TaskRun reconciler to generate the random volume names only once and then pass those generated volumes to the two functions that need them. Additionally the function generating those volumes is renamed to make clear that volume creation is its purpose. A e2e test is added to ensure that the workspace volume and variable value matches.
AverageMarcus
force-pushed
the
multi_arch
branch
from
a709249
to
88f0506
Compare
tekton-robot
added
the
needs-rebase
|
@AverageMarcus: 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/test-infra repository. |
tekton-robot
added
size/XL
|
@AverageMarcus: The following tests failed, say
Full PR test history. Your PR dashboard. 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/test-infra repository. I understand the commands that are listed here. |
|
I somehow managed to make a mess of the rebase it seems. Not quite sure what went wrong but I think I'll just open a new PR with the correct changes. |
|
Please see: #3451 |
Changes
Partial work towards #856
Adds support for building images for other architectures.
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
See the contribution guide for more details.
Double check this list of stuff that's easy to miss:
cmddir, please updatethe release Task to build and release this image.
Reviewer Notes
If API changes are included, additive changes must be approved by at least two OWNERS and backwards incompatible changes must be approved by more than 50% of the OWNERS, and they must first be added in a backwards compatible way.
Release Notes