Skip to content
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.

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

gha: Add cloud-hypervisor (runtime-rs) support to cri-containerd tests #9182

Merged
merged 4 commits into from Feb 29, 2024

Conversation

GabyCT
Copy link
Contributor

@GabyCT GabyCT commented Feb 28, 2024

This PR adds the Cloud Hypervisor driver, integrated with the runtime-rs, as part of the cri-containerd tests.

Fixes #9181

This PR adds the Cloud Hypervisor driver, integrated with the runtime-rs,
as part of the cri-containerd tests.

Fixes kata-containers#9181

Signed-off-by: Gabriela Cervantes <gabriela.cervantes.tellez@intel.com>
This PR skips the cri-containerd in gha-run script for cloud hypervisor
runtime-rs.

Signed-off-by: Gabriela Cervantes <gabriela.cervantes.tellez@intel.com>
This PR implements general fixes to the gha-run script for the
cri-containerd tests.

Signed-off-by: Gabriela Cervantes <gabriela.cervantes.tellez@intel.com>
@GabyCT
Copy link
Contributor Author

GabyCT commented Feb 28, 2024

/test

Copy link
Contributor

@dborquez dborquez left a comment

Choose a reason for hiding this comment

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

LGTM but I leave a few comments.

@@ -48,7 +48,7 @@ function install_dependencies() {

for github_dep in "${github_deps[@]}"; do
IFS=":" read -r -a dep <<< "${github_dep}"
install_${dep[0]} "${dep[1]}"
install_"${dep[0]}" "${dep[1]}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Here you could enclose the entire word using double quotes:

Suggested change
install_"${dep[0]}" "${dep[1]}"
"install_${dep[0]}" "${dep[1]}"

@@ -35,7 +35,7 @@ function install_dependencies() {
sudo apt-get -y install "${system_deps[@]}"

ensure_yq
${repo_root_dir}/tests/install_go.sh -p -f
"${repo_root_dir}"/tests/install_go.sh -p -f
Copy link
Contributor

Choose a reason for hiding this comment

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

You could enclose the path and the command using double quotes, and call it with the source command (which is an alias for . )

Suggested change
"${repo_root_dir}"/tests/install_go.sh -p -f
. "${repo_root_dir}/tests/install_go.sh" -p -f

This PR adds general variable fixes to gha-run script.

Signed-off-by: Gabriela Cervantes <gabriela.cervantes.tellez@intel.com>
Copy link
Contributor

@dborquez dborquez left a comment

Choose a reason for hiding this comment

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

LGTM thank you @GabyCT

@GabyCT
Copy link
Contributor Author

GabyCT commented Feb 29, 2024

/test

@GabyCT GabyCT merged commit a4f5815 into kata-containers:main Feb 29, 2024
277 of 294 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ok-to-test size/small Small and simple task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

gha: Add cloud-hypervisor (runtime-rs) support to cri-containerd tests
4 participants