Run feature tests in dynamic Jenkins workers #611

Sunnatillo · 2023-12-21T20:17:22Z

This PR makes feature tests run directly in dynamic workers instead of using worker as a jumphost

Sunnatillo · 2023-12-30T22:41:17Z

/test-ubuntu-e2e-feature-main

Sunnatillo · 2023-12-30T22:53:16Z

/test-ubuntu-e2e-feature-main

Sunnatillo · 2024-01-02T08:03:25Z

/test-centos-e2e-feature-main

Sunnatillo · 2024-01-02T08:40:03Z

/test-centos-e2e-feature-main
/test-centos-e2e-feature-release-1-5

Sunnatillo · 2024-01-02T13:27:50Z

/test-centos-e2e-feature-main

Sunnatillo · 2024-01-02T14:01:13Z

/test-ubuntu-e2e-feature-release-1-3

Sunnatillo · 2024-01-02T14:12:34Z

/test-ubuntu-e2e-feature-release-1-5

Sunnatillo · 2024-01-02T14:24:10Z

/test-ubuntu-e2e-feature-release-1-5

Sunnatillo · 2024-01-02T14:24:33Z

/test-centos-e2e-feature-release-1-5

Sunnatillo · 2024-01-02T16:31:15Z

/test-ubuntu-integration-main
/test-centos-e2e-feature-release-1-5

Sunnatillo · 2024-01-02T16:43:35Z

/test-ubuntu-e2e-feature-release-1-5

Sunnatillo · 2024-01-02T21:08:56Z

/test-centos-e2e-feature-main
/test-ubuntu-e2e-feature-main

Sunnatillo · 2024-01-02T23:30:55Z

/test-centos-e2e-feature-main
/test-ubuntu-e2e-feature-main

Sunnatillo · 2024-01-03T05:59:57Z

/test-centos-e2e-feature-main
/test-ubuntu-e2e-feature-main

Rozzii · 2024-01-05T06:12:54Z

/test-ubuntu-integration-main

lentzi90 · 2024-01-08T08:27:56Z

jenkins/scripts/dynamic_worker_workflow/fetch_logs.sh

+fi
+
+# Fetch k8s logs form target cluster
+target_config=$(sudo find /tmp/ -type f -name "kubeconfig*")


Is this file cleaned up between runs? Otherwise we could end up with multiple kubeconfigs!

We are using dynamic vm only for one test, then it is deleted.

lentzi90 · 2024-01-08T08:28:09Z

jenkins/scripts/dynamic_worker_workflow/fetch_logs.sh

+# Fetch k8s logs form target cluster
+target_config=$(sudo find /tmp/ -type f -name "kubeconfig*")
+if [[ -n "${target_config}" ]]; then
+    #fetch target cluster k8s logs


nit

Suggested change

#fetch target cluster k8s logs

# fetch target cluster k8s logs

lentzi90

/approve

tuminoid

Small things to fix and some nits.

tuminoid · 2024-01-08T08:51:51Z

jenkins/scripts/dynamic_worker_workflow/feature_tests.sh

+fi
+cd ../
+
+if [[ ( "${REPO_NAME}" == "metal3-dev-env") ||


Suggested change

if [[ ( "${REPO_NAME}" == "metal3-dev-env") ||

if [[ "${REPO_NAME}" == "metal3-dev-env" ]] || [[ "${REPO_NAME}" == "cluster-api-provider-metal3") ]]; then

One condition per [[ .. ]] and no parethesis please.

tuminoid · 2024-01-08T08:54:08Z

jenkins/scripts/dynamic_worker_workflow/fetch_logs.sh

+fetch_k8s_logs "management_cluster" "/home/metal3ci/.kube/config"
+
+# Fetch Ironic containers logs before pivoting to the target cluster, if they exist
+if [ -d /tmp/"${CONTAINER_RUNTIME}" ] && [ "$(ls /tmp/"${CONTAINER_RUNTIME}"/)" ]; then


Suggested change

if [ -d /tmp/"${CONTAINER_RUNTIME}" ] && [ "$(ls /tmp/"${CONTAINER_RUNTIME}"/)" ]; then

if [[ -d /tmp/"${CONTAINER_RUNTIME}" ]] && [[ -n "$(ls /tmp/"${CONTAINER_RUNTIME}"/)" ]]; then

Not exactly sure what the latter condition is expected to do, but assuming you're trying to check if its empty?

tuminoid · 2024-01-08T08:56:28Z

jenkins/scripts/dynamic_worker_workflow/fetch_logs.sh

+    mkdir -p "${LOGS_DIR}/e2e_artifacts"
+    # only if we triggered the e2e from the capm3 repo it will be cloned under tested_repo
+    # else it is under metal3
+    if [[ -d "/home/metal3ci/tested_repo/_artifacts" ]]; then


Quite a bit of repetition for the home dir / tested_repo dir, could we use ${HOME} or other variable to avoid repetition?

tuminoid · 2024-01-08T08:58:08Z

jenkins/scripts/dynamic_worker_workflow/fetch_logs.sh

+    mkdir -p "${LOGS_DIR}/${dir_name}"
+    for NAMESPACE in ${NAMESPACES}; do
+        mkdir -p "${LOGS_DIR}/${dir_name}/${NAMESPACE}"
+        PODS="$(kubectl --kubeconfig="${kconfig}" get pods -n "${NAMESPACE}" -o jsonpath='{.items[*].metadata.name}' 2> /dev/null)"


Same here, a lot of kubectl --kubeconfig="${kconfig}" repetition. Maybe put that in a var to make this more readable?

jenkins/scripts/dynamic_worker_workflow/fetch_logs.sh

jenkins/scripts/dynamic_worker_workflow/run_clean.sh

Rozzii

/hold
Hold until the nits are fixed.

Sunnatillo · 2024-01-09T08:54:57Z

@tuminoid @Rozzii I have addressed most of the comments. Regarding implementing a function and other improvements, we need to open new PR and fix them here and in other integration tests(static worker workflow) as well.

WDYT?

Sunnatillo · 2024-01-09T08:57:00Z

/test-centos-e2e-feature-main
/test-ubuntu-e2e-feature-main

tuminoid

/lgtm

I'm cool with this. Please follow up with the clean up, so we keep it maintainable.

jenkins/scripts/dynamic_worker_workflow/fetch_logs.sh

Sunnatillo · 2024-01-09T11:04:08Z

/test-centos-e2e-feature-main
/test-ubuntu-e2e-feature-main

Sunnatillo · 2024-01-09T14:52:46Z

/test-ubuntu-integration-main

tuminoid · 2024-01-10T06:05:42Z

/unhold
Nits are fixed. @Rozzii please change your review to approving.

This PR makes feature tests run directly in dynamic workers instead of using worker as a jumphost Signed-off-by: Sunnatillo <[email protected]>

Sunnatillo · 2024-01-10T11:22:04Z

/test-centos-e2e-feature-main
/test-ubuntu-e2e-feature-main
/test-ubuntu-integration-main

tuminoid

/lgtm

Sunnatillo · 2024-01-10T14:24:56Z

/test-ubuntu-e2e-feature-main
/test-ubuntu-e2e-feature-release-1-5

Sunnatillo · 2024-01-10T17:10:59Z

/test-ubuntu-e2e-feature-main

Sunnatillo · 2024-01-10T21:51:14Z

/test-ubuntu-e2e-feature-main

lentzi90 · 2024-01-11T06:02:14Z

/test-ubuntu-e2e-feature-main

metal3-io-bot · 2024-01-11T06:47:17Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lentzi90, Rozzii

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [lentzi90]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

metal3-io-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Dec 21, 2023

Sunnatillo force-pushed the move-feature-tests-to-dynamic-workers/sunnat branch from 1c47dae to 0f47108 Compare December 21, 2023 20:26

Sunnatillo force-pushed the move-feature-tests-to-dynamic-workers/sunnat branch from 0f47108 to 0eb3077 Compare December 30, 2023 22:15

Sunnatillo force-pushed the move-feature-tests-to-dynamic-workers/sunnat branch from 0eb3077 to fb4da27 Compare December 30, 2023 22:52

Sunnatillo changed the title ~~WIP: Move running feature tests to dynamic Jenkins workers~~ Move running feature tests to dynamic Jenkins workers Jan 2, 2024

metal3-io-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 2, 2024

Sunnatillo changed the title ~~Move running feature tests to dynamic Jenkins workers~~ Run feature tests in dynamic Jenkins workers Jan 2, 2024

Sunnatillo force-pushed the move-feature-tests-to-dynamic-workers/sunnat branch from fb4da27 to 6bc6412 Compare January 2, 2024 21:08

lentzi90 reviewed Jan 8, 2024

View reviewed changes

metal3-io-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 8, 2024

tuminoid reviewed Jan 8, 2024

View reviewed changes

Rozzii requested changes Jan 8, 2024

View reviewed changes

metal3-io-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 8, 2024

Sunnatillo force-pushed the move-feature-tests-to-dynamic-workers/sunnat branch 2 times, most recently from cb3d5df to 501ccc9 Compare January 9, 2024 08:25

tuminoid reviewed Jan 9, 2024

View reviewed changes

jenkins/scripts/dynamic_worker_workflow/fetch_logs.sh Show resolved Hide resolved

metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Jan 9, 2024

metal3-io-bot added needs-rebase Indicates that a PR cannot be merged because it has merge conflicts with HEAD. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Jan 10, 2024

Move running feature tests to dynamic Jenkins workers

c20ac15

This PR makes feature tests run directly in dynamic workers instead of using worker as a jumphost Signed-off-by: Sunnatillo <[email protected]>

Sunnatillo force-pushed the move-feature-tests-to-dynamic-workers/sunnat branch from 501ccc9 to c20ac15 Compare January 10, 2024 11:18

metal3-io-bot removed lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates that a PR cannot be merged because it has merge conflicts with HEAD. labels Jan 10, 2024

tuminoid reviewed Jan 10, 2024

View reviewed changes

metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Jan 10, 2024

Rozzii self-requested a review January 11, 2024 06:47

Rozzii approved these changes Jan 11, 2024

View reviewed changes

metal3-io-bot merged commit babfbfb into metal3-io:main Jan 11, 2024
8 checks passed

Sunnatillo deleted the move-feature-tests-to-dynamic-workers/sunnat branch January 11, 2024 08:45

	#fetch target cluster k8s logs
	# fetch target cluster k8s logs

	if [[ ( "${REPO_NAME}" == "metal3-dev-env") \|\|
	if [[ "${REPO_NAME}" == "metal3-dev-env" ]] \|\| [[ "${REPO_NAME}" == "cluster-api-provider-metal3") ]]; then

	if [ -d /tmp/"${CONTAINER_RUNTIME}" ] && [ "$(ls /tmp/"${CONTAINER_RUNTIME}"/)" ]; then
	if [[ -d /tmp/"${CONTAINER_RUNTIME}" ]] && [[ -n "$(ls /tmp/"${CONTAINER_RUNTIME}"/)" ]]; then

Run feature tests in dynamic Jenkins workers #611

Run feature tests in dynamic Jenkins workers #611

Conversation

Sunnatillo commented Dec 21, 2023

Sunnatillo commented Dec 30, 2023

Sunnatillo commented Dec 30, 2023

Sunnatillo commented Jan 2, 2024

Sunnatillo commented Jan 2, 2024

Sunnatillo commented Jan 2, 2024

Sunnatillo commented Jan 2, 2024

Sunnatillo commented Jan 2, 2024

Sunnatillo commented Jan 2, 2024

Sunnatillo commented Jan 2, 2024

Sunnatillo commented Jan 2, 2024

Sunnatillo commented Jan 2, 2024

Sunnatillo commented Jan 2, 2024

Sunnatillo commented Jan 2, 2024

Sunnatillo commented Jan 3, 2024

Rozzii commented Jan 5, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lentzi90 left a comment

Choose a reason for hiding this comment

tuminoid left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Rozzii left a comment

Choose a reason for hiding this comment

Sunnatillo commented Jan 9, 2024

Sunnatillo commented Jan 9, 2024

tuminoid left a comment • edited Loading

Choose a reason for hiding this comment

Sunnatillo commented Jan 9, 2024

Sunnatillo commented Jan 9, 2024

tuminoid commented Jan 10, 2024

Sunnatillo commented Jan 10, 2024

tuminoid left a comment

Choose a reason for hiding this comment

Sunnatillo commented Jan 10, 2024

Sunnatillo commented Jan 10, 2024

Sunnatillo commented Jan 10, 2024

lentzi90 commented Jan 11, 2024

metal3-io-bot commented Jan 11, 2024

tuminoid left a comment •

edited

Loading