Support Jenkins CI #163

abdallahyas · 2021-07-15T12:14:27Z

This patch adds a Jenkins file example, it also modifies the E2E
kind script to support a new mode of operation: the bash service.
The modification was done to support running the scripts in a limited
privilege shell.

abdallahyas · 2021-07-15T12:18:39Z

Another jenkins job is configured to update the admin list. It needs to be run from a PR and can be triggered using /update-admins

abdallahyas · 2021-07-15T12:21:12Z

currently the jobs are disabled, tested them on my fork.

abdallahyas · 2021-07-15T12:51:08Z

@martinkennelly @adrianchiris can you guys PTAL, thanks.

adrianchiris · 2021-07-19T15:12:02Z

Thanks @AbdYsn ! will review this soon

adrianchiris

Thanks for submitting this !

I still need to give vf-netns-switcher.sh a deeper look. provided some comments/questions/nits so we can get the ball rolling :)

im thinking after this we should split the kind cluster setup from tests, this will allow to reuse this for development environment setup as well.

hack/e2e.md

hack/vf-netns-switcher.sh

adrianchiris · 2021-07-28T17:39:05Z

hack/vf-netns-switcher.sh

+        done
+        sleep $TIMEOUT
+    done
+    return $status


will we ever reach this point ?

no, removed it.

adrianchiris · 2021-07-28T17:44:05Z

hack/vf-netns-switcher.sh

+fi
+
+variables_check
+let status=$status+$?


unless i missed something, status is first defined here why do not use $? directly ?

you are correct, done out of habit, fixed it.

hack/vf-netns-switcher.sh

adrianchiris · 2021-10-13T08:00:20Z

doc/testing-kind.md

+
+There are two modes of moving the specified SR-IOV capable PCI net device to the KIND worker namespace:
+
+ * `test-suit` (default): In this mode, the E2E test suit handle the PF and its VFs switching to the test namespace.


maybe test-suite ?

https://en.wikipedia.org/wiki/Test_suite

adrianchiris · 2021-10-13T08:04:37Z

ci/README.md

@@ -0,0 +1,5 @@
+## Jenkins CI configurations and examples
+This folder holds jenkins CI configurations and examples. The configurations currently are only limited to the admin-list. The admin list contains the list of github users and organizations that have permision to trigger the Jenkins CI, it is updated on the PR by commenting `/update-admins` on it by a current admin.


The configurations currently are only limited to the admin-list

what does it mean ? is this information on the examples below ?

can you structure this README so we have:

General informaiton about this folder

Information on admin list - what it is, who should use is, how its updated

information on the examples

information on the format of the build triggers for e2e tests (as presented in NPWG resource mgmt meeting)

Done, can you have a look?

adrianchiris · 2021-10-13T08:05:43Z

doc/testing-kind.md

 ### How to test
+#### `test-suit` SRIOV device switcher


nit: test-suite

Suggested reword : Device netns switcher mode test-suite

adrianchiris · 2021-10-13T08:11:55Z

doc/testing-kind.md

+cp ./hack/vf-switcher.service /etc/systemd/system/
+systemctl daemon-reload
+```
+For the service to work probably the `jq` tool is needed.


nit: probably -> properly

doc/testing-kind.md

adrianchiris · 2021-10-13T12:39:44Z

hack/vf-netns-switcher.sh

+
+get_pci_from_net_name(){
+    local interface_name=$1
+    local worker_netns="${2:-$netns}"


why not just $2 ?

hack/vf-netns-switcher.sh

adrianchiris · 2021-11-03T09:26:34Z

hack/run-e2e-test-kind.sh

-export TEST_NETNS_PATH="${netns_path}"
+if [[ "${INTERFACES_SWITCHER}" == "system-service" ]];then
+    pf="$(ls /sys/bus/pci/devices/${test_pf_pci_addr}/net)"
+    cat <<EOF > /etc/vf-switcher/vf-switcher.yaml


need to make sure directory exists

Getting the following:

## make KinD's sysfs writable (required to create VFs) ## label KinD's control-plane-node as sriov capable node/kind-worker labeled ## label KinD worker as worker node/kind-worker labeled ./hack/run-e2e-test-kind.sh: line 112: /etc/vf-switcher/vf-switcher.yaml: No such file or directory

adrianchiris · 2021-11-03T09:49:10Z

doc/testing-kind.md

+#### Cleaning up the `system-service` service files
+```
+$ sudo rm -f /etc/systemd/system/vf-switcher.service
+$ rm -f /etc/vf-switcher/vf-switcher.yaml


this is deleted in teardown-e2e-kind-cluster.sh so i dont think you need it here

doc/testing-kind.md

adrianchiris · 2021-11-03T09:54:44Z

hack/run-e2e-test-kind.sh

@@ -4,11 +4,34 @@ here="$(dirname "$(readlink --canonicalize "${BASH_SOURCE[0]}")")"
 root="$(readlink --canonicalize "$here/..")"
 export SRIOV_NETWORK_OPERATOR_IMAGE="${SRIOV_NETWORK_OPERATOR_IMAGE:-sriov-network-operator:latest}"
 export SRIOV_NETWORK_CONFIG_DAEMON_IMAGE="${SRIOV_NETWORK_CONFIG_DAEMON_IMAGE:-origin-sriov-network-config-daemon:latest}"
+export KUBECONFIG="${KUBECONFIG:-${HOME}/.kube/config}"
+INTERFACES_SWITCHER="${INTERFACES_SWITCHER:-"test-suit"}"


suit -> suite

adrianchiris · 2021-11-03T09:54:50Z

hack/run-e2e-test-kind.sh

@@ -4,11 +4,34 @@ here="$(dirname "$(readlink --canonicalize "${BASH_SOURCE[0]}")")"
 root="$(readlink --canonicalize "$here/..")"
 export SRIOV_NETWORK_OPERATOR_IMAGE="${SRIOV_NETWORK_OPERATOR_IMAGE:-sriov-network-operator:latest}"
 export SRIOV_NETWORK_CONFIG_DAEMON_IMAGE="${SRIOV_NETWORK_CONFIG_DAEMON_IMAGE:-origin-sriov-network-config-daemon:latest}"
+export KUBECONFIG="${KUBECONFIG:-${HOME}/.kube/config}"
+INTERFACES_SWITCHER="${INTERFACES_SWITCHER:-"test-suit"}"
+SUPPORTED_INTERFACE_SWTICHER_MODES=("test-suit", "system-service")


suit -> suite

adrianchiris · 2021-11-04T11:53:08Z

ci/README.md

@@ -0,0 +1,23 @@
+## CI configurations and examples
+This folder holds vendors CIes configurations and examples. Configurations are used to control the vendors CIes behaviors, and examples are used as a reference for other vendors to be able to setup a CI of their own.


first time seeing "CIes", can you just use CI throughout this file (or CIs)?

done, changed it to Vendors CI

adrianchiris · 2021-11-04T12:05:59Z

ci/README.md

+ * `sub-test`: In case there are many tests for the `test-type`, this field specify what sub-test to run.
+
+In addition to the convention, all vendors CI should be triggered on the general phrase `/test-all`
+


there are a couple of things that are unclear to me.

can you elaborate on the expected CI behaviour when <sub-test> is not provided in test/skip and when both <vendor> and <sub-test> are not provided in test/skip ?

also what is the expected behaviour with skip in general ?

Nothing would happen, essentially a job gets triggered on a specific trigger phrase, if you replace one section with all all the tests for that section will get triggered.
To elaborate, consider a test called e2e-nvidia-cx5, if you want to test it, you should comment one of the following:

/test-e2e-nvidia-cx5

/test-e2e-nvidia-all

/test-e2e-all

/test-all

For the skip, it would be a separate job which gets triggered on the /skip phrases and the same trigger convention as the /test convention above. The job would just exit 0 to have a green report on the PR with the same status-context as the test job.

Please see if my understanding is correct for the /test and /skip usage:

comment /test-e2e-nvidia-all, it runs on all sub-tests (e.g. cx-5 and cx-6).

assume cx-5 sub-test fails, the PR is blocked due to CI failure

then comment skip-e2e-nvidia-cx5, it updates the CI failure to green (success)

the PR is unblocked due to the above skip comment

The sub-test is used to specify the device model, right?

Not quite, the patch should be analyze, and determined whether it should be tested or not against the vendor CI. If it should then the /test is used to trigger the job and merge will be blocked until the CI is green (either by retesting the test, or uploading a new patchset), ideally the CI would only fail because of a change in the patch and logs should be provided to be able to debug why the patch failed.
Only when it is determined that there is no need to test this test against this patch, then the /skip should be used to pass the test.

well it is not just device, it can also be other stuff, say openshift, kind, and baremetal can also be added as a sub-test. A note here is that another github workflow will be added after this patch is merged to comment available CI triggers on PR creation.

I see, thanks for the explanation!
I think we don't define openshift, kind or baremetal yet, right? they are just possible examples, could be well be a different type (platform for example) in the test- convention.

I think we don't define openshift, kind or baremetal yet, right?

right

could be well be a different type (platform for example) in the test- convention

yes

adrianchiris · 2021-11-04T12:10:38Z

hack/teardown-e2e-kind-cluster.sh

@@ -7,3 +7,8 @@ if ! command -v kind &> /dev/null; then
 fi

 kind delete cluster
+if [[ "${INTERFACES_SWITCHER}" == "system-service" ]];then


This will not work if you use --device-netns-switcher

done, can you have a look.

adrianchiris · 2021-11-16T12:36:52Z

testing mellanox CI:

/test-all

adrianchiris · 2021-11-16T15:02:12Z

/test-all

adrianchiris · 2021-11-18T09:23:53Z

/test-all

adrianchiris · 2021-11-18T11:25:11Z

/skip-all

adrianchiris

CI triggers work. and Nvidia CI is running.

zshi-redhat · 2021-11-26T03:05:07Z

/test-e2e-nvidia-cx5

zshi-redhat · 2021-11-26T03:07:16Z

/test-e2e-nvidia-cx5

It seems the CI is not triggered, maybe I need to wait until this PR is merged so that admin list takes effect?

zshi-redhat · 2021-11-26T03:08:15Z

Is there a way to check the jenkins log? I didn't see the Details link in the Nvidia e2e test CI (maybe it is because the test is marked as skipped now)?

Eoghan1232 · 2021-11-26T09:31:26Z

/test-all

Eoghan1232 · 2021-11-26T09:32:28Z

/test-all

I assume as I am not on Admin list, I cannot run Jenkins job?
@AbdYsn

abdallahyas · 2021-11-30T08:45:07Z

/test-e2e-nvidia-cx5

It seems the CI is not triggered, maybe I need to wait until this PR is merged so that admin list takes effect?

That actually because there is no such trigger. currently because there is only a single test, and i did not want to tie it to CX5 devices, i did not add a subtest to job trigger, and it can be trigger using the /test-e2e-nvidia-all trigger. Also since the NVIDIA jenkins instance can not be accessed outside the company, the polling mechanism is used to trigger the jobs against the webhook mechanism. so there is a delay of at most 5 minutes until the trigger will take effet.

abdallahyas · 2021-11-30T08:46:24Z

Is there a way to check the jenkins log? I didn't see the Details link in the Nvidia e2e test CI (maybe it is because the test is marked as skipped now)?

my mistake, forgot to enable them in the CI configuration, will do that shortly

abdallahyas · 2021-11-30T08:53:43Z

/test-all

I assume as I am not on Admin list, I cannot run Jenkins job? @AbdYsn

Correct, If you want to be added to the admin-list, then a patch should be proposed to add you to the admin list, and for you to actually be added, on the proposed patch a current admin should comment /update-admins and then merge the commit. A note here is that there is a daily job that would read the main branch admin-list and update the job admin list accordingly, so the admin list will be overridden once each day to align it with main branch. The other way to update it is to contact the maintainer of the verndor CI and ask him to update it manually.

Eoghan1232 · 2021-11-30T09:01:43Z

/test-all

I assume as I am not on Admin list, I cannot run Jenkins job? @AbdYsn

Correct, If you want to be added to the admin-list, then a patch should be proposed to add you to the admin list, and for you to actually be added, on the proposed patch a current admin should comment /update-admins and then merge the commit. A note here is that there is a daily job that would read the main branch admin-list and update the job admin list accordingly, so the admin list will be overridden once each day to align it with main branch. The other way to update it is to contact the maintainer of the verndor CI and ask him to update it manually.

Ah, I see.
Just to clarify.
Steps:
PR -> adding user to the admins list. (admin-list.yaml)
Admin comments: '/update-admins' before it is merged.
Then merge after.

hack/vf-netns-switcher.sh

Eoghan1232

LGTM.
Thank you for working on this!

zshi-redhat · 2021-11-30T12:08:40Z

/test-e2e-nvidia-all

abdallahyas · 2021-11-30T12:45:06Z

/test-e2e-nvidia-all

abdallahyas · 2021-11-30T12:47:38Z

/test-all

abdallahyas · 2021-11-30T12:49:44Z

/test-all

abdallahyas · 2021-11-30T12:54:47Z

/test-e2e-nvidia-all

abdallahyas · 2021-11-30T12:58:02Z

/test-e2e-nvidia-all

abdallahyas · 2021-11-30T13:40:40Z

/test-e2e-nvidia-all

abdallahyas · 2021-11-30T14:24:11Z

@zshi-redhat can you try triggering it now with any of the supported phrases?

zshi-redhat · 2021-11-30T14:39:39Z

/test-e2e-nvidia-all

This patch adds a Jenkins file example, it also modifies the E2E kind script to support a new mode of operation: the bash service. The modification was done to support running the scripts in a limited privilege shell.

abdallahyas · 2021-12-01T07:35:17Z

/test-all

I assume as I am not on Admin list, I cannot run Jenkins job? @AbdYsn

Correct, If you want to be added to the admin-list, then a patch should be proposed to add you to the admin list, and for you to actually be added, on the proposed patch a current admin should comment /update-admins and then merge the commit. A note here is that there is a daily job that would read the main branch admin-list and update the job admin list accordingly, so the admin list will be overridden once each day to align it with main branch. The other way to update it is to contact the maintainer of the verndor CI and ask him to update it manually.

Ah, I see. Just to clarify. Steps: PR -> adding user to the admins list. (admin-list.yaml) Admin comments: '/update-admins' before it is merged. Then merge after.

@Eoghan1232 Yes, if you want to have the admin list updated immediately.

abdallahyas · 2021-12-01T08:13:45Z

Checking the logs, /test-all

adrianchiris · 2021-12-01T14:28:26Z

@AbdYsn BIG thanks for working on it ! 🚀

adrianchiris reviewed Jul 28, 2021

View reviewed changes

abdallahyas force-pushed the testing-ci branch from 187ad99 to 9b09d80 Compare August 16, 2021 13:59

adrianchiris requested changes Oct 13, 2021

View reviewed changes

abdallahyas force-pushed the testing-ci branch 2 times, most recently from 6aada80 to ca56ec9 Compare October 26, 2021 06:57

adrianchiris reviewed Nov 3, 2021

View reviewed changes

doc/testing-kind.md Show resolved Hide resolved

adrianchiris reviewed Nov 3, 2021

View reviewed changes

abdallahyas force-pushed the testing-ci branch 3 times, most recently from ef2664b to a9a625a Compare November 4, 2021 08:03

adrianchiris reviewed Nov 4, 2021

View reviewed changes

abdallahyas force-pushed the testing-ci branch 2 times, most recently from b60185a to 10974b1 Compare November 4, 2021 12:03

adrianchiris reviewed Nov 4, 2021

View reviewed changes

abdallahyas force-pushed the testing-ci branch 2 times, most recently from b22f5b4 to a3568f4 Compare November 11, 2021 07:59

abdallahyas force-pushed the testing-ci branch from a3568f4 to 6b34130 Compare November 16, 2021 09:13

abdallahyas force-pushed the testing-ci branch from 6b34130 to 9751438 Compare November 18, 2021 08:16

adrianchiris approved these changes Nov 18, 2021

View reviewed changes

zshi-redhat requested a review from Eoghan1232 November 26, 2021 03:06

Eoghan1232 reviewed Nov 30, 2021

View reviewed changes

hack/vf-netns-switcher.sh Show resolved Hide resolved

Eoghan1232 reviewed Nov 30, 2021

View reviewed changes

hack/vf-netns-switcher.sh Show resolved Hide resolved

Eoghan1232 reviewed Nov 30, 2021

View reviewed changes

Support Jenkins CI

1996c00

This patch adds a Jenkins file example, it also modifies the E2E kind script to support a new mode of operation: the bash service. The modification was done to support running the scripts in a limited privilege shell.

abdallahyas force-pushed the testing-ci branch from 9751438 to 1996c00 Compare December 1, 2021 07:30

zshi-redhat approved these changes Dec 1, 2021

View reviewed changes

Eoghan1232 approved these changes Dec 1, 2021

View reviewed changes

adrianchiris merged commit 5802d2f into k8snetworkplumbingwg:master Dec 1, 2021


		There are two modes of moving the specified SR-IOV capable PCI net device to the KIND worker namespace:

		* `test-suit` (default): In this mode, the E2E test suit handle the PF and its VFs switching to the test namespace.

		@@ -0,0 +1,5 @@
		## Jenkins CI configurations and examples
		This folder holds jenkins CI configurations and examples. The configurations currently are only limited to the admin-list. The admin list contains the list of github users and organizations that have permision to trigger the Jenkins CI, it is updated on the PR by commenting `/update-admins` on it by a current admin.

		@@ -0,0 +1,23 @@
		## CI configurations and examples
		This folder holds vendors CIes configurations and examples. Configurations are used to control the vendors CIes behaviors, and examples are used as a reference for other vendors to be able to setup a CI of their own.

		* `sub-test`: In case there are many tests for the `test-type`, this field specify what sub-test to run.

		In addition to the convention, all vendors CI should be triggered on the general phrase `/test-all`

Support Jenkins CI #163

Support Jenkins CI #163

Conversation

abdallahyas commented Jul 15, 2021

abdallahyas commented Jul 15, 2021

abdallahyas commented Jul 15, 2021 • edited Loading

abdallahyas commented Jul 15, 2021

adrianchiris commented Jul 19, 2021

adrianchiris 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

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

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

Choose a reason for hiding this comment

adrianchiris Nov 3, 2021 • edited Loading

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

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

adrianchiris Nov 4, 2021 • edited Loading

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

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

adrianchiris commented Nov 16, 2021

adrianchiris commented Nov 16, 2021

adrianchiris commented Nov 18, 2021

adrianchiris commented Nov 18, 2021

adrianchiris left a comment

Choose a reason for hiding this comment

zshi-redhat commented Nov 26, 2021

zshi-redhat commented Nov 26, 2021

zshi-redhat commented Nov 26, 2021

Eoghan1232 commented Nov 26, 2021

Eoghan1232 commented Nov 26, 2021

abdallahyas commented Nov 30, 2021

abdallahyas commented Nov 30, 2021

abdallahyas commented Nov 30, 2021 • edited Loading

Eoghan1232 commented Nov 30, 2021 • edited Loading

Eoghan1232 left a comment

Choose a reason for hiding this comment

zshi-redhat commented Nov 30, 2021

abdallahyas commented Nov 30, 2021

abdallahyas commented Nov 30, 2021

abdallahyas commented Nov 30, 2021

abdallahyas commented Nov 30, 2021

abdallahyas commented Nov 30, 2021

abdallahyas commented Nov 30, 2021

abdallahyas commented Nov 30, 2021

zshi-redhat commented Nov 30, 2021

abdallahyas commented Dec 1, 2021 • edited Loading

abdallahyas commented Dec 1, 2021

adrianchiris commented Dec 1, 2021

abdallahyas commented Jul 15, 2021 •

edited

Loading

adrianchiris Nov 3, 2021 •

edited

Loading

adrianchiris Nov 4, 2021 •

edited

Loading

abdallahyas commented Nov 30, 2021 •

edited

Loading

Eoghan1232 commented Nov 30, 2021 •

edited

Loading

abdallahyas commented Dec 1, 2021 •

edited

Loading