Changes to csi prow to run e2e tests in sidecars #206

RaunakShah · 2022-09-23T07:02:47Z

Able to run on external-provisioner when tests are added to that repo:

Thu Sep 22 10:20:31 IST 2022 go1.18.3 /Users/raunakshah/go/src/src/github.com/kubernetes-csi/external-provisioner$ env KUBECONFIG=/Users/raunakshah/.kube/config KUBE_TEST_REPO_LIST= ginkgo -v -focus=Test1 /Users/raunakshah/go/src/pkg/csiprow.ProhizqqL5/e2e.test -- -report-dir /Users/raunakshah/go/src/pkg/csiprow.ProhizqqL5/artifacts
Sep 22 10:20:32.938: INFO: The --provider flag is not set. Continuing as if --provider=skeleton had been used.
Running Suite: E2E suite - /Users/raunakshah/go/src/pkg/csiprow.ProhizqqL5
==========================================================================
Random Seed: 1663822232

Will run 1 of 1 specs
------------------------------
Test1 test1
  test-1-A
  /Users/raunakshah/go/src/github.com/kubernetes-csi/external-provisioner/test/e2e/storage/provision.go:13

...

Ran 1 of 1 Specs in 0.034 seconds
SUCCESS! -- 1 Passed | 0 Failed | 0 Pending | 0 Skipped
PASS

Add ability to run e2e tests written in sidecar repos

xing-yang · 2022-10-03T18:41:01Z

/assign @pohly

It's marked as WIP, but please take a look of the general approach.

RaunakShah · 2022-10-04T03:46:56Z

prow.sh

-    run_with_loggers env KUBECONFIG="$KUBECONFIG" KUBE_TEST_REPO_LIST="$(if [ -e "${CSI_PROW_WORK}/e2e-repo-list" ]; then echo "${CSI_PROW_WORK}/e2e-repo-list"; fi)" ginkgo -v "$@" "${CSI_PROW_WORK}/e2e.test" -- -report-dir "${ARTIFACTS}" -storage.testdriver="${CSI_PROW_WORK}/test-driver.yaml"
+    if [ "${name}" == "local" ]; then
+        cd "${GOPATH}/src/${CSI_PROW_SIDECAR_E2E_IMPORT_PATH}" &&
+        run_with_loggers env KUBECONFIG="$KUBECONFIG" KUBE_TEST_REPO_LIST="$(if [ -e "${CSI_PROW_WORK}/e2e-repo-list" ]; then echo "${CSI_PROW_WORK}/e2e-repo-list"; fi)" ginkgo -v "$@" "${CSI_PROW_WORK}/e2e.test" -- -report-dir "${ARTIFACTS}"


NOTE: the -storage.testdriver parameter is not supported in sidecar repos, so it needs a different call to run

pohly · 2022-10-04T06:22:39Z

prow.sh

-        ln -s "${GOPATH}/src/${CSI_PROW_E2E_IMPORT_PATH}/_output/bin/e2e.test" "${CSI_PROW_WORK}" &&
-        run_with_go "$go_version" make WHAT=vendor/github.com/onsi/ginkgo/ginkgo "-C${GOPATH}/src/${CSI_PROW_E2E_IMPORT_PATH}" &&
-        ln -s "${GOPATH}/src/${CSI_PROW_E2E_IMPORT_PATH}/_output/bin/ginkgo" "${CSI_PROW_BIN}"
+    if [ "${CSI_PROW_SIDECAR_E2E_IMPORT_PATH}" != "none" ]; then


This can be replaced with sidecar_tests_enabled, right?

I'm unclear about one thing regarding this PR: is the goal to run either a local E2E suite or the one from Kubernetes, or is the goal to support both in the same job?

Because of this if check here it seems to be the former, because install_e2e can only be used to install one suite, but not both. I think it would be better to support both suites in the same job.

@pohly That makes sense. I've updated the code in the latest commit to run the tests in a single job. My original testing only ran the local tests, so I didn't realise that install_e2e is run only one (although now the if at the start of that function makes more sense :))

@pohly tests running in the sidecar repos would potentially want to enable some feature gates in the container. I think this file would need further changes to support that. If it runs in a single job, the feature gates would be enabled for the Kubernetes tests as well.

Unless of course we undo the change after the local tests are complete

I'm not sure whether it's that particular line. Injecting feature gate flags into the driver deployments seems orthogonal to injecting a locally built image, which is that if block is about.

Okay, I'll need to take a deeper look into where to update the feature flags. Potentially here - https://github.com/kubernetes-csi/csi-release-tools/blob/master/prow.sh#L695 but let me play around with it a bit

Should we move ahead with this PR in the meanwhile? I don't think there's an obvious solution to adding features gates in a particular sidecar container.

