🌱 Tag current e2e tests as PR-Blocking by lentzi90 · Pull Request #1390 · kubernetes-sigs/cluster-api-provider-openstack

lentzi90 · 2022-11-21T13:53:23Z

What this PR does / why we need it:

This is preparation for adding more e2e tests which we may not want to run on all PRs. Also adds E2E_GINKGO_FOCUS and E2E_GINKGO_SKIP as convenience variables, and made E2E_GINKGO_PARALLEL configurable

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):

Special notes for your reviewer:

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.
x-ref to test-infra PR where we can make use of this: capo: add e2e full test kubernetes/test-infra#28082

TODOs:

squashed commits
if necessary:
- includes documentation
- adds unit tests

netlify · 2022-11-21T13:53:27Z

✅ Deploy Preview for kubernetes-sigs-cluster-api-openstack ready!

Name	Link
🔨 Latest commit	`11198e0`
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-cluster-api-openstack/deploys/63845aa554daea00099d1fa8
😎 Deploy Preview	https://deploy-preview-1390--kubernetes-sigs-cluster-api-openstack.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

lentzi90 · 2022-11-21T13:57:27Z

This is where the inspiration is coming from:

CAPI is using tags like [PR-Blocking] for their CI and ginkgos focus/skip functionality to decide what tests to run.

Makefile

jichenjc · 2022-11-23T02:35:40Z

Makefile

+
+# to set multiple ginkgo skip flags, if any
+ifneq ($(strip $(E2E_GINKGO_SKIP)),)
+_SKIP_ARGS := $(foreach arg,$(strip $(E2E_GINKGO_SKIP)),-skip="$(arg)")


for now ,there is no SKIP test , you will add the E2E full test into this skip list (so it will not run every PR
but should be daily/weekly or manual trigger?)

Actually my plan was to use focus to run the default tests (E2E_GINKGO_FOCUS="\[PR-Blocking\]"). Then for the e2e full test we would skip PR-Blocking since it would anyway run on all PRs. This is the same way that CAPI has done it.

Here CAPI focus on PR-Blocking for their default e2e test: https://github.com/kubernetes/test-infra/blob/master/config/jobs/kubernetes-sigs/cluster-api/cluster-api-presubmits-main.yaml#L120-L150
And here they skip PR-Blocking and other tests that would anyway run separately: https://github.com/kubernetes/test-infra/blob/b27527630f6d4c0f2787f0362efd54c4cab5fbd6/config/jobs/kubernetes-sigs/cluster-api/cluster-api-presubmits-main.yaml#L222-L256

mdbooth

Very much in favour of this. Happy to approve once we've resolved the ginkgo args question.

mdbooth · 2022-11-24T09:49:33Z

Makefile

 test-e2e: e2e-prerequisites ## Run e2e tests
-	time $(GINKGO) --failFast -trace -progress -v -tags=e2e --nodes=$(E2E_GINKGO_PARALLEL) $(E2E_GINKGO_ARGS) ./test/e2e/suites/e2e/... -- -config-path="$(E2E_CONF_PATH)" -artifacts-folder="$(ARTIFACTS)" --data-folder="$(E2E_DATA_DIR)" $(E2E_ARGS)
+	time $(GINKGO) --failFast -trace -progress -v -tags=e2e --nodes=$(E2E_GINKGO_PARALLEL) \
+			--focus="$(E2E_GINKGO_FOCUS)" $(_SKIP_ARGS) $(E2E_GINKGO_ARGS) ./test/e2e/suites/e2e/... -- \


Does this error if E2E_GINKGO_FOCUS is empty? Do we need the same dance as on lines 131-133 above for skip?

If not, can we also omit the dance for E2E_GINKGO_SKIP?

It works even if E2E_GINKGO_FOCUS is empty. I actually stole it from CAPI. They also have the same _SKIP_ARGS and I think this is because an empty string would match everything. So empty focus is no problem, we just run all the tests. Skipping everything on the other hand would not be great.

mdbooth · 2022-11-24T10:45:52Z

/approve

