Merge pull request #12784 from hgowda-amd/sev-snp-tests-required

Add sev-snp, qemu-snp CIs as required

docs/use-cases/NVIDIA-GPU-passthrough-and-Kata-QEMU.md

@@ -213,12 +213,10 @@ API and kernel drivers, interacting with the pass-through GPU device.
 An additional step is exercised in our CI samples: when using images from an
 authenticated registry, the guest-pull mechanism triggers attestation using
 Trustee's Key Broker Service (KBS) for secure release of the NGC API
-authentication key used to access the NVCR container registry. As part of
-this, the attestation agent exercises composite attestation and transitions
-the GPU into `Ready` state (without this, the GPU has to explicitly be
-transitioned into `Ready` state by passing the `nvrc.smi.srs=1` kernel
-parameter via the shim config, causing NVRC to transition the GPU into the
-`Ready` state).
+authentication key used to access the NVCR container registry.
+In this flow the CPU and all additional devices are attested.
+GPUs will automatically be set to ready by NVRC per the NVRC configuration
+flag in the default kernel command line.
 
 ## Deployment Guidance
 
@@ -465,8 +463,6 @@ kind: Pod
 metadata:
   name: cuda-vectoradd-kata
   namespace: default
-  annotations:
-    io.katacontainers.config.hypervisor.kernel_params: "nvrc.smi.srs=1"
 spec:
   runtimeClassName: ${GPU_RUNTIME_CLASS_NAME}
   restartPolicy: Never
@@ -613,12 +609,7 @@ You can author pod manifests leveraging your own containers, for instance,
 containers built using the CUDA container toolkit. We recommend to start
 with a CUDA base container.
 
-The GPU is transitioned into the `Ready` state via attestation, for instance,
-when pulling authenticated images. If your deployment scenario does not use
-attestation, please refer back to the CUDA vectorAdd pod manifest. In this
-manifest, we ensure that NVRC sets the GPU to `Ready` state by adding the
-following annotation in the manifest:
-`io.katacontainers.config.hypervisor.kernel_params: "nvrc.smi.srs=1"`
+When using the GPU runtime classes, the GPUs will automatically be set to ready.
 
 > **Notes:**
 >

src/runtime-rs/config/configuration-qemu-snp-runtime-rs.toml.in

@@ -16,8 +16,7 @@
 [hypervisor.qemu]
 path = "@QEMUPATH@"
 kernel = "@KERNELPATH_COCO@"
-initrd = "@INITRDCONFIDENTIALPATH@"
-# image = "@IMAGECONFIDENTIALPATH@"
+image = "@IMAGECONFIDENTIALPATH@"
 machine_type = "@MACHINETYPE@"
 
 # Enable confidential guest support.
@@ -99,6 +98,11 @@ valid_hypervisor_paths = @QEMUVALIDHYPERVISORPATHS@
 # container and look for 'default-kernel-parameters' log entries.
 kernel_params = "@KERNELPARAMS@"
 
+# Optional dm-verity parameters (comma-separated key=value list):
+# root_hash=...,salt=...,data_blocks=...,data_block_size=...,hash_block_size=...
+# These are used by the runtime to assemble dm-verity kernel params.
+kernel_verity_params = "@KERNELVERITYPARAMS@"
+
 # Path to the firmware.
 # If you want that qemu uses the default firmware leave this option empty
 firmware = "@FIRMWARE_SNP_PATH@"

src/runtime/config/configuration-qemu-snp.toml.in

@@ -15,7 +15,7 @@
 [hypervisor.qemu]
 path = "@QEMUPATH@"
 kernel = "@KERNELCONFIDENTIALPATH@"
-initrd = "@INITRDCONFIDENTIALPATH@"
+image = "@IMAGECONFIDENTIALPATH@"
 machine_type = "@MACHINETYPE@"
 
 # rootfs filesystem type:
@@ -91,6 +91,11 @@ snp_guest_policy = 196608
 # container and look for 'default-kernel-parameters' log entries.
 kernel_params = "@KERNELPARAMS@"
 
+# Optional dm-verity parameters (comma-separated key=value list):
+# root_hash=...,salt=...,data_blocks=...,data_block_size=...,hash_block_size=...
+# These are used by the runtime to assemble dm-verity kernel params.
+kernel_verity_params = "@KERNELVERITYPARAMS@"
+
 # Path to the firmware.
 # If you want that qemu uses the default firmware leave this option empty
 firmware = "@FIRMWARESNPPATH@"

tests/integration/kubernetes/k8s-measured-rootfs.bats

@@ -16,7 +16,7 @@ shim_config_file="/opt/kata/share/defaults/kata-containers/configuration-${KATA_
 
 check_and_skip() {
 	case "${KATA_HYPERVISOR}" in
-		qemu-tdx|qemu-coco-dev)
+		qemu-tdx|qemu-coco-dev|qemu-snp)
 			if [ "$(uname -m)" == "s390x" ]; then
 				skip "measured rootfs tests not implemented for s390x"
 			fi

tools/packaging/kata-deploy/Dockerfile

@@ -47,7 +47,8 @@ RUN \
 		libc6-dev \
 		musl-tools && \
 	apt-get clean && rm -rf /var/lib/apt/lists/ && \
-	curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y --default-toolchain ${RUST_TOOLCHAIN}
+	curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y --default-toolchain ${RUST_TOOLCHAIN} && \
+	rustup component add rustfmt clippy
 
 # Build from the repository root so kata-deploy uses the root Cargo workspace:
 #   docker build -f tools/packaging/kata-deploy/Dockerfile .
@@ -91,6 +92,17 @@ RUN \
 	esac; \
 	echo "${rust_target}" > /tmp/rust_target
 
+# Verify code formatting and run cargo check before tests and build
+RUN \
+	set -e && \
+	rust_target="$(cat /tmp/rust_target)" && \
+	echo "Checking code formatting..." && \
+	cargo fmt -p kata-deploy --check && \
+	echo "Code formatting check passed!" && \
+	echo "Running cargo clippy with target ${rust_target}..." && \
+	cargo clippy -p kata-deploy --all-targets --all-features --release --locked --target "${rust_target}" -- -D warnings && \
+	echo "Cargo clippy passed!"
+
 # Run tests using --test-threads=1 to prevent environment variable pollution between tests,
 # and this is fine as we'll never ever have multiple binaries running at the same time.
 RUN \

tools/packaging/kata-deploy/binary/src/artifacts/install.rs

@@ -341,10 +341,10 @@ fn copy_artifacts(src: &str, dst: &str) -> Result<()> {
                 if let Ok(rel) = link_target.strip_prefix(src_path) {
                     Path::new(dst).join(rel)
                 } else {
-                    link_target.into()
+                    link_target
                 }
             } else {
-                link_target.into()
+                link_target
             };
 
             if let Some(parent) = dst_path.parent() {
@@ -385,7 +385,7 @@ fn copy_artifacts(src: &str, dst: &str) -> Result<()> {
 }
 
 fn set_executable_permissions(dir: &str) -> Result<()> {
-    let bin_paths = vec!["bin", "runtime-rs/bin"];
+    let bin_paths = ["bin", "runtime-rs/bin"];
 
     for bin_path in bin_paths.iter() {
         let bin_dir = Path::new(dir).join(bin_path);

tools/packaging/kata-deploy/binary/src/main.rs

@@ -177,36 +177,30 @@ async fn install(config: &config::Config, runtime: &str) -> Result<()> {
     }
 
     // Validate snapshotter if needed
-    match config.experimental_setup_snapshotter.as_ref() {
-        Some(snapshotter) => {
-            let non_empty_snapshotters: Vec<_> =
-                snapshotter.iter().filter(|s| !s.is_empty()).collect();
+    if let Some(snapshotter) = config.experimental_setup_snapshotter.as_ref() {
+        let non_empty_snapshotters: Vec<_> = snapshotter.iter().filter(|s| !s.is_empty()).collect();
 
-            if !non_empty_snapshotters.is_empty() {
-                if runtime == "crio" {
-                    log::warn!("EXPERIMENTAL_SETUP_SNAPSHOTTER is being ignored!");
-                    log::warn!("Snapshotter is a containerd specific option.");
-                } else {
-                    for s in &non_empty_snapshotters {
-                        match s.as_str() {
-                            "erofs" => {
-                                runtime::containerd::containerd_erofs_snapshotter_version_check(
-                                    config,
-                                )
+        if !non_empty_snapshotters.is_empty() {
+            if runtime == "crio" {
+                log::warn!("EXPERIMENTAL_SETUP_SNAPSHOTTER is being ignored!");
+                log::warn!("Snapshotter is a containerd specific option.");
+            } else {
+                for s in &non_empty_snapshotters {
+                    match s.as_str() {
+                        "erofs" => {
+                            runtime::containerd::containerd_erofs_snapshotter_version_check(config)
                                 .await?;
-                            }
-                            "nydus" => {}
-                            _ => {
-                                return Err(anyhow::anyhow!(
-                                    "{s} is not a supported snapshotter by kata-deploy"
-                                ));
-                            }
+                        }
+                        "nydus" => {}
+                        _ => {
+                            return Err(anyhow::anyhow!(
+                                "{s} is not a supported snapshotter by kata-deploy"
+                            ));
                         }
                     }
                 }
             }
         }
-        None => {}
     }
 
     runtime::containerd::setup_containerd_config_files(runtime, config).await?;
@@ -215,15 +209,14 @@ async fn install(config: &config::Config, runtime: &str) -> Result<()> {
 
     runtime::configure_cri_runtime(config, runtime).await?;
 
-    match config.experimental_setup_snapshotter.as_ref() {
-        Some(snapshotters) => {
+    if runtime != "crio" {
+        if let Some(snapshotters) = config.experimental_setup_snapshotter.as_ref() {
             for snapshotter in snapshotters {
                 artifacts::snapshotters::install_snapshotter(snapshotter, config).await?;
                 artifacts::snapshotters::configure_snapshotter(snapshotter, runtime, config)
                     .await?;
             }
         }
-        None => {}
     }
 
     info!("About to restart runtime: {}", runtime);
@@ -262,17 +255,21 @@ async fn cleanup(config: &config::Config, runtime: &str) -> Result<()> {
         config.daemonset_name
     );
 
-    match config.experimental_setup_snapshotter.as_ref() {
-        Some(snapshotters) => {
-            for snapshotter in snapshotters {
-                info!("Uninstalling snapshotter: {}", snapshotter);
-                artifacts::snapshotters::uninstall_snapshotter(snapshotter, config).await?;
-                info!("Successfully uninstalled snapshotter: {}", snapshotter);
+    if runtime != "crio" {
+        match config.experimental_setup_snapshotter.as_ref() {
+            Some(snapshotters) => {
+                for snapshotter in snapshotters {
+                    info!("Uninstalling snapshotter: {}", snapshotter);
+                    artifacts::snapshotters::uninstall_snapshotter(snapshotter, config).await?;
+                    info!("Successfully uninstalled snapshotter: {}", snapshotter);
+                }
+            }
+            None => {
+                info!("No experimental snapshotters to uninstall");
             }
         }
-        None => {
-            info!("No experimental snapshotters to uninstall");
-        }
+    } else {
+        info!("Skipping snapshotter uninstall on CRI-O (containerd-specific feature)");
     }
 
     info!("Cleaning up CRI runtime configuration");

tools/packaging/kata-deploy/binary/src/utils/toml.rs

@@ -15,9 +15,9 @@ fn parse_toml_path(path: &str) -> Result<Vec<String>> {
     let mut parts = Vec::new();
     let mut current = String::new();
     let mut in_quotes = false;
-    let mut chars = path.chars().peekable();
+    let chars = path.chars().peekable();
 
-    while let Some(ch) = chars.next() {
+    for ch in chars {
         match ch {
             '"' => {
                 in_quotes = !in_quotes;

tools/packaging/kata-deploy/binary/src/utils/yaml.rs

@@ -37,15 +37,14 @@ pub fn set_yaml_value(file_path: &Path, key_path: &str, value: serde_yaml::Value
         } else {
             // Navigate/create intermediate mappings
             if let Some(map) = current.as_mapping_mut() {
-                if !map.contains_key(&serde_yaml::Value::String(part.to_string())) {
+                let key = serde_yaml::Value::String(part.to_string());
+                if !map.contains_key(&key) {
                     map.insert(
-                        serde_yaml::Value::String(part.to_string()),
+                        key.clone(),
                         serde_yaml::Value::Mapping(serde_yaml::Mapping::new()),
                     );
                 }
-                current = map
-                    .get_mut(&serde_yaml::Value::String(part.to_string()))
-                    .unwrap();
+                current = map.get_mut(&key).unwrap();
             } else {
                 return Err(anyhow::anyhow!("Path component '{part}' is not a mapping"));
             }

tools/testing/gatekeeper/required-tests.yaml

@@ -93,7 +93,7 @@ mapping:
       - Kata Containers CI / kata-containers-ci-on-push / run-k8s-tests-on-free-runner / run-k8s-tests (cloud-hypervisor, active)
       - Kata Containers CI / kata-containers-ci-on-push / run-k8s-tests-on-zvsi / run-k8s-tests (devmapper, qemu, kubeadm)
       - Kata Containers CI / kata-containers-ci-on-push / run-k8s-tests-on-zvsi / run-k8s-tests (nydus, qemu-coco-dev, kubeadm)
-      # - Kata Containers CI / kata-containers-ci-on-push / run-kata-coco-tests / run-k8s-tests-on-tee (sev-snp, qemu-snp)
+      - Kata Containers CI / kata-containers-ci-on-push / run-kata-coco-tests / run-k8s-tests-on-tee (sev-snp, qemu-snp)
       - Kata Containers CI / kata-containers-ci-on-push / run-kata-coco-tests / run-k8s-tests-coco-nontee (qemu-coco-dev, nydus, guest-pull)
       - Kata Containers CI / kata-containers-ci-on-push / run-kata-coco-tests / run-k8s-tests-coco-nontee (qemu-coco-dev-runtime-rs, nydus, guest-pull)
       - Kata Containers CI / kata-containers-ci-on-push / run-kata-deploy-tests / run-kata-deploy-tests (qemu, k0s)