We can do that. Can you squash commits and then try out that commit in some sidecar repo where a local E2E suite is defined? In other words, create a test PR in some other repo?

It will fail the unit check for locally modified release-tools, but that's fine.

Created a PR here - kubernetes-csi/external-provisioner#798
The local test is just a skeleton to prove that the tests within that repo will run with these changes.

prow.sh

pohly · 2022-10-06T15:17:20Z

prow.sh

+    else
+        cd "${GOPATH}/src/${CSI_PROW_E2E_IMPORT_PATH}" &&
+        run_with_loggers env KUBECONFIG="$KUBECONFIG" KUBE_TEST_REPO_LIST="$(if [ -e "${CSI_PROW_WORK}/e2e-repo-list" ]; then echo "${CSI_PROW_WORK}/e2e-repo-list"; fi)" ginkgo -v "$@" "${CSI_PROW_WORK}/e2e.test" -- -report-dir "${ARTIFACTS}" -storage.testdriver="${CSI_PROW_WORK}/test-driver.yaml"
+    fi


When both E2E suites write into the same report dir, won't that overwrite files like junit_01.xml?

I think you need to add -report-prefix local to the local branch.

Done. Do you think we can remove this part as well?

KUBE_TEST_REPO_LIST="$(if [ -e "${CSI_PROW_WORK}/e2e-repo-list" ]; then echo "${CSI_PROW_WORK}/e2e-repo-list"; fi)

No, let's keep it. The local test might need the same image remapping as upstream Kubernetes. If it doesn't, then setting KUBE_TEST_REPO_LIST won't hurt.

pohly · 2022-10-06T15:17:57Z

prow.sh

+
+                if sidecar_tests_enabled; then
+                    if ! run_e2e local \
+                         -focus="some regex"; then


"some regex" needs further work, right? 😁

Oops, this was from my testing. But actually I think we don't need any focus for local tests right? Ideally all tests from within the repo should be run.

Usually yes, but there might be exceptions. As this script is going to be used by many different components, it's better to be flexible. Let's introduce a separate variable for it.

Done! Introduced CSI_PROW_SIDECAR_E2E_FOCUS

Please introduce also CSI_PROW_SIDECAR_E2E_SKIP, we often need to skip test for alpha features or disruptive tests.

xing-yang · 2022-10-12T19:47:53Z

@RaunakShah Can you add a release note?

RaunakShah · 2022-10-13T05:16:59Z

Can you add a release note?

Done!

xing-yang · 2022-10-17T17:30:43Z

/assign @jsafrane

pohly · 2022-10-21T09:59:59Z

prow.sh

@@ -264,7 +267,7 @@ configvar CSI_PROW_DEP_VERSION v0.5.1 "golang dep version to be used for vendor
 # thus only makes sense in repos which provide their own CSI
 # driver. Repos can enable sanity testing by setting
 # CSI_PROW_TESTS_SANITY=sanity.
-configvar CSI_PROW_TESTS "unit parallel serial $(if [ "${CSI_PROW_KUBERNETES_VERSION}" = "latest" ]; then echo parallel-alpha serial-alpha; fi) sanity" "tests to run"
+configvar CSI_PROW_TESTS "local unit parallel serial $(if [ "${CSI_PROW_KUBERNETES_VERSION}" = "latest" ]; then echo parallel-alpha serial-alpha; fi) sanity" "tests to run"


Does adding "local" here have any effect? I don't think so. Can you remove it?

pohly

I did another pass and only found one issue ("local" in CSI_PROW_TESTS).

pohly

/lgtm

k8s-ci-robot · 2022-10-26T06:25:23Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pohly, RaunakShah

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~~ [pohly]

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

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Sep 23, 2022

k8s-ci-robot requested review from ggriffiths and j-griffith September 23, 2022 07:02

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 23, 2022

k8s-ci-robot assigned pohly Oct 3, 2022

xing-yang changed the title ~~[WIP] [Do not review] Draft changes to csi prow to run e2e tests in sidecars~~ [WIP] Draft changes to csi prow to run e2e tests in sidecars Oct 3, 2022

RaunakShah commented Oct 4, 2022

View reviewed changes

pohly reviewed Oct 4, 2022

View reviewed changes

pohly suggested changes Oct 6, 2022

View reviewed changes

RaunakShah changed the title ~~[WIP] Draft changes to csi prow to run e2e tests in sidecars~~ Changes to csi prow to run e2e tests in sidecars Oct 10, 2022

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 10, 2022

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Oct 13, 2022

RaunakShah force-pushed the update-prow branch from 8726d46 to deff7e4 Compare October 17, 2022 15:49

k8s-ci-robot assigned jsafrane Oct 17, 2022

