github/kata-containers
1
0
Fork 1
mirror of https://github.com/kata-containers/kata-containers.git synced 2025-06-24 06:27:39 +00:00
Code Issues Projects Releases Wiki Activity

Merge pull request #9695 from ChengyuZhu6/fix-init

Browse Source 
Fix issues on CI about guest-pull
This commit is contained in:
Fabiano Fidêncio 2024-07-05 11:21:08 +02:00 committed by GitHub
commit 543c90f145
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 26 additions and 213 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

166
src/agent/src/image.rs
View File

@ -20,8 +20,6 @@ use tokio::sync::Mutex;
use crate::rpc::CONTAINER_BASE; use crate::rpc::CONTAINER_BASE;
use crate::AGENT_CONFIG; use crate::AGENT_CONFIG;
// A marker to merge container spec for images pulled inside guest.
const ANNO_K8S_IMAGE_NAME: &str = "io.kubernetes.cri.image-name";
const KATA_IMAGE_WORK_DIR: &str = "/run/kata-containers/image/"; const KATA_IMAGE_WORK_DIR: &str = "/run/kata-containers/image/";
const CONFIG_JSON: &str = "config.json"; const CONFIG_JSON: &str = "config.json";
const KATA_PAUSE_BUNDLE: &str = "/pause_bundle"; const KATA_PAUSE_BUNDLE: &str = "/pause_bundle";
@ -52,23 +50,17 @@ fn copy_if_not_exists(src: &Path, dst: &Path) -> Result<()> {
pub struct ImageService { pub struct ImageService {
image_client: ImageClient, image_client: ImageClient,
images: HashMap<String, String>,
} }
impl ImageService { impl ImageService {
pub fn new() -> Self { pub fn new() -> Self {
Self { Self {
image_client: ImageClient::new(PathBuf::from(KATA_IMAGE_WORK_DIR)), image_client: ImageClient::new(PathBuf::from(KATA_IMAGE_WORK_DIR)),
images: HashMap::new(),
} }
} }
async fn add_image(&mut self, image: String, cid: String) {
self.images.insert(image, cid);
}
/// pause image is packaged in rootfs /// pause image is packaged in rootfs
fn unpack_pause_image(cid: &str, target_subpath: &str) -> Result<String> { fn unpack_pause_image(cid: &str) -> Result<String> {
verify_id(cid).context("The guest pause image cid contains invalid characters.")?; verify_id(cid).context("The guest pause image cid contains invalid characters.")?;
let guest_pause_bundle = Path::new(KATA_PAUSE_BUNDLE); let guest_pause_bundle = Path::new(KATA_PAUSE_BUNDLE);
@ -102,9 +94,7 @@ impl ImageService {
bail!("The number of args should be greater than or equal to one! Please check the pause image."); bail!("The number of args should be greater than or equal to one! Please check the pause image.");
} }
let container_bundle = scoped_join(CONTAINER_BASE, cid)?; let pause_bundle = scoped_join(CONTAINER_BASE, cid)?;
fs::create_dir_all(&container_bundle)?;
let pause_bundle = scoped_join(&container_bundle, target_subpath)?;
fs::create_dir_all(&pause_bundle)?; fs::create_dir_all(&pause_bundle)?;
let pause_rootfs = scoped_join(&pause_bundle, "rootfs")?; let pause_rootfs = scoped_join(&pause_bundle, "rootfs")?;
fs::create_dir_all(&pause_rootfs)?; fs::create_dir_all(&pause_rootfs)?;
@ -125,7 +115,7 @@ impl ImageService {
/// - `cid`: Container id /// - `cid`: Container id
/// - `image_metadata`: Annotations about the image (exp: "containerd.io/snapshot/cri.layer-digest": "sha256:24fb2886d6f6c5d16481dd7608b47e78a8e92a13d6e64d87d57cb16d5f766d63") /// - `image_metadata`: Annotations about the image (exp: "containerd.io/snapshot/cri.layer-digest": "sha256:24fb2886d6f6c5d16481dd7608b47e78a8e92a13d6e64d87d57cb16d5f766d63")
/// # Returns /// # Returns
/// - The image rootfs bundle path. (exp. /run/kata-containers/cb0b47276ea66ee9f44cc53afa94d7980b57a52c3f306f68cb034e58d9fbd3c6/images/rootfs) /// - The image rootfs bundle path. (exp. /run/kata-containers/cb0b47276ea66ee9f44cc53afa94d7980b57a52c3f306f68cb034e58d9fbd3c6/rootfs)
pub async fn pull_image( pub async fn pull_image(
&mut self, &mut self,
image: &str, image: &str,
@ -146,16 +136,13 @@ impl ImageService {
} }
if is_sandbox { if is_sandbox {
let mount_path = Self::unpack_pause_image(cid, "pause")?; let mount_path = Self::unpack_pause_image(cid)?;
self.add_image(String::from(image), String::from(cid)).await;
return Ok(mount_path); return Ok(mount_path);
} }
// Image layers will store at KATA_IMAGE_WORK_DIR, generated bundles // Image layers will store at KATA_IMAGE_WORK_DIR, generated bundles
// with rootfs and config.json will store under CONTAINER_BASE/cid/images. // with rootfs and config.json will store under CONTAINER_BASE/cid/images.
let bundle_base_dir = scoped_join(CONTAINER_BASE, cid)?; let bundle_path = scoped_join(CONTAINER_BASE, cid)?;
fs::create_dir_all(&bundle_base_dir)?;
let bundle_path = scoped_join(&bundle_base_dir, "images")?;
fs::create_dir_all(&bundle_path)?; fs::create_dir_all(&bundle_path)?;
info!(sl(), "pull image {image:?}, bundle path {bundle_path:?}"); info!(sl(), "pull image {image:?}, bundle path {bundle_path:?}");
@ -179,35 +166,9 @@ impl ImageService {
return Err(e); return Err(e);
} }
}; };
self.add_image(String::from(image), String::from(cid)).await;
let image_bundle_path = scoped_join(&bundle_path, "rootfs")?; let image_bundle_path = scoped_join(&bundle_path, "rootfs")?;
Ok(image_bundle_path.as_path().display().to_string()) Ok(image_bundle_path.as_path().display().to_string())
} }
/// Partially merge an OCI process specification into another one.
fn merge_oci_process(&self, target: &mut oci::Process, source: &oci::Process) {
// Override the target args only when the target args is empty and source.args is not empty
if target.args.is_empty() && !source.args.is_empty() {
target.args.append(&mut source.args.clone());
}
// Override the target cwd only when the target cwd is blank and source.cwd is not blank
if target.cwd == "/" && source.cwd != "/" {
target.cwd = String::from(&source.cwd);
}
for source_env in &source.env {
if let Some((variable_name, variable_value)) = source_env.split_once('=') {
debug!(
sl(),
"source spec environment variable: {variable_name:?} : {variable_value:?}"
);
if !target.env.iter().any(|i| i.contains(variable_name)) {
target.env.push(source_env.to_string());
}
}
}
}
} }
/// Set proxy environment from AGENT_CONFIG /// Set proxy environment from AGENT_CONFIG
@ -237,55 +198,6 @@ pub async fn set_proxy_env_vars() {
}; };
} }
/// When being passed an image name through a container annotation, merge its
/// corresponding bundle OCI specification into the passed container creation one.
pub async fn merge_bundle_oci(container_oci: &mut oci::Spec) -> Result<()> {
let image_service = IMAGE_SERVICE.clone();
let mut image_service = image_service.lock().await;
let image_service = image_service
.as_mut()
.expect("Image Service not initialized");
if let Some(image_name) = container_oci.annotations.get(ANNO_K8S_IMAGE_NAME) {
if let Some(container_id) = image_service.images.get(image_name) {
let image_oci_config_path = Path::new(CONTAINER_BASE)
.join(container_id)
.join(CONFIG_JSON);
debug!(
sl(),
"Image bundle config path: {:?}", image_oci_config_path
);
let image_oci = oci::Spec::load(image_oci_config_path.to_str().ok_or_else(|| {
anyhow!(
"Invalid container image OCI config path {:?}",
image_oci_config_path
)
})?)
.context("load image bundle")?;
if let (Some(container_root), Some(image_root)) =
(container_oci.root.as_mut(), image_oci.root.as_ref())
{
let root_path = Path::new(CONTAINER_BASE)
.join(container_id)
.join(image_root.path.clone());
container_root.path =
String::from(root_path.to_str().ok_or_else(|| {
anyhow!("Invalid container image root path {:?}", root_path)
})?);
}
if let (Some(container_process), Some(image_process)) =
(container_oci.process.as_mut(), image_oci.process.as_ref())
{
image_service.merge_oci_process(container_process, image_process);
}
}
}
Ok(())
}
/// Init the image service /// Init the image service
pub async fn init_image_service() { pub async fn init_image_service() {
let image_service = ImageService::new(); let image_service = ImageService::new();
@ -305,71 +217,3 @@ pub async fn pull_image(
image_service.pull_image(image, cid, image_metadata).await image_service.pull_image(image, cid, image_metadata).await
} }
#[cfg(test)]
mod tests {
use super::ImageService;
use rstest::rstest;
#[rstest]
// TODO - how can we tell the user didn't specifically set it to `/` vs not setting at all? Is that scenario valid?
#[case::image_cwd_should_override_blank_container_cwd("/", "/imageDir", "/imageDir")]
#[case::container_cwd_should_override_image_cwd("/containerDir", "/imageDir", "/containerDir")]
#[case::container_cwd_should_override_blank_image_cwd("/containerDir", "/", "/containerDir")]
async fn test_merge_cwd(
#[case] container_process_cwd: &str,
#[case] image_process_cwd: &str,
#[case] expected: &str,
) {
let image_service = ImageService::new();
let mut container_process = oci::Process {
cwd: container_process_cwd.to_string(),
..Default::default()
};
let image_process = oci::Process {
cwd: image_process_cwd.to_string(),
..Default::default()
};
image_service.merge_oci_process(&mut container_process, &image_process);
assert_eq!(expected, container_process.cwd);
}
#[rstest]
#[case::pods_environment_overrides_images(
vec!["ISPRODUCTION=true".to_string()],
vec!["ISPRODUCTION=false".to_string()],
vec!["ISPRODUCTION=true".to_string()]
)]
#[case::multiple_environment_variables_can_be_overrided(
vec!["ISPRODUCTION=true".to_string(), "ISDEVELOPMENT=false".to_string()],
vec!["ISPRODUCTION=false".to_string(), "ISDEVELOPMENT=true".to_string()],
vec!["ISPRODUCTION=true".to_string(), "ISDEVELOPMENT=false".to_string()]
)]
#[case::not_override_them_when_none_of_variables_match(
vec!["ANOTHERENV=TEST".to_string()],
vec!["ISPRODUCTION=false".to_string(), "ISDEVELOPMENT=true".to_string()],
vec!["ANOTHERENV=TEST".to_string(), "ISPRODUCTION=false".to_string(), "ISDEVELOPMENT=true".to_string()]
)]
#[case::a_mix_of_both_overriding_and_not(
vec!["ANOTHERENV=TEST".to_string(), "ISPRODUCTION=true".to_string()],
vec!["ISPRODUCTION=false".to_string(), "ISDEVELOPMENT=true".to_string()],
vec!["ANOTHERENV=TEST".to_string(), "ISPRODUCTION=true".to_string(), "ISDEVELOPMENT=true".to_string()]
)]
async fn test_merge_env(
#[case] container_process_env: Vec<String>,
#[case] image_process_env: Vec<String>,
#[case] expected: Vec<String>,
) {
let image_service = ImageService::new();
let mut container_process = oci::Process {
env: container_process_env,
..Default::default()
};
let image_process = oci::Process {
env: image_process_env,
..Default::default()
};
image_service.merge_oci_process(&mut container_process, &image_process);
assert_eq!(expected, container_process.env);
}
}

16
src/agent/src/rpc.rs
View File

@ -210,11 +210,6 @@ impl AgentService {
"receive createcontainer, storages: {:?}", &req.storages "receive createcontainer, storages: {:?}", &req.storages
); );
// In case of pulling image inside guest, we need to merge the image bundle OCI spec
// into the container creation request OCI spec.
#[cfg(feature = "guest-pull")]
image::merge_bundle_oci(&mut oci).await?;
// Some devices need some extra processing (the ones invoked with // Some devices need some extra processing (the ones invoked with
// --device for instance), and that's what this call is doing. It // --device for instance), and that's what this call is doing. It
// updates the devices listed in the OCI spec, so that they actually // updates the devices listed in the OCI spec, so that they actually
@ -1920,12 +1915,18 @@ pub fn setup_bundle(cid: &str, spec: &mut Spec) -> Result<PathBuf> {
return Err(anyhow!(nix::Error::EINVAL)); return Err(anyhow!(nix::Error::EINVAL));
}; };
let spec_root_path = Path::new(&spec_root.path);
let bundle_path = Path::new(CONTAINER_BASE).join(cid); let bundle_path = Path::new(CONTAINER_BASE).join(cid);
let config_path = bundle_path.join("config.json"); let config_path = bundle_path.join("config.json");
let rootfs_path = bundle_path.join("rootfs"); let rootfs_path = bundle_path.join("rootfs");
let spec_root_path = Path::new(&spec_root.path);
let rootfs_exists = Path::new(&rootfs_path).exists();
info!(
sl(),
"The rootfs_path is {:?} and exists: {}", rootfs_path, rootfs_exists
);
if !rootfs_exists {
fs::create_dir_all(&rootfs_path)?; fs::create_dir_all(&rootfs_path)?;
baremount( baremount(
spec_root_path, spec_root_path,
@ -1935,6 +1936,7 @@ pub fn setup_bundle(cid: &str, spec: &mut Spec) -> Result<PathBuf> {
"", "",
&sl(), &sl(),
)?; )?;
}
let rootfs_path_name = rootfs_path let rootfs_path_name = rootfs_path
.to_str() .to_str()

12
src/agent/src/storage/image_pull_handler.rs
View File

@ -3,6 +3,7 @@
// SPDX-License-Identifier: Apache-2.0 // SPDX-License-Identifier: Apache-2.0
// //
use super::new_device;
use crate::image; use crate::image;
use crate::storage::{StorageContext, StorageHandler}; use crate::storage::{StorageContext, StorageHandler};
use anyhow::{anyhow, Result}; use anyhow::{anyhow, Result};
@ -12,8 +13,6 @@ use protocols::agent::Storage;
use std::sync::Arc; use std::sync::Arc;
use tracing::instrument; use tracing::instrument;
use super::{common_storage_handler, new_device};
#[derive(Debug)] #[derive(Debug)]
pub struct ImagePullHandler {} pub struct ImagePullHandler {}
@ -36,7 +35,7 @@ impl StorageHandler for ImagePullHandler {
#[instrument] #[instrument]
async fn create_device( async fn create_device(
&self, &self,
mut storage: Storage, storage: Storage,
ctx: &mut StorageContext, ctx: &mut StorageContext,
) -> Result<Arc<dyn StorageDevice>> { ) -> Result<Arc<dyn StorageDevice>> {
//Currently the image metadata is not used to pulling image in the guest. //Currently the image metadata is not used to pulling image in the guest.
@ -51,12 +50,7 @@ impl StorageHandler for ImagePullHandler {
.ok_or_else(|| anyhow!("failed to get container id"))?; .ok_or_else(|| anyhow!("failed to get container id"))?;
let bundle_path = image::pull_image(image_name, &cid, &image_pull_volume.metadata).await?; let bundle_path = image::pull_image(image_name, &cid, &image_pull_volume.metadata).await?;
storage.source = bundle_path; new_device(bundle_path)
storage.options = vec!["bind".to_string(), "ro".to_string()];
common_storage_handler(ctx.logger, &storage)?;
new_device(storage.mount_point)
} }
} }

4
tests/integration/kubernetes/k8s-file-volume.bats
View File

@ -12,8 +12,6 @@ TEST_INITRD="${TEST_INITRD:-no}"
setup() { setup() {
[ "${KATA_HYPERVISOR}" == "firecracker" ] && skip "test not working see: ${fc_limitations}" [ "${KATA_HYPERVISOR}" == "firecracker" ] && skip "test not working see: ${fc_limitations}"
[ "${KATA_HYPERVISOR}" == "fc" ] && skip "test not working see: ${fc_limitations}" [ "${KATA_HYPERVISOR}" == "fc" ] && skip "test not working see: ${fc_limitations}"
[[ "${KATA_HYPERVISOR}" == "qemu-tdx" || "${KATA_HYPERVISOR}" == "qemu-coco-dev" ]] && \
skip "See: https://github.com/kata-containers/kata-containers/issues/9667"
pod_name="test-file-volume" pod_name="test-file-volume"
container_name="busybox-file-volume-container" container_name="busybox-file-volume-container"
@ -60,8 +58,6 @@ setup() {
teardown() { teardown() {
[ "${KATA_HYPERVISOR}" == "firecracker" ] && skip "test not working see: ${fc_limitations}" [ "${KATA_HYPERVISOR}" == "firecracker" ] && skip "test not working see: ${fc_limitations}"
[ "${KATA_HYPERVISOR}" == "fc" ] && skip "test not working see: ${fc_limitations}" [ "${KATA_HYPERVISOR}" == "fc" ] && skip "test not working see: ${fc_limitations}"
[[ "${KATA_HYPERVISOR}" == "qemu-tdx" || "${KATA_HYPERVISOR}" == "qemu-coco-dev" ]] && \
skip "See: https://github.com/kata-containers/kata-containers/issues/9667"
kubectl describe pod "$pod_name" kubectl describe pod "$pod_name"

8
tests/integration/kubernetes/k8s-kill-all-process-in-container.bats
View File

@ -9,10 +9,6 @@ load "${BATS_TEST_DIRNAME}/../../common.bash"
load "${BATS_TEST_DIRNAME}/tests_common.sh" load "${BATS_TEST_DIRNAME}/tests_common.sh"
setup() { setup() {
[[ "${KATA_HYPERVISOR}" = "qemu-tdx" || "${KATA_HYPERVISOR}" = "qemu-coco-dev" || \
"${KATA_HYPERVISOR}" = "qemu-sev" || "${KATA_HYPERVISOR}" = "qemu-snp" ]] && \
skip "See: https://github.com/kata-containers/kata-containers/issues/9664"
pod_name="busybox" pod_name="busybox"
first_container_name="first-test-container" first_container_name="first-test-container"
@ -43,10 +39,6 @@ setup() {
} }
teardown() { teardown() {
[[ "${KATA_HYPERVISOR}" = "qemu-tdx" || "${KATA_HYPERVISOR}" = "qemu-coco-dev" || \
"${KATA_HYPERVISOR}" = "qemu-sev" || "${KATA_HYPERVISOR}" = "qemu-snp" ]] && \
skip "See: https://github.com/kata-containers/kata-containers/issues/9664"
# Debugging information # Debugging information
kubectl describe "pod/$pod_name" kubectl describe "pod/$pod_name"

4
tests/integration/kubernetes/k8s-liveness-probes.bats
View File

@ -9,8 +9,6 @@ load "${BATS_TEST_DIRNAME}/../../common.bash"
load "${BATS_TEST_DIRNAME}/tests_common.sh" load "${BATS_TEST_DIRNAME}/tests_common.sh"
setup() { setup() {
[ "${KATA_HYPERVISOR}" = "qemu-tdx" ] && skip "See: https://github.com/kata-containers/kata-containers/issues/9665"
sleep_liveness=20 sleep_liveness=20
agnhost_name="${container_images_agnhost_name}" agnhost_name="${container_images_agnhost_name}"
agnhost_version="${container_images_agnhost_version}" agnhost_version="${container_images_agnhost_version}"
@ -91,8 +89,6 @@ setup() {
} }
teardown() { teardown() {
[ "${KATA_HYPERVISOR}" = "qemu-tdx" ] && skip "See: https://github.com/kata-containers/kata-containers/issues/9665"
# Debugging information # Debugging information
kubectl describe "pod/$pod_name" kubectl describe "pod/$pod_name"

5
tests/integration/kubernetes/k8s-shared-volume.bats
View File

@ -13,8 +13,6 @@ setup() {
} }
@test "Containers with shared volume" { @test "Containers with shared volume" {
[ "${KATA_HYPERVISOR}" = "qemu-coco-dev" ] && \
skip "See: https://github.com/kata-containers/kata-containers/issues/9668"
pod_name="test-shared-volume" pod_name="test-shared-volume"
first_container_name="busybox-first-container" first_container_name="busybox-first-container"
second_container_name="busybox-second-container" second_container_name="busybox-second-container"
@ -42,9 +40,6 @@ setup() {
} }
@test "initContainer with shared volume" { @test "initContainer with shared volume" {
[[ "${KATA_HYPERVISOR}" = "qemu-tdx" || "${KATA_HYPERVISOR}" = "qemu-coco-dev" || \
"${KATA_HYPERVISOR}" = "qemu-sev" || "${KATA_HYPERVISOR}" = "qemu-snp" ]] && \
skip "See: https://github.com/kata-containers/kata-containers/issues/9668"
pod_name="initcontainer-shared-volume" pod_name="initcontainer-shared-volume"
last_container="last" last_container="last"

6
tests/integration/kubernetes/k8s-sysctls.bats
View File

@ -9,9 +9,6 @@ load "${BATS_TEST_DIRNAME}/../../common.bash"
load "${BATS_TEST_DIRNAME}/tests_common.sh" load "${BATS_TEST_DIRNAME}/tests_common.sh"
setup() { setup() {
[[ "${KATA_HYPERVISOR}" = "qemu-tdx" || "${KATA_HYPERVISOR}" = "qemu-coco-dev" || \
"${KATA_HYPERVISOR}" = "qemu-sev" || "${KATA_HYPERVISOR}" = "qemu-snp" ]] && \
skip "See: https://github.com/kata-containers/kata-containers/issues/9666"
pod_name="sysctl-test" pod_name="sysctl-test"
get_pod_config_dir get_pod_config_dir
@ -34,9 +31,6 @@ setup() {
} }
teardown() { teardown() {
[[ "${KATA_HYPERVISOR}" = "qemu-tdx" || "${KATA_HYPERVISOR}" = "qemu-coco-dev" || \
"${KATA_HYPERVISOR}" = "qemu-sev" || "${KATA_HYPERVISOR}" = "qemu-snp" ]] && \
skip "See: https://github.com/kata-containers/kata-containers/issues/9666"
# Debugging information # Debugging information
kubectl describe "pod/$pod_name" kubectl describe "pod/$pod_name"