Skip to content

Conversation

bexxmodd
Copy link
Contributor

@bexxmodd bexxmodd commented Oct 7, 2025

What type of PR is this?
Add one of the following kinds:
/kind bug
/area conformance-machinery

What this PR does / why we need it:
If you currently try to combine --all-features and --exempt-features flags for local testing it will be a noop. This fixes the issue.

Does this PR introduce a user-facing change?:

NONE

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/bug Categorizes issue or PR as related to a bug. area/conformance-machinery Issues or PRs related to the machinery and the suite used to run conformance tests. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Oct 7, 2025
@k8s-ci-robot k8s-ci-robot requested review from candita and kflynn October 7, 2025 03:42
@k8s-ci-robot k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Oct 7, 2025
@k8s-ci-robot
Copy link
Contributor

Hi @bexxmodd. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@bexxmodd
Copy link
Contributor Author

bexxmodd commented Oct 7, 2025

/cc @LiorLieberman

@bexxmodd bexxmodd changed the title Fix that allows to --all-features and --exampt-features flags work together Fix that allows to --all-features and --exempt-features flags work together Oct 7, 2025
@LiorLieberman
Copy link
Member

/approve
Will leave lgtm to someone else

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bexxmodd, LiorLieberman

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 7, 2025
@kflynn
Copy link
Contributor

kflynn commented Oct 7, 2025

This is fixing one small bit of what seems to be a larger problem with the logic. What I found trying to run the 1.4 conformance tests was:

  1. --all-features and --exempt-features didn't work together.
  2. Even when I explictly asked for only the Mesh profile, and marked Gateway as exempt, I was getting Gateway tests trying to run (even after I separately fixed problem 1 in my local copy of conformance).

The combination makes me feel that we introduced a significant problem in the conformance logic, and I'd like to see all of that addressed. Off the top of my head, I'd say

  • read from supportedFeatures if you're allowed to
  • add features from explicitly stated profiles
  • add all features if --all-features is set
  • remove all features listed by --exempt-features (including all features that are part of exempted profiles)

in that order... and I may be wrong even there, but it feels right at the start.

Overall, I'm not a fan of this split between "inferred" and "manual" -- I found the long discussion we had on Slack, and summarized it in #3927 (comment). The above doesn't really depend on that, and I do think it makes sense to be able to read features from supportedFeatures; most of all, though, it must be possible to easily do a mesh-only test.

@bexxmodd
Copy link
Contributor Author

bexxmodd commented Oct 7, 2025

This is fix for the issue n.1 from #4154

@kflynn
Copy link
Contributor

kflynn commented Oct 7, 2025

Yeah, @bexxmodd, what I mean here is that it's a point fix: instead of systematically making sure that all the combinations will do sane things, you're addressing one specific failure. At this point, I think we're better served by taking a bit more time and doing a single PR where we've verified that a known set of flag combinations does sane things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/conformance-machinery Issues or PRs related to the machinery and the suite used to run conformance tests. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. release-note-none Denotes a PR that doesn't merit a release note. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants