Skip to content

🌱 Remove kaniko / extension-developer-e2e cleanup #2116

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jul 24, 2025
Merged

🌱 Remove kaniko / extension-developer-e2e cleanup #2116

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,14 +223,14 @@ E2E_REGISTRY_NAME := docker-registry
E2E_REGISTRY_NAMESPACE := operator-controller-e2e

export REG_PKG_NAME := registry-operator
export LOCAL_REGISTRY_HOST := $(E2E_REGISTRY_NAME).$(E2E_REGISTRY_NAMESPACE).svc:5000
export CLUSTER_REGISTRY_HOST := localhost:30000
export CLUSTER_REGISTRY_HOST := $(E2E_REGISTRY_NAME).$(E2E_REGISTRY_NAMESPACE).svc:5000
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The naming of CLUSTER vs LOCAL for registry host was backwards IMO, so I switched it around.

Copy link
Contributor

Choose a reason for hiding this comment

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

The original naming was cluster-centric... I do wonder if there are better names... e.g.
INSIDE_CLUSTER ?
OUTSIDE_CLUSTER ?

export LOCAL_REGISTRY_HOST := localhost:30000
export E2E_TEST_CATALOG_V1 := e2e/test-catalog:v1
export E2E_TEST_CATALOG_V2 := e2e/test-catalog:v2
export CATALOG_IMG := $(LOCAL_REGISTRY_HOST)/$(E2E_TEST_CATALOG_V1)
export CATALOG_IMG := $(CLUSTER_REGISTRY_HOST)/$(E2E_TEST_CATALOG_V1)
.PHONY: test-ext-dev-e2e
test-ext-dev-e2e: $(OPERATOR_SDK) $(KUSTOMIZE) $(KIND) #HELP Run extension create, upgrade and delete tests.
test/extension-developer-e2e/setup.sh $(OPERATOR_SDK) $(CONTAINER_RUNTIME) $(KUSTOMIZE) $(KIND) $(KIND_CLUSTER_NAME) $(E2E_REGISTRY_NAMESPACE)
test-ext-dev-e2e: $(OPERATOR_SDK) $(KUSTOMIZE) #HELP Run extension create, upgrade and delete tests.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we not keep the $(KIND) here: test-ext-dev-e2e: $(OPERATOR_SDK) $(KUSTOMIZE)
We removed it from the shell test/extension-developer-e2e/setup.sh but I understand that the above will ensure that we have it installed.

Copy link
Contributor Author

@dtfranz dtfranz Jul 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

IMO we should limit dependency declaration like that to targets that actually use the dependency. In my mind this would be a little bit like adding $(KIND) to the e2e target; it's not necessary because e2e can't be run without the prerequisites from test-e2e already being called, and if they weren't then having $(KIND) declared as a dependency for it wouldn't help at all.

Copy link
Contributor

Choose a reason for hiding this comment

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

make sense 👍

test/extension-developer-e2e/setup.sh $(OPERATOR_SDK) $(CONTAINER_RUNTIME) $(KUSTOMIZE) ${LOCAL_REGISTRY_HOST} ${CLUSTER_REGISTRY_HOST}
go test -count=1 -v ./test/extension-developer-e2e/...

