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

Add Podman Compatibility and Enhance Kind cluster setup #11086

Open
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

see-quick
Copy link
Member

@see-quick see-quick commented Jan 28, 2025

Type of change

  • Enhancement / new feature
  • Refactoring

Description

This PR introduces Podman compatibility to the KIND cluster setup script. It seamlessly allows users to use Docker or Podman as their container runtime. The changes ensure proper configuration and support for Kubernetes cluster provisioning.

Checklist

  • Write tests
  • Make sure all tests pass

Signed-off-by: see-quick <[email protected]>
@see-quick see-quick added this to the 0.46.0 milestone Jan 28, 2025
@see-quick see-quick requested a review from a team January 28, 2025 09:54
@see-quick see-quick self-assigned this Jan 28, 2025
@ppatierno
Copy link
Member

Why most of the already existing commands are now started via sudo?

.azure/scripts/setup-kind.sh Show resolved Hide resolved
.azure/scripts/setup-kind.sh Outdated Show resolved Hide resolved
.azure/scripts/setup-kind.sh Outdated Show resolved Hide resolved
@see-quick
Copy link
Member Author

Why most of the already existing commands are now started via sudo?

Mainly because we have to run kind of as rootfull (with podman).

@see-quick see-quick requested review from Frawless and a team January 31, 2025 10:44
@see-quick
Copy link
Member Author

/azp run acceptance

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

}

function label_node {
# It should work for all clusters
for nodeName in $(kubectl get nodes -o custom-columns=:.metadata.name --no-headers);
for nodeName in $($SUDO kubectl get nodes -o custom-columns=:.metadata.name --no-headers);
Copy link
Contributor

Choose a reason for hiding this comment

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

should $SUDO be used everywhere?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, because if do not execute with sudo you will not see kind cluster as we previously started kind as sudo

@see-quick
Copy link
Member Author

/packit test --labels acceptance

// Kaniko pushes the image to the local registry, but Podman stores it in an internal storage
// that is not directly accessible to Kind. Podman must first pull the image from the registry
// so that it is available in the Podman daemon's local storage before "kind load" can work.
Exec.exec("sudo", "podman", "pull", image);
Copy link
Member

Choose a reason for hiding this comment

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

what if I will use kind with docker?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I should add some wrapper to check your container runtime and base on that just use podman/docker.

@Frawless Frawless self-requested a review February 6, 2025 12:36
@see-quick
Copy link
Member Author

/packit test --labels acceptance

Signed-off-by: see-quick <[email protected]>
@see-quick
Copy link
Member Author

/packit test --labels acceptance

Signed-off-by: see-quick <[email protected]>
Exec.exec("sudo", ContainerRuntimeUtils.getRuntime(), "pull", image);
}
// if container-runtime is docker we do not need to pull
Exec.exec("sudo", "kind", "load", "docker-image", image, "--name", "kind-cluster");
Copy link
Member

Choose a reason for hiding this comment

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

I guess we will have to think about a different way how to load it, or take the name of the Kind cluster first. Because not all of the Kind clusters are named kind-cluster.

Copy link
Member

Choose a reason for hiding this comment

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

wouldn't be easier just skip this test with podman runtime + kind ?

Copy link
Member

Choose a reason for hiding this comment

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

I guess that's not desired, or?
I mean, yeah for AZPs we would run it anyway, as we are running the tests on Minikube.
For TF, I think that we are using podman there (if not, then fine).

The only issue is on Jenkins, where we are running the nightly builds and I guess the skip would be bad.

Another thing I was thinking was to build the image before running the tests, similarly as we do it with Connect image. But that would add more conditions and setup just for running the one test 🤷 .

Copy link
Member

Choose a reason for hiding this comment

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

I had discussion with @Frawless
I agree that having another kind of conditions around that just for one test doesn't make sense.

For AZP, it will run without issues. For other like OCP as well. So let's skip the test for kind + podman @see-quick

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, I will update this accordingly.

Signed-off-by: see-quick <[email protected]>
@see-quick
Copy link
Member Author

/packit test --labels acceptance

Signed-off-by: see-quick <[email protected]>
@see-quick
Copy link
Member Author

/packit test --labels acceptance

Signed-off-by: see-quick <[email protected]>
@see-quick
Copy link
Member Author

/packit test --labels acceptance

@@ -118,7 +118,7 @@ prepare:
# Setup local registry for building images and connect image
export DOCKER_REGISTRY="${HOST}:5001"
export DOCKER_ORG="strimzi"
export DOCKER_TAG="test"
export DOCKER_TAG="latest"
Copy link
Member

Choose a reason for hiding this comment

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

why is this needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because with test KafkaBridge tests were failing. With the

sed -i "s#quay.io/strimzi/kafka#${DOCKER_REGISTRY}/${DOCKER_ORG}/kafka#g" packaging/install/cluster-operator/*-Deployment-strimzi-cluster-operator.yaml

we were also modifying the content of the KafkaBridge image instead of just replacing Kafka images. So in other words when I fixed the image issue then

quay.io/strimzi/kafka-bridge:test

was another problem. So I have changed the test docker tag to the latest and we would use:

  1. Kafka/operator etc. images with replaced DOCKER_REGISTRY (e.g., 10.2.4.1:5001/strimzi/kafka:latest-kafka-3.9.0)
  2. but KafkaBridge will be used the latest (e.g.,quay.io/strimzi/kafka-bridge:latest)

Copy link
Member

Choose a reason for hiding this comment

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

wouldn't be enought just specify env var for bridge image?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants