Skip to content

Add KEEP_ON_FAILURE flag to e2e kind tests #832

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
istio-testing merged 1 commit into from
May 29, 2025
Merged

Add KEEP_ON_FAILURE flag to e2e kind tests #832

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
5 changes: 5 additions & 0 deletions Makefile.core.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,11 @@ DOCKER_BUILD_FLAGS ?= "--platform=$(TARGET_OS)/$(TARGET_ARCH)"
GOTEST_FLAGS := $(if $(VERBOSE),-v) $(if $(COVERAGE),-coverprofile=$(REPO_ROOT)/out/coverage-unit.out)
GINKGO_FLAGS ?= $(if $(VERBOSE),-v) $(if $(CI),--no-color) $(if $(COVERAGE),-coverprofile=coverage-integration.out -coverpkg=./... --output-dir=out)

# Fail fast when keeping the environment on failure, to make sure we don't contaminate it with other resources.
ifeq ($(KEEP_ON_FAILURE),true)
GINKGO_FLAGS += --fail-fast
endif

# CHANNELS define the bundle channels used in the bundle.
# Add a new line here if you would like to change its default config. (E.g CHANNELS = "candidate,fast,stable")
# To re-generate a bundle for other specific channels without changing the standard setup, you can:
Expand Down
1 change: 1 addition & 0 deletions tests/e2e/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ The following environment variables define the behavior of the test run:
* CONTROL_PLANE_NS=istio-system - The namespace where the control plane will be deployed.
* DEPLOYMENT_NAME=sail-operator - The name of the operator deployment.
* EXPECTED_REGISTRY=`^docker\.io|^gcr\.io` - Which image registry should the operand images come from. Useful for downstream tests.
* KEEP_ON_FAILURE - If set to true, when using a local KIND cluster, don't clean it up when the test fails. This allows to debug the failure.

### Customizing the test run

Expand Down
1 change: 1 addition & 0 deletions tests/e2e/ambient/ambient_suite_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ var (
skipDeploy = env.GetBool("SKIP_DEPLOY", false)
expectedRegistry = env.Get("EXPECTED_REGISTRY", "^docker\\.io|^gcr\\.io")
multicluster = env.GetBool("MULTICLUSTER", false)
keepOnFailure = env.GetBool("KEEP_ON_FAILURE", false)

k kubectl.Kubectl
)
Expand Down
19 changes: 16 additions & 3 deletions tests/e2e/ambient/ambient_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,10 @@ spec:
})

AfterAll(func(ctx SpecContext) {
if CurrentSpecReport().Failed() && keepOnFailure {
return
}

Comment on lines +280 to +283
Copy link
Copy Markdown
Contributor

@fjglira fjglira May 27, 2025

Choose a reason for hiding this comment

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

I like this overall, but it will be better to centralize the logic in one place. I mean, it will be harder to maintain if we need to make a change, and we have the same logic for each test case. Maybe centralizing everything in one place, for example, util.cleanup() or something similar, and adding the logic to skip cleanup, log the failures, and all the cleanup for the resources could be a better way to keep it more simple and avoid duplication of code in several tc. wdyt?

Copy link
Copy Markdown
Contributor

@fjglira fjglira May 27, 2025

Choose a reason for hiding this comment

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

The improvement of the cleanup already has a different issue: #833. So, no need to be addressed on this PR. Centralizing the deletion of the resources, I think, will be good to avoid the kind of errors described in the issue

By("Deleting the pods")
Expect(k.DeleteNamespace(common.HttpbinNamespace, common.SleepNamespace)).
To(Succeed(), "Failed to delete namespaces")
Expand Down Expand Up @@ -334,6 +338,9 @@ spec:
if CurrentSpecReport().Failed() {
common.LogDebugInfo(common.Ambient, k)
debugInfoLogged = true
if keepOnFailure {
return
}
}

By("Cleaning up the Istio namespace")
Expand All @@ -348,9 +355,15 @@ spec:
})

AfterAll(func() {
if CurrentSpecReport().Failed() && !debugInfoLogged {
common.LogDebugInfo(common.Ambient, k)
debugInfoLogged = true
if CurrentSpecReport().Failed() {
if !debugInfoLogged {
common.LogDebugInfo(common.Ambient, k)
debugInfoLogged = true
}

if keepOnFailure {
return
}
}

if skipDeploy {
Expand Down
1 change: 1 addition & 0 deletions tests/e2e/controlplane/control_plane_suite_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ var (
expectedRegistry = env.Get("EXPECTED_REGISTRY", "^docker\\.io|^gcr\\.io")
sampleNamespace = env.Get("SAMPLE_NAMESPACE", "sample")
multicluster = env.GetBool("MULTICLUSTER", false)
keepOnFailure = env.GetBool("KEEP_ON_FAILURE", false)
ipFamily = env.Get("IP_FAMILY", "ipv4")

k kubectl.Kubectl
Expand Down
19 changes: 16 additions & 3 deletions tests/e2e/controlplane/control_plane_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,10 @@ spec:
})

AfterAll(func(ctx SpecContext) {
if CurrentSpecReport().Failed() && keepOnFailure {
return
}

By("Deleting sample")
Expect(k.DeleteNamespace(sampleNamespace)).To(Succeed(), "sample namespace failed to be deleted")
Success("sample deleted")
Expand Down Expand Up @@ -281,6 +285,9 @@ spec:
if CurrentSpecReport().Failed() {
common.LogDebugInfo(common.ControlPlane, k)
debugInfoLogged = true
if keepOnFailure {
return
}
}

By("Cleaning up the Istio namespace")
Expand All @@ -294,9 +301,15 @@ spec:
})

AfterAll(func() {
if CurrentSpecReport().Failed() && !debugInfoLogged {
common.LogDebugInfo(common.ControlPlane, k)
debugInfoLogged = true
if CurrentSpecReport().Failed() {
if !debugInfoLogged {
common.LogDebugInfo(common.ControlPlane, k)
debugInfoLogged = true
}

if keepOnFailure {
return
}
}
})
})
Expand Down
15 changes: 12 additions & 3 deletions tests/e2e/controlplane/control_plane_update_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,9 @@ spec:
if CurrentSpecReport().Failed() {
common.LogDebugInfo(common.ControlPlane, k)
debugInfoLogged = true
if keepOnFailure {
return
}
}

By("Cleaning up sample namespace")
Expand All @@ -324,9 +327,15 @@ spec:
})

AfterAll(func() {
if CurrentSpecReport().Failed() && !debugInfoLogged {
common.LogDebugInfo(common.ControlPlane, k)
debugInfoLogged = true
if CurrentSpecReport().Failed() {
if !debugInfoLogged {
common.LogDebugInfo(common.ControlPlane, k)
debugInfoLogged = true

if keepOnFailure {
return
}
}
}
})
})
Expand Down
1 change: 1 addition & 0 deletions tests/e2e/dualstack/dualstack_suite_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ var (
skipDeploy = env.GetBool("SKIP_DEPLOY", false)
expectedRegistry = env.Get("EXPECTED_REGISTRY", "^docker\\.io|^gcr\\.io")
multicluster = env.GetBool("MULTICLUSTER", false)
keepOnFailure = env.GetBool("KEEP_ON_FAILURE", false)
ipFamily = env.Get("IP_FAMILY", "ipv4")

k kubectl.Kubectl
Expand Down
19 changes: 16 additions & 3 deletions tests/e2e/dualstack/dualstack_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,10 @@ spec:
})

AfterAll(func(ctx SpecContext) {
if CurrentSpecReport().Failed() && keepOnFailure {
return
}

By("Deleting the pods")
Expect(k.DeleteNamespace(DualStackNamespace, IPv4Namespace, IPv6Namespace, SleepNamespace)).
To(Succeed(), "Failed to delete namespaces")
Expand Down Expand Up @@ -272,6 +276,9 @@ spec:
if CurrentSpecReport().Failed() {
common.LogDebugInfo(common.DualStack, k)
debugInfoLogged = true
if keepOnFailure {
return
}
}

By("Cleaning up the Istio namespace")
Expand All @@ -285,9 +292,15 @@ spec:
})

AfterAll(func() {
if CurrentSpecReport().Failed() && !debugInfoLogged {
common.LogDebugInfo(common.DualStack, k)
debugInfoLogged = true
if CurrentSpecReport().Failed() {
if !debugInfoLogged {
common.LogDebugInfo(common.DualStack, k)
debugInfoLogged = true
}

if keepOnFailure {
return
}
}

if skipDeploy {
Expand Down
22 changes: 22 additions & 0 deletions tests/e2e/integ-suite-kind.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export MULTICLUSTER="${MULTICLUSTER:-false}"
export IP_FAMILY="${IP_FAMILY:-ipv4}"
export ISTIOCTL="${ISTIOCTL:-${ROOT}/bin/istioctl}"
export KIND_CLUSTER_NAME="${KIND_CLUSTER_NAME:-operator-integration-tests}"
export KEEP_ON_FAILURE="${KEEP_ON_FAILURE:-}"

function check_prerequisites() {
if ! command -v "${ISTIOCTL}" &> /dev/null; then
Expand All @@ -41,6 +42,22 @@ function run_integration_tests() {
fi
}

function keep_on_failure() {
if [ $? -eq 0 ]; then
original_trap="${original_trap#*\'}"
eval "${original_trap%\'*}"
exit 0
fi

KUBECONFIG="${ARTIFACTS}/config"
if [ "${MULTICLUSTER}" == "true" ]; then
printf -v KUBECONFIG '%s:' "${KUBECONFIGS[@]}"
fi

echo "The kind cluster has been kept due to a test failure."
echo "To access it use \`export KUBECONFIG=${KUBECONFIG%:}\`"
}

check_prerequisites

source "${ROOT}/tests/e2e/setup/setup-kind.sh"
Expand All @@ -50,4 +67,9 @@ export HUB="${KIND_REGISTRY}"
# Workaround make inside make: ovewrite this variable so it is not recomputed in Makefile.core.mk
export IMAGE="${HUB}/${IMAGE_BASE:-sail-operator}:${TAG:-latest}"

if [ "${KEEP_ON_FAILURE}" == "true" ]; then
original_trap="$(trap -p EXIT)"
trap keep_on_failure EXIT
fi

run_integration_tests
15 changes: 12 additions & 3 deletions tests/e2e/multicluster/multicluster_externalcontrolplane_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,9 @@ spec:
if CurrentSpecReport().Failed() {
common.LogDebugInfo(common.MultiCluster, k1, k2)
debugInfoLogged = true
if keepOnFailure {
return
}
}

// Delete namespaces to ensure clean up for new tests iteration
Expand All @@ -496,9 +499,15 @@ spec:
})

AfterAll(func(ctx SpecContext) {
if CurrentSpecReport().Failed() && !debugInfoLogged {
common.LogDebugInfo(common.MultiCluster, k1, k2)
debugInfoLogged = true
if CurrentSpecReport().Failed() {
if !debugInfoLogged {
common.LogDebugInfo(common.MultiCluster, k1, k2)
debugInfoLogged = true
}

if keepOnFailure {
return
}
}

// Delete the Sail Operator from both clusters
Expand Down
15 changes: 12 additions & 3 deletions tests/e2e/multicluster/multicluster_multiprimary_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,9 @@ spec:
if CurrentSpecReport().Failed() {
common.LogDebugInfo(common.MultiCluster, k1, k2)
debugInfoLogged = true
if keepOnFailure {
return
}
}

// Delete namespaces to ensure clean up for new tests iteration
Expand All @@ -362,9 +365,15 @@ spec:
})

AfterAll(func(ctx SpecContext) {
if CurrentSpecReport().Failed() && !debugInfoLogged {
common.LogDebugInfo(common.MultiCluster, k1, k2)
debugInfoLogged = true
if CurrentSpecReport().Failed() {
if !debugInfoLogged {
common.LogDebugInfo(common.MultiCluster, k1, k2)
debugInfoLogged = true
}

if keepOnFailure {
return
}
}

// Delete the Sail Operator from both clusters
Expand Down
15 changes: 12 additions & 3 deletions tests/e2e/multicluster/multicluster_primaryremote_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,9 @@ spec:
if CurrentSpecReport().Failed() {
common.LogDebugInfo(common.MultiCluster, k1, k2)
debugInfoLogged = true
if keepOnFailure {
return
}
}

// Delete namespaces to ensure clean up for new tests iteration
Expand Down Expand Up @@ -400,9 +403,15 @@ spec:
})

AfterAll(func(ctx SpecContext) {
if CurrentSpecReport().Failed() && !debugInfoLogged {
common.LogDebugInfo(common.MultiCluster, k1, k2)
debugInfoLogged = true
if CurrentSpecReport().Failed() {
if !debugInfoLogged {
common.LogDebugInfo(common.MultiCluster, k1, k2)
debugInfoLogged = true
}

if keepOnFailure {
return
}
}

// Delete the Sail Operator from both clusters
Expand Down
1 change: 1 addition & 0 deletions tests/e2e/multicluster/multicluster_suite_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ var (
image = env.Get("IMAGE", "quay.io/maistra-dev/sail-operator:latest")
skipDeploy = env.GetBool("SKIP_DEPLOY", false)
multicluster = env.GetBool("MULTICLUSTER", false)
keepOnFailure = env.GetBool("KEEP_ON_FAILURE", false)
kubeconfig = env.Get("KUBECONFIG", "")
kubeconfig2 = env.Get("KUBECONFIG2", "")
artifacts = env.Get("ARTIFACTS", "/tmp/artifacts")
Expand Down
1 change: 1 addition & 0 deletions tests/e2e/multicontrolplane/multi_control_plane_suite_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ var (
appNamespace2a = env.Get("APP_NAMESPACE2A", "app2a")
appNamespace2b = env.Get("APP_NAMESPACE2B", "app2b")
multicluster = env.GetBool("MULTICLUSTER", false)
keepOnFailure = env.GetBool("KEEP_ON_FAILURE", false)
ipFamily = env.Get("IP_FAMILY", "ipv4")

k kubectl.Kubectl
Expand Down
16 changes: 13 additions & 3 deletions tests/e2e/multicontrolplane/multi_control_plane_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,10 @@ spec:
})

AfterAll(func() {
if CurrentSpecReport().Failed() && keepOnFailure {
return
}

By("Cleaning up the application namespaces")
Expect(k.DeleteNamespace(appNamespace1, appNamespace2a, appNamespace2b)).To(Succeed())

Expand All @@ -188,9 +192,15 @@ spec:
})

AfterAll(func() {
if CurrentSpecReport().Failed() && !debugInfoLogged {
common.LogDebugInfo(common.MultiControlPlane, k)
debugInfoLogged = true
if CurrentSpecReport().Failed() {
if !debugInfoLogged {
common.LogDebugInfo(common.MultiControlPlane, k)
debugInfoLogged = true
}

if keepOnFailure {
return
}
}

if skipDeploy {
Expand Down
7 changes: 7 additions & 0 deletions tests/e2e/operator/operator_install_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,10 @@ spec:
})

AfterAll(func() {
if CurrentSpecReport().Failed() && keepOnFailure {
return
}

Expect(k.DeleteNamespace(curlNamespace)).To(Succeed(), "failed to delete curl namespace")
exec.Command("kubectl", "delete", "--ignore-not-found", "clusterrolebinding", "metrics-reader-rolebinding").Run()

Expand All @@ -198,6 +202,9 @@ spec:
AfterAll(func() {
if CurrentSpecReport().Failed() {
common.LogDebugInfo(common.Operator, k)
if keepOnFailure {
return
}
}

if skipDeploy {
Expand Down
1 change: 1 addition & 0 deletions tests/e2e/operator/operator_suite_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ var (
deploymentName = env.Get("DEPLOYMENT_NAME", "sail-operator")
serviceAccountName = deploymentName
multicluster = env.GetBool("MULTICLUSTER", false)
keepOnFailure = env.GetBool("KEEP_ON_FAILURE", false)
curlNamespace = "curl-metrics"

k kubectl.Kubectl
Expand Down