UNIT_TEST_DIRS := $(shell go list ./... | grep -v /test/)
Expand Down
8 changes: 4 additions & 4 deletions test/e2e/cluster_extension_install_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -730,7 +730,7 @@ func TestClusterExtensionInstallReResolvesWhenCatalogIsPatched(t *testing.T) {

// patch imageRef tag on test-catalog image with v2 image
t.Log("By patching the catalog ImageRef to point to the v2 catalog")
updatedCatalogImage := fmt.Sprintf("%s/e2e/test-catalog:v2", os.Getenv("LOCAL_REGISTRY_HOST"))
updatedCatalogImage := fmt.Sprintf("%s/e2e/test-catalog:v2", os.Getenv("CLUSTER_REGISTRY_HOST"))
err := patchTestCatalog(context.Background(), testCatalogName, updatedCatalogImage)
require.NoError(t, err)
require.EventuallyWithT(t, func(ct *assert.CollectT) {
Expand Down Expand Up @@ -759,12 +759,12 @@ func TestClusterExtensionInstallReResolvesWhenNewCatalog(t *testing.T) {

// Tag the image with the new tag
var err error
v1Image := fmt.Sprintf("%s/%s", os.Getenv("CLUSTER_REGISTRY_HOST"), os.Getenv("E2E_TEST_CATALOG_V1"))
v1Image := fmt.Sprintf("%s/%s", os.Getenv("LOCAL_REGISTRY_HOST"), os.Getenv("E2E_TEST_CATALOG_V1"))
err = crane.Tag(v1Image, latestImageTag, crane.Insecure)
require.NoError(t, err)

// create a test-catalog with latest image tag
latestCatalogImage := fmt.Sprintf("%s/e2e/test-catalog:latest", os.Getenv("LOCAL_REGISTRY_HOST"))
latestCatalogImage := fmt.Sprintf("%s/e2e/test-catalog:latest", os.Getenv("CLUSTER_REGISTRY_HOST"))
extensionCatalog, err := createTestCatalog(context.Background(), testCatalogName, latestCatalogImage)
require.NoError(t, err)
clusterExtensionName := fmt.Sprintf("clusterextension-%s", rand.String(8))
Expand Down Expand Up @@ -810,7 +810,7 @@ func TestClusterExtensionInstallReResolvesWhenNewCatalog(t *testing.T) {

// update tag on test-catalog image with v2 image
t.Log("By updating the catalog tag to point to the v2 catalog")
v2Image := fmt.Sprintf("%s/%s", os.Getenv("CLUSTER_REGISTRY_HOST"), os.Getenv("E2E_TEST_CATALOG_V2"))
v2Image := fmt.Sprintf("%s/%s", os.Getenv("LOCAL_REGISTRY_HOST"), os.Getenv("E2E_TEST_CATALOG_V2"))
err = crane.Tag(v2Image, latestImageTag, crane.Insecure)
require.NoError(t, err)
require.EventuallyWithT(t, func(ct *assert.CollectT) {
Expand Down
2 changes: 1 addition & 1 deletion test/experimental-e2e/experimental_e2e_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ func TestWebhookSupport(t *testing.T) {
Source: ocv1.CatalogSource{
Type: ocv1.SourceTypeImage,
Image: &ocv1.ImageSource{
Ref: fmt.Sprintf("%s/e2e/test-catalog:v1", os.Getenv("LOCAL_REGISTRY_HOST")),
Ref: fmt.Sprintf("%s/e2e/test-catalog:v1", os.Getenv("CLUSTER_REGISTRY_HOST")),
PollIntervalMinutes: ptr.To(1),
},
},
Expand Down
3 changes: 3 additions & 0 deletions test/extension-developer-e2e/extension_developer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ func TestExtensionDeveloper(t *testing.T) {
require.NoError(t, corev1.AddToScheme(scheme))
require.NoError(t, rbacv1.AddToScheme(scheme))

require.NotEmpty(t, os.Getenv("CATALOG_IMG"), "environment variable CATALOG_IMG must be set")
Copy link
Contributor Author

@dtfranz dtfranz Jul 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

These fields are required to be set for the test, and if not it will fail in an unclear way.

require.NotEmpty(t, os.Getenv("REG_PKG_NAME"), "environment variable REG_PKG_NAME must be set")

c, err := client.New(cfg, client.Options{Scheme: scheme})
require.NoError(t, err)

Expand Down
150 changes: 26 additions & 124 deletions test/extension-developer-e2e/setup.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,27 +11,26 @@ following bundle formats:
This script will ensure that all images built are loaded onto
a KinD cluster with the name specified in the arguments.
The following environment variables are required for configuring this script:
- \$CATALOG_IMG - the tag for the catalog image that contains the registry+v1 bundle.
- \$E2E_TEST_CATALOG_V1 - the tag for the catalog image that contains the registry+v1 bundle.
- \$REG_PKG_NAME - the name of the package for the extension that uses the registry+v1 bundle format.
- \$LOCAL_REGISTRY_HOST - hostname:port of the local docker-registry
setup.sh also takes 5 arguments.

Usage:
setup.sh [OPERATOR_SDK] [CONTAINER_RUNTIME] [KUSTOMIZE] [KIND] [KIND_CLUSTER_NAME] [NAMESPACE]
setup.sh [OPERATOR_SDK] [CONTAINER_RUNTIME] [KUSTOMIZE] [LOCAL_REGISTRY_HOST] [CLUSTER_REGISTRY_HOST]
"

########################################
# Input validation
########################################

if [[ "$#" -ne 6 ]]; then
if [[ "$#" -ne 5 ]]; then
echo "Illegal number of arguments passed"
echo "${help}"
exit 1
fi

if [[ -z "${CATALOG_IMG}" ]]; then
echo "\$CATALOG_IMG is required to be set"
if [[ -z "${E2E_TEST_CATALOG_V1}" ]]; then
echo "\$E2E_TEST_CATALOG_V1 is required to be set"
echo "${help}"
exit 1
fi
Expand All @@ -42,12 +41,6 @@ if [[ -z "${REG_PKG_NAME}" ]]; then
exit 1
fi

if [[ -z "${LOCAL_REGISTRY_HOST}" ]]; then
echo "\$LOCAL_REGISTRY_HOST is required to be set"
echo "${help}"
exit 1
fi

########################################
# Setup temp dir and local variables
########################################
Expand All @@ -64,15 +57,25 @@ mkdir -p "${REG_DIR}"
operator_sdk=$1
container_tool=$2
kustomize=$3
kind=$4
kcluster_name=$5
namespace=$6
# The path we use to push the image from _outside_ the cluster
local_registry_host=$4
# The path we use _inside_ the cluster
cluster_registry_host=$5

tls_flag=""
if [[ "$container_tool" == "podman" ]]; then
echo "Using podman container runtime; adding tls disable flag"
tls_flag="--tls-verify=false"
fi

catalog_push_tag="${local_registry_host}/${E2E_TEST_CATALOG_V1}"
reg_pkg_name="${REG_PKG_NAME}"

reg_img="${DOMAIN}/registry:v0.0.1"
reg_bundle_img="${LOCAL_REGISTRY_HOST}/bundles/registry-v1/registry-bundle:v0.0.1"
reg_bundle_path="bundles/registry-v1/registry-bundle:v0.0.1"

catalog_img="${CATALOG_IMG}"
reg_pkg_name="${REG_PKG_NAME}"
reg_bundle_img="${cluster_registry_host}/${reg_bundle_path}"
reg_bundle_push_tag="${local_registry_host}/${reg_bundle_path}"

########################################
# Create the registry+v1 based extension
Expand All @@ -84,7 +87,7 @@ reg_pkg_name="${REG_PKG_NAME}"
# NOTE: This is a rough edge that users will experience

# The Makefile in the project scaffolded by operator-sdk uses an SDK binary
# in the path path if it is present. Override via `export` to ensure we use
# in the path if it is present. Override via `export` to ensure we use
# the same version that we scaffolded with.
# NOTE: this is a rough edge that users will experience

Expand All @@ -102,7 +105,8 @@ reg_pkg_name="${REG_PKG_NAME}"
make docker-build IMG="${reg_img}" && \
sed -i -e 's/$(OPERATOR_SDK) generate kustomize manifests -q/$(OPERATOR_SDK) generate kustomize manifests -q --interactive=false/g' Makefile && \
make bundle IMG="${reg_img}" VERSION=0.0.1 && \
make bundle-build BUNDLE_IMG="${reg_bundle_img}"
make bundle-build BUNDLE_IMG="${reg_bundle_push_tag}"
${container_tool} push ${reg_bundle_push_tag} ${tls_flag}
)

###############################
Expand Down Expand Up @@ -149,107 +153,5 @@ cat <<EOF > "${TMP_ROOT}"/catalog/index.yaml
}
EOF

# Add a .indexignore to make catalogd ignore
# reading the symlinked ..* files that are created when
# mounting a ConfigMap
cat <<EOF > "${TMP_ROOT}"/catalog/.indexignore
..*
EOF

kubectl create configmap -n "${namespace}" --from-file="${TMP_ROOT}"/catalog.Dockerfile extension-dev-e2e.dockerfile
kubectl create configmap -n "${namespace}" --from-file="${TMP_ROOT}"/catalog extension-dev-e2e.build-contents

kubectl apply -f - << EOF
apiVersion: batch/v1
kind: Job
metadata:
name: kaniko
namespace: "${namespace}"
spec:
template:
spec:
containers:
- name: kaniko
image: gcr.io/kaniko-project/executor:latest
args: ["--dockerfile=/workspace/catalog.Dockerfile",
"--context=/workspace/",
"--destination=${catalog_img}",
"--skip-tls-verify"]
volumeMounts:
- name: dockerfile
mountPath: /workspace/
- name: build-contents
mountPath: /workspace/catalog/
restartPolicy: Never
volumes:
- name: dockerfile
configMap:
name: extension-dev-e2e.dockerfile
items:
- key: catalog.Dockerfile
path: catalog.Dockerfile
- name: build-contents
configMap:
name: extension-dev-e2e.build-contents
EOF

kubectl wait --for=condition=Complete -n "${namespace}" jobs/kaniko --timeout=60s

# Make sure all files are removable. This is necessary because
# the Makefiles generated by the Operator-SDK have targets
# that install binaries under the bin/ directory. Those binaries
# don't have write permissions so they can't be removed unless
# we ensure they have the write permissions
chmod -R +w "${REG_DIR}/bin"

# Load the bundle image into the docker-registry

kubectl create configmap -n "${namespace}" --from-file="${REG_DIR}/bundle.Dockerfile" operator-controller-e2e-${reg_pkg_name}.root

tgz="${REG_DIR}/manifests.tgz"
tar czf "${tgz}" -C "${REG_DIR}" bundle
kubectl create configmap -n "${namespace}" --from-file="${tgz}" operator-controller-${reg_pkg_name}.manifests

kubectl apply -f - << EOF
apiVersion: batch/v1
kind: Job
metadata:
name: "kaniko-${reg_pkg_name}"
namespace: "${namespace}"
spec:
template:
spec:
initContainers:
- name: copy-manifests
image: busybox
command: ['sh', '-c', 'cp /manifests-data/* /manifests']
volumeMounts:
- name: manifests
mountPath: /manifests
- name: manifests-data
mountPath: /manifests-data
containers:
- name: kaniko
image: gcr.io/kaniko-project/executor:latest
args: ["--dockerfile=/workspace/bundle.Dockerfile",
"--context=tar:///workspace/manifests/manifests.tgz",
"--destination=${reg_bundle_img}",
"--skip-tls-verify"]
volumeMounts:
- name: dockerfile
mountPath: /workspace/
- name: manifests
mountPath: /workspace/manifests/
restartPolicy: Never
volumes:
- name: dockerfile
configMap:
name: operator-controller-e2e-${reg_pkg_name}.root
- name: manifests
emptyDir: {}
- name: manifests-data
configMap:
name: operator-controller-${reg_pkg_name}.manifests
EOF

kubectl wait --for=condition=Complete -n "${namespace}" jobs/kaniko-${reg_pkg_name} --timeout=60s
${container_tool} build -f "${TMP_ROOT}/catalog.Dockerfile" -t "${catalog_push_tag}" "${TMP_ROOT}/"
${container_tool} push ${catalog_push_tag} ${tls_flag}
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to ensure that I understand it, do we need to push the catalog image for a registry build locally, because in OCP we cannot do a kind load image, am I right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using a registry vs kind load is theoretically faster since kind load will transfer all image layers each time it's run, while the registry can reuse them. It's still an option, but I think the registry is probably more feature-rich.