Skip to content
This repository was archived by the owner on Feb 1, 2026. It is now read-only.

tests/e2e: allow to superseed images in operator.sh#341

Open
wainersm wants to merge 2 commits into
confidential-containers:mainfrom
wainersm:operator_install_not_local
Open

tests/e2e: allow to superseed images in operator.sh#341
wainersm wants to merge 2 commits into
confidential-containers:mainfrom
wainersm:operator_install_not_local

Conversation

@wainersm

Copy link
Copy Markdown
Member

I started working on adapting the Kata CI scripts to install from this operator. I will be trying to re-use the scripts in tests/e2e/ as much as possible, for example, tests/e2e/operator.sh install to install and check Kata Containers in the cluster's nodes. So far I faced two problems with operator.sh though:

  1. one cannot overwrite the default kata-payload image. And it will be needed to point the operator to the image built by the kata CI, if it runs in a pull-request triggered job;
  2. likewise one cannot overwirte the operator controller and reqs-payload images. For those images there is an extra problem which is operator.sh uses the built-and-pushed-to-local-registry images so it always tries to start a local registry service. If it is installing from quay.io (or ghcr.io), for example, it makes no sense the local registry.

So this PR address those two problems. Also by making operator.sh more flexible, this will likely to solve issues when we start improving the operator CI pipeline as suggested in #309

If that helps on review, here goes a sample of the code I'm writing for the Kata CI side:

function install_coco_operator() {
	local repo

	repo=$(get_from_kata_deps "externals.coco-operator.url")
	if [ -f "$COCO_OPERATOR_DIR" ]; then
		rm -rf "$COCO_OPERATOR_DIR"
	fi
	git clone "$repo" "$COCO_OPERATOR_DIR"
	pushd "$COCO_OPERATOR_DIR"
		# Let's install the kustomize tool if not present.
		make kustomize
		echo "::group::Install operator"
		PATH="$PATH:${PWD}/bin" echo "./tests/e2e/operator.sh install"
		echo "::endgroup::"
	popd
}

I tested locally by running:

$ IMG="quay.io/confidential-containers/operator:latest" \
PRE_INSTALL_IMG="quay.io/confidential-containers/reqs-payload:latest" \
KATA_PAYLOAD_IMG="quay.io/kata-containers/kata-deploy-ci:kata-containers-latest" \
PATH=$PATH:${PWD}/bin \
./tests/e2e/operator.sh install

`./tests/e2e/operator.sh install` is still tightly coupled with the
build phase, which happen to push images to a local registry, so the
install tries to always start the registry service. Now suppose that you
want to run `./tests/e2e/operator.sh install` stand-alone to install
images from quay.io, then the registry service locally is not needed.

Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
By exporting $KATA_PAYLOAD_IMG variable the `./tests/e2e/operator.sh
install` will replace the default kata-payload image.

Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>

@stevenhorsman stevenhorsman left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, Thanks Wainer

@portersrc

Copy link
Copy Markdown
Member

lgtm

Comment thread tests/e2e/operator.sh
"install": install only,
"wait_for_stabilization": wait for CoCo pods to be stable
"uninstall": uninstall the operator.
environment variables :

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I really like this section, could you please (in a separate commit) add the RUNTIMECLASS, which is already supported to be set?

@ldoktor ldoktor left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Overall it looks good, only the already-supported-to-be-set variable is missing in the usage message. Could you please add it (Ideally as a first commit to introduce this section, alternatively just as a part of the current first commit which defines the section)

@GabyCT

GabyCT commented Jan 29, 2024

Copy link
Copy Markdown
Contributor

lgtm

@beraldoleal beraldoleal left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Except from the @ldoktor comments, LGTM.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants