Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shift generic alpha and beta jobs to generic alpha and beta feature tags #34456

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Shift generic alpha and beta jobs to generic alpha and beta feature tags #34456

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions config/jobs/kubernetes/sig-cloud-provider/gcp/gcp-gce.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,7 @@ presubmits:
- --gcp-region=us-central1
- --provider=gce
- --runtime-config=api/all=true
- --test_args=--ginkgo.focus=\[Feature:(WatchList|InPlacePodVerticalScaling|APIServerTracing|SidecarContainers|StorageVersionAPI|PodPreset|ClusterTrustBundle|ClusterTrustBundleProjection|PodLifecycleSleepAction|RecoverVolumeExpansionFailure)\] --ginkgo.skip=\[Feature:(SCTPConnectivity|Volumes|Networking-Performance)\]|IPv6|csi-hostpath-v0|\[KubeUp\] --minStartupPods=8
- --test_args=--ginkgo.focus=\[Feature:(Alpha|WatchList|InPlacePodVerticalScaling|APIServerTracing|SidecarContainers|StorageVersionAPI|PodPreset|ClusterTrustBundle|ClusterTrustBundleProjection|PodLifecycleSleepAction|RecoverVolumeExpansionFailure)\] --ginkgo.skip=\[Feature:(Beta|SCTPConnectivity|Volumes|Networking-Performance)\]|IPv6|csi-hostpath-v0|\[KubeUp\] --minStartupPods=8
Copy link
Contributor

@pohly pohly Mar 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Instead of tweaking focus regexps further I prefer to convert to -ginkgo.label-filter. Let's end the pain.

Copy link
Member

Choose a reason for hiding this comment

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

in this case, would that mean the following?

Suggested change
- --test_args=--ginkgo.focus=\[Feature:(Alpha|WatchList|InPlacePodVerticalScaling|APIServerTracing|SidecarContainers|StorageVersionAPI|PodPreset|ClusterTrustBundle|ClusterTrustBundleProjection|PodLifecycleSleepAction|RecoverVolumeExpansionFailure)\] --ginkgo.skip=\[Feature:(Beta|SCTPConnectivity|Volumes|Networking-Performance)\]|IPv6|csi-hostpath-v0|\[KubeUp\] --minStartupPods=8
- --test_args=--ginkgo.label-filter=/Feature:(Alpha|WatchList|InPlacePodVerticalScaling|APIServerTracing|SidecarContainers|StorageVersionAPI|PodPreset|ClusterTrustBundle|ClusterTrustBundleProjection|PodLifecycleSleepAction|RecoverVolumeExpansionFailure)/ && !(/Feature:(Beta|SCTPConnectivity|Volumes|Networking-Performance)/ || IPv6 || csi-hostpath-v0 || KubeUp) --minStartupPods=8

Though I am not sure what "IPv6" and "csi-hostpath-v0" stand for, these don't appear to be labels in the k/k e2e test suite. I actually couldn't find mention of "csi-hostpath-v0" in the whole k/k codebase.

Copy link
Contributor

Choose a reason for hiding this comment

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

No, that's not quite how Ginkgo --label-filter works. See #32911 and https://groups.google.com/a/kubernetes.io/g/dev/c/DTFEng143NY/m/FqdD3RADAQAJ for instructions.

A good first step would be to explain in plain English which tests should run in this job. Is it generic (= run everything that can run) or specific to certain features (as implied by the current focus expression which only runs tests selected by that expression)?

Copy link
Member

Choose a reason for hiding this comment

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

That is actually quite a good mental exercise. I wonder what this and the below tests are supposed to do. From both the test name and the current test definition, I suspect the plain English explanation would be run tests for these specific features as long as they are alpha.

Copy link
Member Author

@liggitt liggitt Mar 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

to unblock ClusterTrustBundle beta promotion, if something like kubernetes/kubernetes#130580 or kubernetes/kubernetes#130567 can get us coverage via the kind alpha / alpha-beta / beta jobs (which it seems like they both can), I'd actually be fine just dropping ClusterTrustBundle|ClusterTrustBundleProjection from ~all the other e2e job configurations for now

then those other jobs can switch to label filter at their leisure, and we're not entangled with ClusterTrustBundle work

Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder what this and the below tests are supposed to do.

That is the million dollar question. The answer might very well be "generic job which supports all feature-gated tests that have no special other requirements", in which case the label filter is as simple as the one in the kind jobs.

Most of the focus expression then becomes historic garbage that we should get rid of.

Who owns these jobs? The job owner should know what they are for and what they are willing to support with regards to triaging test failures.

then those other jobs can switch to label filter at their leisure, and we're not entangled with ClusterTrustBundle work

That sounds like a good plan.

Copy link
Member

Choose a reason for hiding this comment

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

That is the million dollar question. The answer might very well be "generic job which supports all feature-gated tests that have no special other requirements", in which case the label filter is as simple as the one in the kind jobs.

IMHO that should be the end-state.

Most of the focus expression then becomes historic garbage that we should get rid of.

+1

Who owns these jobs? The job owner should know what they are for and what they are willing to support with regards to triaging test failures.

The GCE ones are tragedy of the commons.
The kind ones @aojea and I are actively maintaining.

Copy link
Contributor

Choose a reason for hiding this comment

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

in which case the label filter is as simple as the one in the kind jobs.

IMHO that should be the end-state.

Cross-reference: kubernetes/kubernetes#130580 (comment) proposes a slightly different label filter for "cluster with default configuration".

The GCE ones are tragedy of the commons.

Should we make that official and move them under SIG Testing or Release? SIG release might be more appropriate, considering that they already get the alerts (testgrid-alert-email: [email protected]).

- --timeout=180m
image: gcr.io/k8s-staging-test-infra/kubekins-e2e:v20250212-16f67660c2-master
resources:
Expand Down Expand Up @@ -942,7 +942,7 @@ periodics:
- --gcp-region=us-central1
- --provider=gce
- --runtime-config=api/all=true
- --test_args=--ginkgo.focus=\[Feature:(AdmissionWebhookMatchConditions|InPlacePodVerticalScaling|SidecarContainers|StorageVersionAPI|PodPreset|StatefulSetAutoDeletePVC|ClusterTrustBundle|ClusterTrustBundleProjection)\]|Networking --ginkgo.skip=\[Feature:(SCTPConnectivity|Volumes|Networking-Performance|Example)\]|IPv6|csi-hostpath-v0 --minStartupPods=8
- --test_args=--ginkgo.focus=\[Feature:(Alpha|AdmissionWebhookMatchConditions|InPlacePodVerticalScaling|SidecarContainers|StorageVersionAPI|PodPreset|StatefulSetAutoDeletePVC|ClusterTrustBundle|ClusterTrustBundleProjection)\]|Networking --ginkgo.skip=\[Feature:(Beta|SCTPConnectivity|Volumes|Networking-Performance|Example)\]|IPv6|csi-hostpath-v0 --minStartupPods=8
- --timeout=180m
image: gcr.io/k8s-staging-test-infra/kubekins-e2e:v20250212-16f67660c2-master
resources:
Expand Down
4 changes: 2 additions & 2 deletions config/jobs/kubernetes/sig-cloud-provider/periodic-e2e.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,8 @@ periodics:
--test-package-url=https://dl.k8s.io/ \
--test-package-dir=ci/fast \
--test-package-marker=latest-fast.txt \
--focus-regex="\[Feature:(AdmissionWebhookMatchConditions|InPlacePodVerticalScaling|SidecarContainers|StorageVersionAPI|PodPreset|StatefulSetAutoDeletePVC|ClusterTrustBundle|ClusterTrustBundleProjection)\]|Networking" \
--skip-regex="\[Feature:(SCTPConnectivity|Volumes|Networking-Performance|Example)\]|IPv6|csi-hostpath-v0" \
--focus-regex="\[Feature:(Alpha|AdmissionWebhookMatchConditions|InPlacePodVerticalScaling|SidecarContainers|StorageVersionAPI|PodPreset|StatefulSetAutoDeletePVC|ClusterTrustBundle|ClusterTrustBundleProjection)\]|Networking" \
--skip-regex="\[Feature:(Beta|SCTPConnectivity|Volumes|Networking-Performance|Example)\]|IPv6|csi-hostpath-v0" \
--parallel=25
env:
- name: USE_DOCKERIZED_BUILD
Expand Down
4 changes: 2 additions & 2 deletions config/jobs/kubernetes/sig-node/sig-node-presubmit.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -917,8 +917,8 @@ presubmits:
--test-package-url=https://dl.k8s.io/ \
--test-package-dir=ci/fast \
--test-package-marker=latest-fast.txt \
--focus-regex="\[Feature:(AdmissionWebhookMatchConditions|InPlacePodVerticalScaling|SidecarContainers|StorageVersionAPI|PodPreset|StatefulSetAutoDeletePVC|ClusterTrustBundle|ClusterTrustBundleProjection)\]|Networking" \
--skip-regex="\[Feature:(SCTPConnectivity|Volumes|Networking-Performance)\]|IPv6|csi-hostpath-v0" \
--focus-regex="\[Feature:(Alpha|AdmissionWebhookMatchConditions|InPlacePodVerticalScaling|SidecarContainers|StorageVersionAPI|PodPreset|StatefulSetAutoDeletePVC|ClusterTrustBundle|ClusterTrustBundleProjection)\]|Networking" \
--skip-regex="\[Feature:(Beta|SCTPConnectivity|Volumes|Networking-Performance)\]|IPv6|csi-hostpath-v0" \
--parallel=25
env:
- name: USE_DOCKERIZED_BUILD
Expand Down
12 changes: 6 additions & 6 deletions releng/test_config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,8 +271,8 @@ testSuites:
# Panic if anything mutates a shared informer cache
- --env=ENABLE_CACHE_MUTATION_DETECTOR=true
- --runtime-config=api/all=true
- --test_args=--ginkgo.focus=\[Feature:(AdmissionWebhookMatchConditions|InPlacePodVerticalScaling|SidecarContainers|StorageVersionAPI|PodPreset|StatefulSetAutoDeletePVC|ClusterTrustBundle|ClusterTrustBundleProjection)\]|Networking
--ginkgo.skip=\[Feature:(SCTPConnectivity|Volumes|Networking-Performance|Example)\]|IPv6|csi-hostpath-v0
- --test_args=--ginkgo.focus=\[Feature:(Alpha|AdmissionWebhookMatchConditions|InPlacePodVerticalScaling|SidecarContainers|StorageVersionAPI|PodPreset|StatefulSetAutoDeletePVC|ClusterTrustBundle|ClusterTrustBundleProjection)\]|Networking
--ginkgo.skip=\[Feature:(Beta|SCTPConnectivity|Volumes|Networking-Performance|Example)\]|IPv6|csi-hostpath-v0
--minStartupPods=8
cluster: k8s-infra-prow-build
alphafeatures-eventedpleg:
Expand All @@ -284,8 +284,8 @@ testSuites:
# Panic if anything mutates a shared informer cache
- --env=ENABLE_CACHE_MUTATION_DETECTOR=true
- --runtime-config=api/all=true
- --test_args=--ginkgo.focus=\[Feature:(AdmissionWebhookMatchConditions|InPlacePodVerticalScaling|SidecarContainers|StorageVersionAPI|PodPreset|StatefulSetAutoDeletePVC|ClusterTrustBundle|ClusterTrustBundleProjection)\]|Networking
--ginkgo.skip=\[Feature:(SCTPConnectivity|Volumes|Networking-Performance|Example)\]|IPv6|csi-hostpath-v0
- --test_args=--ginkgo.focus=\[Feature:(Alpha|AdmissionWebhookMatchConditions|InPlacePodVerticalScaling|SidecarContainers|StorageVersionAPI|PodPreset|StatefulSetAutoDeletePVC|ClusterTrustBundle|ClusterTrustBundleProjection)\]|Networking
--ginkgo.skip=\[Feature:(Beta|SCTPConnectivity|Volumes|Networking-Performance|Example)\]|IPv6|csi-hostpath-v0
--minStartupPods=8
cluster: k8s-infra-prow-build
alphafeatures-no-ccm:
Expand All @@ -297,8 +297,8 @@ testSuites:
# Panic if anything mutates a shared informer cache
- --env=ENABLE_CACHE_MUTATION_DETECTOR=true
- --runtime-config=api/all=true
- --test_args=--ginkgo.focus=\[Feature:(Audit|BlockVolume|PodPreset|ExpandCSIVolumes|ExpandInUseVolumes)\]|Networking
--ginkgo.skip=\[Feature:(SCTPConnectivity|Volumes|Networking-Performance|Networking-IPv6)\]|csi-hostpath-v0
- --test_args=--ginkgo.focus=\[Feature:(Alpha|Audit|BlockVolume|PodPreset|ExpandCSIVolumes|ExpandInUseVolumes)\]|Networking
--ginkgo.skip=\[Feature:(Beta|SCTPConnectivity|Volumes|Networking-Performance|Networking-IPv6)\]|csi-hostpath-v0
--minStartupPods=8
cluster: k8s-infra-prow-build
betaapis: # copied from "default". Tests that require beta APIs should use discovery in the e2e test to decide whether or not to skip the test.
Expand Down