RaunakShah mentioned this pull request Oct 18, 2022

Test PR - Do not review kubernetes-csi/external-provisioner#798

Closed

RaunakShah force-pushed the update-prow branch from deff7e4 to f56b9a3 Compare October 18, 2022 08:54

pohly reviewed Oct 21, 2022

View reviewed changes

pohly suggested changes Oct 21, 2022

View reviewed changes

Changes to csi prow to run e2e tests in sidecars

7d410d8

RaunakShah force-pushed the update-prow branch from b36019e to 7d410d8 Compare October 26, 2022 05:13

pohly approved these changes Oct 26, 2022

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 26, 2022

jsafrane mentioned this pull request Nov 11, 2022

release-3.1: update release-tools kubernetes-csi/external-provisioner#812

Merged

jsafrane mentioned this pull request Nov 11, 2022

release-3.2: update release-tools kubernetes-csi/external-provisioner#813

Merged

This was referenced Nov 11, 2022

release-3.3: update release-tools kubernetes-csi/external-provisioner#814

Merged

release-1.3: update release-tools kubernetes-csi/external-resizer#228

Merged

jsafrane mentioned this pull request Nov 11, 2022

release-1.4: update release-tools kubernetes-csi/external-resizer#229

Merged

jsafrane mentioned this pull request Nov 11, 2022

release-1.5: update release-tools kubernetes-csi/external-resizer#230

Merged

jsafrane mentioned this pull request Nov 11, 2022

release-1.6: update release-tools kubernetes-csi/external-resizer#231

Merged

jsafrane mentioned this pull request Nov 11, 2022

release-2.5: update release-tools kubernetes-csi/node-driver-registrar#241

Merged

jsafrane mentioned this pull request Nov 11, 2022

release-0.7: update release-tools kubernetes-csi/external-health-monitor#138

Merged

jsafrane mentioned this pull request Nov 14, 2022

release-6.1: update release-tools kubernetes-csi/external-snapshotter#786

Merged

jsafrane mentioned this pull request Nov 14, 2022

master: update release-tools kubernetes-sigs/gcp-compute-persistent-disk-csi-driver#1078

Merged

RaunakShah mentioned this pull request Dec 10, 2022

Add volume mode conversion flag to snapshot-controller manifest kubernetes-csi/external-snapshotter#790

Merged

jsafrane mentioned this pull request Jan 2, 2023

release-3.5: update release-tools kubernetes-csi/external-attacher#401

Merged

chrishenzie mentioned this pull request Jan 31, 2023

Update release tools kubernetes-csi/csi-test#431

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Changes to csi prow to run e2e tests in sidecars #206

Changes to csi prow to run e2e tests in sidecars #206

RaunakShah commented Sep 23, 2022 •

edited

Loading

xing-yang commented Oct 3, 2022 •

edited

Loading

RaunakShah Oct 4, 2022 •

edited

Loading

pohly Oct 4, 2022

pohly Oct 4, 2022

RaunakShah Oct 6, 2022

RaunakShah Oct 7, 2022

RaunakShah Oct 7, 2022

pohly Oct 11, 2022

RaunakShah Oct 12, 2022

RaunakShah Oct 12, 2022

pohly Oct 17, 2022

RaunakShah Oct 18, 2022

pohly Oct 6, 2022

RaunakShah Oct 7, 2022

pohly Oct 7, 2022

pohly Oct 6, 2022

RaunakShah Oct 7, 2022

pohly Oct 7, 2022

RaunakShah Oct 10, 2022

jsafrane Oct 17, 2022

RaunakShah Oct 18, 2022

xing-yang commented Oct 12, 2022

RaunakShah commented Oct 13, 2022 •

edited

Loading

xing-yang commented Oct 17, 2022

pohly Oct 21, 2022

RaunakShah Oct 26, 2022

pohly left a comment

pohly left a comment

k8s-ci-robot commented Oct 26, 2022

Changes to csi prow to run e2e tests in sidecars #206

Changes to csi prow to run e2e tests in sidecars #206

Conversation

RaunakShah commented Sep 23, 2022 • edited Loading

xing-yang commented Oct 3, 2022 • edited Loading

RaunakShah Oct 4, 2022 • 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

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

xing-yang commented Oct 12, 2022

RaunakShah commented Oct 13, 2022 • edited Loading

xing-yang commented Oct 17, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pohly left a comment

Choose a reason for hiding this comment

pohly left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Oct 26, 2022

RaunakShah commented Sep 23, 2022 •

edited

Loading

xing-yang commented Oct 3, 2022 •

edited

Loading

RaunakShah Oct 4, 2022 •

edited

Loading

RaunakShah commented Oct 13, 2022 •

edited

Loading