k8s-ci-robot · 2022-11-24T10:46:05Z

[APPROVALNOTIFIER] This PR is APPROVED

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

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [mdbooth]

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

seanschneeweiss

/lgtm

Thank you for taking such a close look at how CAPI solves the same challenge and adapting to their implementation.

lentzi90 · 2022-11-25T06:11:03Z

Some kind of temporary issue with the devstack from what I understand
/test pull-cluster-api-provider-openstack-e2e-test

jichenjc · 2022-11-25T06:11:56Z

#1397 tracking

lentzi90 · 2022-11-28T06:09:42Z

/test pull-cluster-api-provider-openstack-e2e-test

This is preparation for adding more e2e tests which we may not want to run on all PRs. Also adds E2E_GINKGO_FOCUS and E2E_GINKGO_SKIP as convenience variables, and made E2E_GINKGO_PARALLEL configurable

lentzi90 · 2022-11-28T08:40:53Z

Rebased to get the CI fix. Tests are passing 🎉

erkanerol · 2022-11-28T08:45:47Z

@lentzi90 THANKS A LOT!!!

seanschneeweiss · 2022-11-29T13:12:44Z

/lgtm

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 21, 2022

k8s-ci-robot requested review from jichenjc and mdbooth November 21, 2022 13:53

k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Nov 21, 2022

lentzi90 mentioned this pull request Nov 22, 2022

capo: add e2e full test kubernetes/test-infra#28082

Merged

jichenjc reviewed Nov 23, 2022

View reviewed changes

mdbooth reviewed Nov 24, 2022

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 24, 2022

lentzi90 mentioned this pull request Nov 24, 2022

🌱 Add e2e clusterctl upgrade tests #1371

Merged

3 tasks

seanschneeweiss reviewed Nov 25, 2022

View reviewed changes

k8s-ci-robot assigned seanschneeweiss Nov 25, 2022

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

Tag current e2e tests as PR-Blocking

11198e0

This is preparation for adding more e2e tests which we may not want to run on all PRs. Also adds E2E_GINKGO_FOCUS and E2E_GINKGO_SKIP as convenience variables, and made E2E_GINKGO_PARALLEL configurable

lentzi90 force-pushed the lentzi90/prepare-additional-e2e-tests branch from 081e9ed to 11198e0 Compare November 28, 2022 06:52

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 28, 2022

lentzi90 requested a review from seanschneeweiss November 28, 2022 08:41

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

k8s-ci-robot merged commit 9b6cdd9 into kubernetes-sigs:main Nov 29, 2022

lentzi90 deleted the lentzi90/prepare-additional-e2e-tests branch November 30, 2022 06:24

Conversation

lentzi90 commented Nov 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

netlify bot commented Nov 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for kubernetes-sigs-cluster-api-openstack ready!

Uh oh!

lentzi90 commented Nov 21, 2022

Uh oh!

Uh oh!

jichenjc Nov 23, 2022

Choose a reason for hiding this comment

Uh oh!

lentzi90 Nov 23, 2022

Choose a reason for hiding this comment

Uh oh!

mdbooth left a comment

Choose a reason for hiding this comment

Uh oh!

mdbooth Nov 24, 2022

Choose a reason for hiding this comment

Uh oh!

lentzi90 Nov 24, 2022

Choose a reason for hiding this comment

Uh oh!

mdbooth commented Nov 24, 2022

Uh oh!

k8s-ci-robot commented Nov 24, 2022

Uh oh!

seanschneeweiss left a comment

Choose a reason for hiding this comment

Uh oh!

lentzi90 commented Nov 25, 2022

Uh oh!

jichenjc commented Nov 25, 2022

Uh oh!

lentzi90 commented Nov 28, 2022

Uh oh!

lentzi90 commented Nov 28, 2022

Uh oh!

erkanerol commented Nov 28, 2022

Uh oh!

seanschneeweiss commented Nov 29, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

lentzi90 commented Nov 21, 2022 •

edited

Loading

netlify bot commented Nov 21, 2022 •

edited

Loading