Skip to content

Run zenko tests out of cluster#2336

Open
SylvainSenechal wants to merge 6 commits intodevelopment/2.14from
improvement/ZENKO-5201
Open

Run zenko tests out of cluster#2336
SylvainSenechal wants to merge 6 commits intodevelopment/2.14from
improvement/ZENKO-5201

Conversation

@SylvainSenechal
Copy link
Contributor

@SylvainSenechal SylvainSenechal commented Feb 26, 2026

Issue: ZENKO-5201

@bert-e
Copy link
Contributor

bert-e commented Feb 26, 2026

Hello sylvainsenechal,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Available options
name description privileged authored
/after_pull_request Wait for the given pull request id to be merged before continuing with the current one.
/bypass_author_approval Bypass the pull request author's approval
/bypass_build_status Bypass the build and test status
/bypass_commit_size Bypass the check on the size of the changeset TBA
/bypass_incompatible_branch Bypass the check on the source branch prefix
/bypass_jira_check Bypass the Jira issue check
/bypass_peer_approval Bypass the pull request peers' approval
/bypass_leader_approval Bypass the pull request leaders' approval
/approve Instruct Bert-E that the author has approved the pull request. ✍️
/create_pull_requests Allow the creation of integration pull requests.
/create_integration_branches Allow the creation of integration branches.
/no_octopus Prevent Wall-E from doing any octopus merge and use multiple consecutive merge instead
/unanimity Change review acceptance criteria from one reviewer at least to all reviewers
/wait Instruct Bert-E not to run until further notice.
Available commands
name description privileged
/help Print Bert-E's manual in the pull request.
/status Print Bert-E's current status in the pull request TBA
/clear Remove all comments from Bert-E from the history TBA
/retry Re-start a fresh build TBA
/build Re-start a fresh build TBA
/force_reset Delete integration branches & pull requests, and restart merge process from the beginning.
/reset Try to remove integration branches unless there are commits on them which do not appear on the source branch.

Status report is not available.

@bert-e
Copy link
Contributor

bert-e commented Feb 26, 2026

Incorrect fix version

The Fix Version/s in issue ZENKO-5201 contains:

  • None

Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:

  • 2.14.0

Please check the Fix Version/s of ZENKO-5201, or the target
branch of this pull request.

@SylvainSenechal SylvainSenechal force-pushed the improvement/ZENKO-5201 branch 5 times, most recently from d6a0f4c to 245e27d Compare February 26, 2026 22:10
@SylvainSenechal SylvainSenechal force-pushed the improvement/ZENKO-5201 branch from 77b703d to 8d5ede4 Compare March 20, 2026 11:47
@SylvainSenechal SylvainSenechal force-pushed the improvement/ZENKO-5201 branch from 8d5ede4 to 18c02d8 Compare March 23, 2026 09:26
@SylvainSenechal SylvainSenechal force-pushed the improvement/ZENKO-5201 branch from 18c02d8 to 733afdf Compare March 23, 2026 11:51
)
done

