-
Notifications
You must be signed in to change notification settings - Fork 2.8k
sig-testing: alpha/beta "enabled" and "conformance" presubmits #35534
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.
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
Conversation
/hold
For testing alpha features, sure. But we can cover that by exposing that the test has such dependencies and skipping them. For starting a cluster, it should absolutely be safe to set AllAlpha, not use those features, and have GA / default features work fine. |
- name: RUNTIME_CONFIG | ||
value: '{"api/alpha":"true", "api/ga":"true"}' | ||
- name: LABEL_FILTER | ||
value: "Feature: isSubsetOf OffByDefault && !BetaOffByDefault && !Deprecated && !Slow && !Disruptive && !Flaky" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we skip BetaOffByDefault here so that we do not need to remove this job
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem is not which tests we run, it is the cluster config itself (see kubernetes/kubernetes#133697 (comment) and following).
I see this discussion now: kubernetes/kubernetes#133697 (comment) I think we should retain "only test default features (which should be stable), but enable alphas (which shouldn't destabilize the cluster if unused)", but I'm not sure what to call that job. It will instead be "enable all features". |
Isn't that the "alpha-beta-enabled" variant that we discussed in https://kubernetes.slack.com/archives/C09QZ4DQB/p1749500650854679?thread_ts=1748885684.963169&cid=C09QZ4DQB? We never got around to adding those. I didn't do it at that time and then had the impression that you were working on it together with some communication to the project about this. We can also do "beta-enabled". We can't do "alpha-enabled", for the same reason that we cannot do "alpha-features" (potentially invalid cluster config). |
yeah. I think the most value will be the alpha-beta-enabled but we should also do beta-enabled. Also commented back on the main thread, I don't know what to call it without being a mouthful but something like alpha-beta-conformance in the way we have ga-only conformance, I think breaking conformance tests by merely enabling these is a red flag and maybe that's a better target for a periodic than just alpha-enabled + default presubmits |
The difference between We can do both, but I am not sure how useful Coming back to this PR: can we merge it now and then add the missing jobs separately, or do you want me to turn this PR into the overall "clean up feature gate testing jobs" PR? |
s/GA/on-by-default/g, and not all, typically we would be skipping serial and slow for example, whereas conformance does not. they have overlap.
We have no rule that says this. And anyhow, enabling it should be safe. Regardless, new jobs must start by proposing to informing, not blocking yet.
I don't follow why it's redundant, conformance is largely a superset of the tests we run in presubmit. It may make more sense to just have the conformance version though. |
oh, I totally forgot that to do alpha, beta feature jobs we also added the capacity to run the e2e tests with that specific labels. |
Ack to on-by-default. That typically means tests for beta features, and with "beta is the new GA" that means we would run tests that really should pass. Test failures in "alpha-beta-enabled" (all features enabled, run on-by-default tests) need the same attention as failures in "alpha-beta-conformance" (all features enabled, run only conformance tests). I don't see why we should skip serial or slow in an "alpha-beta-enabled" CI job. If we have such tests, we should run them. I would also include them in an optional presubmit. The only situation where excluding them may make sense is in a presubmit which always runs, to keep resource usage lower and to avoid delays.
Providing "alpha-beta-conformance" as a slightly more stable (because less tests) version of "alpha-beta-enabled" for SIG release may make sense. It all depends on who is going to monitor those jobs. So shall I include the new jobs here or not? |
fad1d81
to
346da54
Compare
The kind-alpha-features jobs got removed through a different PR. As we started the discussion around them (again...) here, let's repurpose this PR to add the missing jobs. I've started with presubmits for now and will do periodics next if this works out okay. |
config/jobs/kubernetes/sig-testing/kubernetes-kind-presubmits.yaml
Outdated
Show resolved
Hide resolved
d0c3fa8
to
212dcd7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
We've had kind-alpha-beta-features and kind-beta-features jobs for a while. The original purpose was to run stable tests in a cluster with features enabled to detect when enabling those breaks stable functionality. Later the jobs were extended to also run all tests which should work in such a cluster. The kind-alpha-features job got removed recently because it's not necessarily a valid cluster configuration. What this adds for the both cluster configs is: - "-enabled": running only tests for on-by-default features, i.e. excluding tests for stable features. This matches the original purpose of the jobs. - "-enabled-conformance": restricts the test selection even further to only conformance tests. Both can eventually get promoted to release informing or even blocking. Only presubmits get added for now. If testing the jobs in a trial PR works, the corresponding periodics can be added.
212dcd7
to
04f210d
Compare
I made the same change as in #35575 (disable the SKIP default): https://github.com/kubernetes/test-infra/compare/212dcd7c76b7f31e94c8614c7742abae45d63829..04f210d27d9f0c864998a8d4b2d804dc68be8bdf @aojea: can you perhaps LGTM + approve both this and the other PR? I'd like to make some progress on this. |
absolutely |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aojea, pohly 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:
Approvers can indicate their approval by writing |
/hold cancel |
@pohly: Updated the
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
This mirrors the presubmit counterparts that were added in kubernetes#35534 and tested in kubernetes/kubernetes#134250. I can help monitor the existing alpha/beta features jobs. Those need to be renamed in testgrid to avoid confusion. Other than that they remain unchanged for now. Potential future work: - Changes related to serial and/or slow jobs. Serial tests are excluded implicitly by e2e-k8s.sh because the jobs enables PARALLEL (kubernetes#35594). Slow jobs are disabled in LABEL_FILTER because that is what the existing periodics did. We might be able to run them because as long as they overlap with other tests there shouldn't be much impact on overall job duration (same applies to presubmits!). Scheduling of slow tests may be relevant (onsi/ginkgo#1599). - Release informing/blocking. The existing jobs are release informing. alpha-beta-features shouldn't be because breaking alpha tests is not something that the release team should have to deal with. Instead, the new jobs should get promoted once they are known to be stable. beta-features can remain release informing, tests for beta features (even if off-by-default) need to be stable. - Decision about "enabled-conformance". The conformance jobs got included because it was suggested on Slack. They run a subset of the tests run by their "enabled" counterparts. It remains to be seen whether having two jobs instead of one really provides a better release signal.
We've had kind-alpha-beta-features and kind-beta-features jobs for a while. The original purpose was to run stable tests in a cluster with features enabled to detect when enabling those breaks stable functionality. Later the jobs were extended to also run all tests which should work in such a cluster. The kind-alpha-features job got removed recently because it's not necessarily a valid cluster configuration.
What this adds for the both cluster configs is:
Both can eventually get promoted to release informing or even blocking.
Only presubmits get added for now. If testing the jobs in a trial PR works, the corresponding periodics can be added.
For reference: