Add s390x for ubuntu and centos artifacts
What this PR does / why we need it: This pull request adds s390x architecture in Containerdisks for Ubuntu and CentOS artifacts . These changes ensure compatibility and resolve build issues encountered on the s390x architecture.
Release note:
Add s390x artifacts
Signed-off-by: Aditi Sharma [email protected]
Hi @aditi-sharma-1. Thanks for your PR.
PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.
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-sigs/prow repository.
Hi @lyarwood @0xFelix, thank you for your review and comments on the PR.
I co-work with @aditi-sharma-1 on the s390x efforts.
I take this opportunity to explain to you the importance and urgency of this PR and why we need it merged on the s390x side at the earliest.
The downstream tests (OCP virt - tier 0/1 tests), has a tight dependency expecting the container disks images for fedora, centos, ubuntu to be available from quay.io.
We are ticking on the timeline to unblock the downstream tests and to have a clean run next week.
kindly, is it possible to consider the urgency of the PR and a possibility to merge the changes with the confidence that we got the testing covered from our local s390x verification? (Aditi can give you the logs again, please refer https://github.com/kubevirt/containerdisks/pull/203#discussion_r1801649950). Meanwhile, we are also motivated to provide you a s390x CI environment but that would take some time.
so, is it possible to push the changes forward with so called label 'untested-on-CI' so we mark it now and re-visit later with motivation?
Also, for the related changes for s390x/fedora in this PR, we have covered the local s390x verification for it inclusively from the common-instance types. so, these changes are also good to get piggy backed.
Please let me know your thoughts. Thank you.
I have no problem adding the containerdisks, but I would ask you to remove the environment variables for instancetypes and preferences from them for now, as they don't exist yet.
Hi @lyarwood @0xFelix, thank you for your review and comments on the PR.
I co-work with @aditi-sharma-1 on the s390x efforts.
I take this opportunity to explain to you the importance and urgency of this PR and why we need it merged on the s390x side at the earliest.
The downstream tests (OCP virt - tier 0/1 tests), has a tight dependency expecting the container disks images for fedora, centos, ubuntu to be available from
quay.io. We are ticking on the timeline to unblock the downstream tests and to have a clean run next week.kindly, is it possible to consider the urgency of the PR and a possibility to merge the changes with the confidence that we got the testing covered from our local s390x verification? (Aditi can give you the logs again, please refer #203 (comment)). Meanwhile, we are also motivated to provide you a s390x CI environment but that would take some time.
so, is it possible to push the changes forward with so called label '
untested-on-CI' so we mark it now and re-visit later with motivation?Also, for the related changes for s390x/fedora in this PR, we have covered the local s390x verification for it inclusively from the common-instance types. so, these changes are also good to get piggy backed.
Please let me know your thoughts. Thank you.
Yeah appreciated, lets land this PR without the instance type and preference envs as suggested by @0xFelix and come back later once we have CI runners to ensure coverage. @aditi-sharma-1 can you update the PR and we will get it merged tomorrow?
Hi @0xFelix, I have added the unit tests for the CentOS and Ubuntu artifacts.
I am currently working on the Fedora artifact and will raise a separate PR for it soon. This PR focuses on the Ubuntu and CentOS artifacts.
I’ve also attached the logs for building and verification of the Ubuntu and CentOS artifacts on s390x for your reference.
Thank you!
/test all
/approve cancel
Can you please check the CI failures? They are related.
/retest-required
@aditi-sharma-1: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.
In response to this:
/retest-required
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.
/ok-to-test
@aditi-sharma-1: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.
In response to this:
/ok-to-test
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.
/test all
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: 0xFelix
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~OWNERS~~ [0xFelix]
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
/hold
time="2024-11-11T14:39:35Z" level=error msg="error introspecting image \"quay.io/containerdisks/ubuntu:24.04\": Error parsing manifest for image: choosing image instance: no image found in manifest list for architecture \"s390x\", variant \"\", OS \"linux\"" name=ubuntu version=24.04
time="2024-11-11T14:39:35Z" level=error msg="error introspecting image \"quay.io/containerdisks/ubuntu:22.04\": Error parsing manifest for image: choosing image instance: no image found in manifest list for architecture \"s390x\", variant \"\", OS \"linux\"" name=ubuntu version=22.04
I don't recall hitting this with the arm64 images? Looking now.
Reproduces locally FWIW....
$ make medius
CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -o bin/medius kubevirt.io/containerdisks/cmd/medius
$ bin/medius images push --target-registry=localhost:5000 --dry-run=false --insecure-skip-tls --focus=ubuntu:22.04
INFO[0005] Latest containerdisk checksum: "55c687a9a242fab7b0ec89ac69f9def77696c4e160e6f640879a0b0031a08318" name=ubuntu version=22.04
INFO[0008] Latest containerdisk checksum: "2edb369b85141fbeff6c87c7d92e08f315ec236f639235e35429e4930d98e2de" name=ubuntu version=22.04
ERRO[0009] error introspecting image "quay.io/containerdisks/ubuntu:22.04": Error parsing manifest for image: choosing image instance: no image found in manifest list for architecture "s390x", variant "", OS "linux" name=ubuntu version=22.04
INFO[0009] Writing results file
FATA[0009] error introspecting image "quay.io/containerdisks/ubuntu:22.04": Error parsing manifest for image: choosing image instance: no image found in manifest list for architecture "s390x", variant "", OS "linux"
The error is coming from github.com/containers/image/v5, quick and dirty workaround below works for me for now:
git diff
diff --git a/cmd/medius/images/push.go b/cmd/medius/images/push.go
index 36455d34..71a5f1ea 100644
--- a/cmd/medius/images/push.go
+++ b/cmd/medius/images/push.go
@@ -176,6 +176,8 @@ func (b *buildAndPublish) handleMetadataError(imageName string, err error) error
b.Log.Info("Tag does not yet exist, it will be created")
case repository.IsTagUnknownError(err):
b.Log.Info("Tag is gone but seems to have existed already, it will be created")
+ case repository.IsArchUnknownError(err):
+ b.Log.Info("Image with arch does not exist yet, it will be created")
default:
return fmt.Errorf("error introspecting image %q: %v", imageName, err)
}
diff --git a/pkg/repository/repository.go b/pkg/repository/repository.go
index 4bb19f99..1bd9795c 100644
--- a/pkg/repository/repository.go
+++ b/pkg/repository/repository.go
@@ -159,6 +159,10 @@ func IsTagUnknownError(err error) bool {
return false
}
+func IsArchUnknownError(err error) bool {
+ return strings.Contains(err.Error(), "no image found in manifest list for architecture")
+}
+
func getErrorCode(err error) errcode.ErrorCoder {
for {
if unwrapped := errors.Unwrap(err); unwrapped != nil {
$ make medius
CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -o bin/medius kubevirt.io/containerdisks/cmd/medius
$ bin/medius images push --target-registry=localhost:5000 --dry-run=false --insecure-skip-tls --focus=ubuntu:22.04
INFO[0005] Latest containerdisk checksum: "55c687a9a242fab7b0ec89ac69f9def77696c4e160e6f640879a0b0031a08318" name=ubuntu version=22.04
INFO[0010] Latest containerdisk checksum: "2edb369b85141fbeff6c87c7d92e08f315ec236f639235e35429e4930d98e2de" name=ubuntu version=22.04
INFO[0011] Image with arch does not exist yet, it will be created name=ubuntu version=22.04
INFO[0011] Rebuild needed, downloading "https://cloud-images.ubuntu.com/releases/22.04/release/ubuntu-22.04-server-cloudimg-amd64.img" ... name=ubuntu version=22.04
INFO[0017] Building containerdisk ... name=ubuntu version=22.04
INFO[0019] Rebuild needed, downloading "https://cloud-images.ubuntu.com/releases/22.04/release/ubuntu-22.04-server-cloudimg-arm64.img" ... name=ubuntu version=22.04
INFO[0024] Building containerdisk ... name=ubuntu version=22.04
INFO[0026] Rebuild needed, downloading "https://cloud-images.ubuntu.com/releases/22.04/release/ubuntu-22.04-server-cloudimg-s390x.img" ... name=ubuntu version=22.04
INFO[0031] Building containerdisk ... name=ubuntu version=22.04
INFO[0032] Pushing localhost:5000/ubuntu:22.04-2411111627 image index name=ubuntu version=22.04
INFO[0034] Pushing localhost:5000/ubuntu:22.04 image index name=ubuntu version=22.04
INFO[0034] Writing results file
$ podman search --list-tags --no-trunc localhost:5000/ubuntu
NAME TAG
localhost:5000/ubuntu 22.04-2411111627
localhost:5000/ubuntu 22.04
$ podman pull --arch s390x localhost:5000/ubuntu:22.04
Trying to pull localhost:5000/ubuntu:22.04...
Getting image source signatures
Copying blob sha256:e7cd03db0bb75f3780e80df734c614f2dea4e4e6a3a45e9315b379edd29a9238
Copying config sha256:ae7d5c8454fc402b2db723838bd94f2e25e84e92a9b6d12f0f921a775893a6e8
Writing manifest to image destination
ae7d5c8454fc402b2db723838bd94f2e25e84e92a9b6d12f0f921a775893a6e8
/hold cancel
/lgtm
Hi @0xFelix, @lyarwood
Thank you for reviewing and merging the PR. I really appreciate the feedback and collaboration, and look forward to contributing more!!
The error is coming from
github.com/containers/image/v5, quick and dirty workaround below works for me for now
@lyarwood Thats more of a bug on our side. I knew there was handling for unknown tags in medius (hence my comment), but I did not know that unknown archs have to be handled separately. Thanks for addressing it!
Thanks for you contribution @aditi-sharma-1! :)
The error is coming from
github.com/containers/image/v5, quick and dirty workaround below works for me for now@lyarwood Thats more of a bug on our side. I knew there was handling for unknown tags in medius (hence my comment), but I did not know that unknown archs have to be handled separately. Thanks for addressing it!
Yeah I think that error in github.com/containers/image/v5 is new however as I don't recall it being an issue with the arm64 disks, still good to get this landed finally.