(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No need for this anymore as we don't need the container, and config is handled differently

working-directory: ./.github/scripts/end2end
- name: Linting
shell: bash
run: bash run-e2e-test.sh "end2end" ${E2E_IMAGE_NAME}:${E2E_IMAGE_TAG} "lint" "default"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No need to do the linting in the deploy action..

  • this deploy action is run multiple times in the ci
  • it was building an image, taking time, all that just for linting
    Now you will see later in end2end.yaml there is a simple yarn run lint

DIR=$(dirname "$0")

# Set up ingress endpoints and /etc/hosts for out-of-cluster access
source "$DIR/configure-e2e-endpoints.sh"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This setup will be common to ctst and end2end tests.
Hopefully we will soon be able to have a common setup for both type of tests, to avoid having to go from configure-ctst to configure e2e, with many things that are common

@@ -0,0 +1,139 @@
#!/bin/bash
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be idempotent. I used it multiple times in Codespace without issues


SERVICE_ACCOUNT="${ZENKO_NAME}-config"
POD_NAME="${ZENKO_NAME}-config"
MANAGEMENT_ENDPOINT="http://${ZENKO_NAME}-management-orbit-api:5001"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the kind of things that we have to change now that the test doesn't run in a pod within the Zenko cluster

Comment on lines +54 to +58
pip3 install --break-system-packages -r "$ZENKO_TESTS_DIR/requirements.txt"

cd "$ZENKO_TESTS_DIR"

envsubst < e2e-config.yaml.template > e2e-config.yaml
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This used to be setup in the Dockerfile that I deleted

@@ -0,0 +1,130 @@
#!/usr/bin/env bash
Copy link
Contributor Author

@SylvainSenechal SylvainSenechal Mar 23, 2026

Choose a reason for hiding this comment

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

I came up with this to deal with the env variables.
It's a bit tricky as I wanted it to work for both github ci and codespace

Variables setup here are pretty much the same as the ones that were setup in the (now deleted) run-e2e-test.sh file

- name: Deploy second Zenko for PRA
run: bash deploy-zenko.sh end2end-pra default './configs/zenko.yaml'
env:
ZENKO_MONGODB_DATABASE: pradb
Copy link
Contributor Author

Choose a reason for hiding this comment

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

setup in prepare-pra.sh directly

cache-from: type=gha,scope=kafka-connect-${{ env.KAFKA_CONNECT_TAG }}
cache-to: type=gha,mode=max,scope=kafka-connect-${{ env.KAFKA_CONNECT_TAG }}

build-test-image:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No more image, just a new lint step as I removed it from the deploy action

working-directory: ./.github/scripts/end2end
run: |
source .github/scripts/end2end/setup-e2e-env.sh
yarn run test_operator
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I prefer this, simple, direct use of yarn commands instead of going through scripts with FOUR arguments 🧐

I just don't really like having to source the env var for each run so if you have suggestion I'm open

Copy link
Contributor

Choose a reason for hiding this comment

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

you can instead add a first step which updates the env for followup steps

c.f. https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-commands#setting-an-environment-variable

RoleSessionName='end2end',
WebIdentityToken=token,
DurationSeconds=60 * 60 * 12, # 12 hrs
DurationSeconds=60 * 60 * 12, # 12 hrs (max allowed by STS for assume role)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanna keep this comment, I tried to use 30 days because in my codespace it was expiring after 12h, but when I used 30 days it broke because the api only accepts 12h

"mocha": "^10.0.0",
"mocha-junit-reporter": "^2.2.1",
"mocha-multi-reporters": "^1.1.7",
"mocha-tags": "^1.0.1",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • Never used,
  • Could be useful in theory,
  • Will never be used in practice, so just add again if needed (will never be needed 🌚 )

"test_operator": "mocha --exit -t 10000 --reporter mocha-multi-reporters --reporter-options configFile=config.json,cmrOutput=mocha-junit-reporter+testsuitesTitle+test_operator ./init_test.js",
"test_smoke": "mocha --exit -t 10000 --reporter mocha-multi-reporters --reporter-options configFile=config.json,cmrOutput=mocha-junit-reporter+testsuitesTitle+test_smoke --recursive smoke_tests",
"test_iam_policies": "mocha --exit -t 15000 --reporter mocha-multi-reporters --reporter-options configFile=config.json,cmrOutput=mocha-junit-reporter+testsuitesTitle+test_iam_policies --recursive iam_policies",
"test_all_extensions": "run-p --aggregate-output test_crr test_aws_crr test_expiration test_transition test_ingestion_oob_s3c",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Btw, I just had a look at end2end-sharded. The step "Run backbeat end to end tests", which is running test_all_extensions, now runs within ~12 min, against ~28min before.

I had to double check, as I thought some tests were simply not run, but we run the same tests. I think previously the parallel run was not working, maybe because of some weird stuff happening when this command is run in a pod, the pod can't run stuff in parallel or something.

Copy link
Contributor

Choose a reason for hiding this comment

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

it seems this step perform a yarn install: by having it running directly in the runner, it will now be cached (automatically!) by setup-node, and would thus be much faster?

@scality scality deleted a comment from bert-e Mar 23, 2026
@bert-e
Copy link
Contributor

bert-e commented Mar 23, 2026

Incorrect fix version

The Fix Version/s in issue ZENKO-5201 contains:

  • None

Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:

  • 2.14.2

Please check the Fix Version/s of ZENKO-5201, or the target
branch of this pull request.

@SylvainSenechal SylvainSenechal requested review from a team, delthas, francoisferrand and maeldonn and removed request for maeldonn March 23, 2026 14:41
@SylvainSenechal SylvainSenechal marked this pull request as ready for review March 23, 2026 14:41
else
trap "kill ${_MONGO_PF_PID} 2>/dev/null || true" EXIT
fi
sleep 2
Copy link
Contributor

Choose a reason for hiding this comment

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

why sleep?

Comment on lines +44 to +51
if [[ "${BASH_SOURCE[0]:-}" != "${0}" ]]; then
# Sourced — register cleanup only if not already registered
if [ -z "${_SETUP_E2E_CLEANUP_SET:-}" ]; then
trap "kill ${_MONGO_PF_PID} 2>/dev/null || true" EXIT
export _SETUP_E2E_CLEANUP_SET=1
fi
else
trap "kill ${_MONGO_PF_PID} 2>/dev/null || true" EXIT
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if [[ "${BASH_SOURCE[0]:-}" != "${0}" ]]; then
# Sourced — register cleanup only if not already registered
if [ -z "${_SETUP_E2E_CLEANUP_SET:-}" ]; then
trap "kill ${_MONGO_PF_PID} 2>/dev/null || true" EXIT
export _SETUP_E2E_CLEANUP_SET=1
fi
else
trap "kill ${_MONGO_PF_PID} 2>/dev/null || true" EXIT
if [[ "${BASH_SOURCE[0]:-}" == "${0}" ]]; then
trap "kill ${_MONGO_PF_PID} 2>/dev/null || true" EXIT
elif [ -z "${_SETUP_E2E_CLEANUP_SET:-}" ]; then
# Sourced — register cleanup only if not already registered
trap "kill ${_MONGO_PF_PID} 2>/dev/null || true" EXIT
export _SETUP_E2E_CLEANUP_SET=1
fi

! lsof -i ":${MONGO_PORT}" &>/dev/null; then
kubectl port-forward -n "${MONGO_NS}" "svc/${MONGO_SVC}" "${MONGO_PORT}:${MONGO_PORT}" &
_MONGO_PF_PID=$!
# Clean up on exit only if we started it (not when sourced for interactive use)
Copy link
Contributor

Choose a reason for hiding this comment

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

comment is not correct anymore, seems we now always kill mongo? (i.e. except I already registered)

so ust handling the "double registration" should be enough:

if [ -z "${_SETUP_E2E_CLEANUP_SET:-}" ]; then
    trap "kill ${_MONGO_PF_PID} 2>/dev/null || true" EXIT
    export _SETUP_E2E_CLEANUP_SET=1
fi

Comment on lines +16 to +17
# Auto-source in future Codespace terminals
echo '[ -f "$HOME/.zenko.env" ] && source "$HOME/.zenko.env"' >> "$HOME/.bashrc"
Copy link
Contributor

Choose a reason for hiding this comment

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

should probably be added to the devcontariner's Dockerfile instead

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.

3 participants