Skip to content

Feat | Implement initdata for bare-metal/qemu for s390x #11640

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Aug 15, 2025
Merged

Feat | Implement initdata for bare-metal/qemu for s390x #11640

merged 2 commits into from
Aug 15, 2025

Conversation

rafsal-rahim
Copy link
Contributor

Opening this PR for enabling support for initdata in s390x Issue : 11628

@rafsal-rahim rafsal-rahim marked this pull request as draft July 31, 2025 06:44
@stevenhorsman
Copy link
Member

The commits need to be updated to match the https://github.com/kata-containers/community/blob/main/CONTRIBUTING.md#patch-format, but I've added some test enablement for s390x, so we can see if it works

@stevenhorsman
Copy link
Member

@rafsal-rahim - the code isn't valid:

virtcontainers/qemu_amd64.go:162:9: cannot use q (variable of type *qemuAmd64) as qemuArch value in return statement: *qemuAmd64 does not implement qemuArch (missing method buildInitdataDevice)

can you fix this and ensure it compiles locally before re-pushing please? Thanks

@stevenhorsman
Copy link
Member

Still failing to build:

virtcontainers/qemu_test.go:59:11: cannot use &qemuArchBase{} (value of type *qemuArchBase) as qemuArch value in struct literal: *qemuArchBase does not implement qemuArch (missing method buildInitdataDevice)
virtcontainers/qemu_test.go:269:9: cannot use &qemuArchBase{} (value of type *qemuArchBase) as qemuArch value in struct literal: *qemuArchBase does not implement qemuArch (missing method buildInitdataDevice)
virtcontainers/qemu_test.go:295:9: cannot use &qemuArchBase{} (value of type *qemuArchBase) as qemuArch value in struct literal: *qemuArchBase does not implement qemuArch (missing method buildInitdataDevice)
virtcontainers/qemu_test.go:343:9: cannot use &qemuArchBase{} (value of type *qemuArchBase) as qemuArch value in struct literal: *qemuArchBase does not implement qemuArch (missing method buildInitdataDevice)
virtcontainers/qemu_test.go:478:9: cannot use &qemuArchBase{} (value of type *qemuArchBase) as qemuArch value in struct literal: *qemuArchBase does not implement qemuArch (missing method buildInitdataDevice)
virtcontainers/qemu_test.go:507:11: cannot use qkvm (variable of type *qemuArchBase) as qemuArch value in struct literal: *qemuArchBase does not implement qemuArch (missing method buildInitdataDevice)

@stevenhorsman
Copy link
Member

Nice - the initdata tests are passing on the zvsi: https://github.com/kata-containers/kata-containers/actions/runs/16666552296/job/47174979611?pr=11640. @rafsal-rahim - can you please fix up your commits as requested to we can move this forward

@rafsal-rahim
Copy link
Contributor Author

Sure steve

@BbolroC
Copy link
Member

BbolroC commented Aug 12, 2025 •
edited
Loading

Test that creating a container from an rejected image configured by initdata, fails according to policy reject

  • passed

Test that creating a container from an rejected image not configured by initdata, fails according to CDH error

  • failed (time-out) passed

I will look into it tomorrow.

To be honest, I stopped the 2nd test because RunContainerError and CrashLoopBackOff took turns persisting for over 90 secs (with 4 restarts) and the test wasn't finished. So I was thinking it will be timed out and fail. But when I let the test running today, it ended up with success.

I've check if this behavior also appears for qemu-coco-dev and confirmed the similar pattern was shown, but the test passed after 120 secs. I would like to figure out why the 2nd test persists that long.

The whole tests passed. 👍

@stevenhorsman
Copy link
Member

I've check if this behavior also appears for qemu-coco-dev and confirmed the similar pattern was shown, but the test passed after 120 secs. I would like to figure out why the 2nd test persists that long.

Is it that the create container timeout is 120s, so Kubernetes doesn't give up on it until that point?

@BbolroC
Copy link
Member

BbolroC commented Aug 13, 2025 •
edited
Loading

I've check if this behavior also appears for qemu-coco-dev and confirmed the similar pattern was shown, but the test passed after 120 secs. I would like to figure out why the 2nd test persists that long.

Is it that the create container timeout is 120s, so Kubernetes doesn't give up on it until that point?

sounds reasonable. I will check that out. Thanks for feedback.

yeah, Get Resource failed appeared at 7 secs, but the test wasn't finished.

This is an expected behavior. The 2nd test uses k8s_create_pod() instead of assert_pod_fail() used in the 1st one. The function waits until a pod reaches a condition ready with a threshold:

kata-containers/tests/integration/kubernetes/lib.sh

Line 85 in 9379a18

local wait_time="${2:-120}"
The 2nd test expects that a pod creation should keep failing after the wait time and tries to catch a given log message from journal.

I will start reviewing the code. Thanks!

BbolroC
BbolroC reviewed Aug 13, 2025
View reviewed changes
Copy link
Member

@BbolroC BbolroC left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. I've left something to discuss.

@BbolroC
Copy link
Member

BbolroC commented Aug 13, 2025 •
edited
Loading

Hold on! I am not seeing virtio-blk-ccw from the QEMU command line during the test.

My bad, I've experienced the phenomenon from the 2nd test which does not use initdata. I've seen the device from the 1st test. Sorry.

  • Command line from the 1st
/opt/kata/bin/qemu-system-s390x -name sandbox-99c8da8558b22fec62abcd77e19b21676e0cec169fba85fa10ce0b56ed0774b3,debug-threads=on -uuid eccfeccf-c493-4bfd-944d-77676fd11ce2 -machine s390-ccw-virtio,accel=kvm,confidential-guest-support=pv0 -cpu host, -qmp unix:fd=3,server=on,wait=off -m 2048M,slots=10,maxmem=90260M -device virtio-serial-ccw,id=serial0,devno=fe.0.0001 -device virtconsole,chardev=charconsole0,id=console0 -chardev socket,id=charconsole0,path=/run/vc/vm/99c8da8558b22fec62abcd77e19b21676e0cec169fba85fa10ce0b56ed0774b3/console.sock,server=on,wait=off -device virtio-scsi-ccw,id=scsi0,devno=fe.0.0002 
-device virtio-blk-ccw,drive=initdata,config-wce=off,devno=fe.0.0003,serial=initdata 
-drive id=initdata,file=/run/kata-containers/shared/initdata/99c8da8558b22fec62abcd77e19b21676e0cec169fba85fa10ce0b56ed0774b3/data.img,aio=threads,format=raw,if=none 
-object s390-pv-guest,id=pv0 -device vhost-vsock-ccw,vhostfd=4,id=vsock-941369764,guest-cid=941369764,devno=fe.0.0004 -netdev tap,id=network-0,fds=5 -device driver=virtio-net-ccw,netdev=network-0,mac=26:21:71:53:c9:27,mq=on,devno=fe.0.0005 -rtc base=utc,driftfix=slew,clock=host -global kvm-pit.lost_tick_policy=discard -vga none -no-user-config -nodefaults -nographic --no-reboot -object memory-backend-ram,id=dimm1,size=2048M -machine memory-backend=dimm1 -kernel /opt/kata/share/kata-containers/kata-containers-se.img -append debug panic=1 nr_cpus=4 selinux=0 scsi_mod.scan=none agent.log=debug agent.debug_console agent.debug_console_vport=1026 cgroup_no_v1=all systemd.unified_cgroup_hierarchy=1 agent.log=debug initcall_debug agent.image_policy_file=kbs:///default/security-policy/test agent.enable_signature_verification=true -pidfile /run/vc/vm/99c8da8558b22fec62abcd77e19b21676e0cec169fba85fa10ce0b56ed0774b3/pid -smp 1,cores=1,threads=1,sockets=4,maxcpus=4

rafsal-rahim and others added 2 commits August 14, 2025 17:10
@rafsal-rahim
runtime: Add initdata support to s390x
9891b11 
- Added support for initdata device on s390x.
- Generalized devno generation for QEMU CCW devices.

Signed-off-by: rafsalrahim <rafsal.rahim@ibm.com>
@stevenhorsman @rafsal-rahim
test/k8s: Extend initdata tests to run on s390x
43cdde4 
Enable testing of initdata on the qemu-coco-dev and qemu-se
runtime classes, so we can validate the function on s390x

Signed-off-by: stevenhorsman <steven@uk.ibm.com>
@BbolroC
Copy link
Member

BbolroC commented Aug 14, 2025

For reviewers, the e2e test for the last push has passed successfully for IBM SEL: https://github.com/kata-containers/kata-containers/actions/runs/16956314713/job/48094463961#step:3:8573

BbolroC
BbolroC approved these changes Aug 14, 2025
View reviewed changes
Copy link
Member

@BbolroC BbolroC left a comment

Choose a reason for hiding this comment

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

LGTM thanks @rafsal-rahim !

Apokleos
Apokleos approved these changes Aug 15, 2025
View reviewed changes
Copy link
Contributor

@Apokleos Apokleos left a comment

Choose a reason for hiding this comment

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

Thx @rafsal-rahim LGTM. BTW, could you please add support for this feature in runtime-rs ?

@stevenhorsman
Copy link
Member

Thx @rafsal-rahim LGTM. BTW, could you please add support for this feature in runtime-rs ?

Yes - let's ensure that the issue isn't closed and is reworked to reflect the runtime-rs update

@BbolroC BbolroC merged commit 28bd0cf into kata-containers:main Aug 15, 2025
464 of 539 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stevenhorsman stevenhorsman stevenhorsman left review comments

@BbolroC BbolroC BbolroC approved these changes

@Apokleos Apokleos Apokleos approved these changes

Assignees
No one assigned
Labels
ok-to-test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@rafsal-rahim @stevenhorsman @BbolroC @Apokleos