msft-preview: 3.31.0 rebase

[Redent0r]

Test Methodology

CI: https://dev.azure.com/mariner-org/mariner/_build/results?buildId=1127027&view=logs&j=1d103282-d184-539c-6f02-9cecc7887239&t=7581b7d8-55b2-5d9a-91e4-4f9c30e5e89d:

• bb: https://dev.azure.com/mariner-org/mariner/_build/results?buildId=1127047&view=results
• image: https://dev.azure.com/mariner-org/mariner/_build/results?buildId=1127107&view=results
• conformance: https://dev.azure.com/mariner-org/mariner/_build/results?buildId=1127971&view=results
  • rerun with full internal e2e pass https://dev.azure.com/mariner-org/mariner/_build/results?buildId=1128506&view=logs&j=68969b86-11ac-5f8e-d744-19d1afb7c1a4&t=98827e81-2447-5483-445a-cd8743892305&s=e3d9774e-f534-5b4e-225f-291938d68dcf
• performance: https://dev.azure.com/mariner-org/mariner/_build/results?buildId=1127972&view=results

How are current diff (msft-preview) looks like for comparison: #451

Worth to note changes:

• dropped kata-containers@c06b470 (enforce min 128Mi) and it's revert kata-containers@a3da27e
• dropped CH api updates in runtime-go (eg kata-containers@9ef2bb6): we are aligned at the moment (upstream uses 51 and we are at 51 currently)
• Changes present in upstream changes now:
  • network: preseed default-gateway neighbor kata-containers@a136359
  • runtime: run gofmt kata-containers@b27d196
  • clh: Add VFIO device cold-plug support kata-containers@35da6c2
  • runtime: clh: disable nested vCPUs on MSHV kata-containers@040791b
  • static-build: delete cloud-hypervisor directory kata-containers@29f1b96
  • build: Don't fail cargo check on a dirty tree kata-containers@585134b
• cloud-hypervisor/runtime-rs naming got changed from cloud-hypervisor to clh-runtime-rs. This affects our node builder recipes at the time of copying the configs
• kata-ctl binary output is placed in the repo's /target folder, no longer under src/tools/kata-ctl target
• kata-containers-tools need to install the VERSION file for the UVM to build as expected

[Redent0r]

todo: without ac1ca3d , uvm build fails with https://dev.azure.com/mariner-org/mariner/_build/results?buildId=1122834&view=logs&j=bbe135f3-541e-58e6-8b02-61d4669cdf5f&t=5095dea9-1b86-57b3-726c-ffe250f063d1&l=1963 . Figure out what changed, if we need to upstream this:

• Per kata-containers@62fd84d , VERSION should not be needed
• this hasn't been an issue until now

[Redent0r]

todo: without ac1ca3d , uvm build fails with https://dev.azure.com/mariner-org/mariner/_build/results?buildId=1122834&view=logs&j=bbe135f3-541e-58e6-8b02-61d4669cdf5f&t=5095dea9-1b86-57b3-726c-ffe250f063d1&l=1963 . Figure out what changed, if we need to upstream this:

• Per kata-containers@62fd84d , VERSION should not be needed
• this hasn't been an issue until now

Mystery solved. Due to shellcheck fix in kata-containers@6471894#diff-9cff33aa4403bdc6b3a82a5fb4abd4aa4c6038938f8d0f63bbf9c5e9456f7695 , particularly updating

local agent_version=$(cat ${agentdir}/VERSION 2> /dev/null)
[ -z "$agent_version" ] && agent_version="unknown"

to

local agent_version
agent_version=$(cat "${agentdir}/VERSION" 2> /dev/null)
[[ -z "${agent_version}" ]] && agent_version="unknown"

make us fail due to not copying VERSION during package_tools_install.sh. So we'll copy that now.

Demo showing change in behaviour.

#!/usr/bin/env bash

set -o errexit
set -o nounset
set -o pipefail

missing_file="/definitely-missing-file"

demo_local_declaration_assignment() {
	echo "demo 1: local declaration with command substitution"
	local value=$(cat "${missing_file}" 2>/dev/null)
	[[ -z "${value}" ]] && value="unknown"
	echo "demo 1 result: ${value}"
}

demo_split_declaration_assignment() {
	echo "demo 2: split declaration and assignment"
	local value
	value=$(cat "${missing_file}" 2>/dev/null)
	[[ -z "${value}" ]] && value="unknown"
	echo "demo 2 result: ${value}"
}

main() {
	echo "Running with set -euo pipefail"
	echo "Missing file: ${missing_file}"
	echo

	echo "Expect demo 1 to survive and print unknown."
	demo_local_declaration_assignment
	echo "demo 1 completed"
	echo

	echo "Expect demo 2 to exit before printing a result."
	demo_split_declaration_assignment
	echo "demo 2 completed"
}

main "$@"

[Redent0r]

DO NOT MERGE: we'll force push to msft-preview instead once approved

[sprt]

This almost LGTM! A few asks:

• Can you change the first commit message to this:

ci: switch default branch to msft-preview

* Update the default branch to msft-preview in different places for
  the CI to work with our fork.
* Add the MSFT-required SECURITY.md and corresponding dictionary entries.

Signed-off-by: Saul Paredes <saulparedes@microsoft.com>

• Can you link to passing BB/conformance/performance test runs in the PR description?

---

Re: the actual merge, I actually think it's better to lean even more into this up/3.31.0 branch to have a paper trail. So I removed the WIP label and when this PR is fully ready, let us:

1. Ensure the gatekeeper is green.
2. Get approvals.
3. Merge this PR into up/3.31.0 as is (WITH the last commit with the temporary up/3.31.0 CI branch changes).
4. Then we can just do the following to force push msft-preview:

git checkout msft-preview
git reset --hard up/3.31.0 # This clones up/3.31.0 into msft-preview
git reset --hard HEAD^ # This removes the last commit with the temporary up/3.31.0 CI branch changes
git push -f # Push the new msft-preview

[Redent0r]

This almost LGTM! A few asks:

• Can you change the first commit message to this:

ci: switch default branch to msft-preview

* Update the default branch to msft-preview in different places for
  the CI to work with our fork.
* Add the MSFT-required SECURITY.md and corresponding dictionary entries.

Signed-off-by: Saul Paredes <saulparedes@microsoft.com>

• Can you link to passing BB/conformance/performance test runs in the PR description?

Re: the actual merge, I actually think it's better to lean even more into this up/3.31.0 branch to have a paper trail. So I removed the WIP label and when this PR is fully ready, let us:

1. Ensure the gatekeeper is green.
2. Get approvals.
3. Merge this PR into up/3.31.0 as is (WITH the last commit with the temporary up/3.31.0 CI branch changes).
4. Then we can just do the following to force push msft-preview:

git checkout msft-preview
git reset --hard up/3.31.0 # This clones up/3.31.0 into msft-preview
git reset --hard HEAD^ # This removes the last commit with the temporary up/3.31.0 CI branch changes
git push -f # Push the new msft-preview

addressed and trying to re-require a few more tests (will squash if it works)

[sprt]

Just one nit as we wait for the test results!

[sprt]

LGTM, understanding that we have one pending regression that will be prioritized right after this.

[Redent0r]

LGTM, understanding that we have one pending regression that will be prioritized right after this.

https://dev.azure.com/mariner-org/container-runtime/_workitems/edit/20444

[danmihai1]

I would prefer to have commit SHA values next to description like

runtime: Remove unused VMM options for mem alloc

• We only ever tested these fork changes with CLH+MSHV
• Remove these options as we don't use QEMU/FC

Signed-off-by: Manuel Huber mahuber@microsoft.com

But, missing that information is not a big deal, if it's relatively difficult to include.

[danmihai1]

I would prefer to see in the commit description of the "webhook: enforce minimum memory limit" change a more detailed explanation of the goals for this change, what fails if we don't make the change, that the change is specific to testing, etc.

[Redent0r]

@danmihai1 I'll address these next rebase