-
Notifications
You must be signed in to change notification settings - Fork 38.7k
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
Restructure IngressClass e2e tests for potential Conformance bump #92147
Comments
/sig network |
/triage unresolved Comment 🤖 I am a bot run by vllry. 👩🔬 |
/remove-triage unresolved |
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
Related to #93427 |
This bug is still there even after #93427, we occasionally see it in cri-o tests (see e.g. cri-o/cri-o#4253). /remove-lifecycle stale |
@liggitt Do we have precedent for these type of defaulting e2e tests? Is it sufficient to just check if a default exists already and only then try to create one? |
I'm not sure... I don't see storageclasses, or priorityclasses creating defaulting objects in e2e tests I don't see anything testing priorityclass defaulting in e2e tests Apps and Storage have e2e tests (but not conformance tests) that depend on a default storageclass existing, and skip them if none exists: https://github.com/kubernetes/kubernetes/blob/release-1.19/test/e2e/framework/pv/pv.go#L821-L827 |
@robscott Do you have any thoughts about not creating the defaults for e2e tests and just skipping if don't exist? |
I'd discussed this a bit with @cmluciano on Slack today but just realized I never followed up here. I think these tests are still valuable to have in the e2e suite. I'd hate to not have any coverage for this feature in e2e tests which I think is what we'd end up with if we only ran the tests in clusters that already had a default ingressclass. Maybe in time more environments will actually provide a default ingressclass, but I'm thinking that won't be widespread anytime in the next few months. Maybe the safest option here is to leave this out of conformance and update it to use an existing default class if it exists. |
My bad 🤦, just found out we forgot to skip |
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-contributor-experience at kubernetes/community. |
/remove-lifecycle stale |
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-contributor-experience at kubernetes/community. |
Stale issues rot after 30d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-contributor-experience at kubernetes/community. |
The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs. This bot triages issues and PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /close |
@k8s-triage-robot: Closing this issue. 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/test-infra repository. |
What happened:
The existing ingressclass make assumptions about no default ingressclass being present and they should be run in Serial mode.
What you expected to happen:
These tests should be refactored to not make assumptions about the cluster state, so that they can be added to the conformance suite for IngressClass.
J Liggitt suggests the following changes need to happen:
How to reproduce it (as minimally and precisely as possible):
Run the tests in an existing cluster with similar ingressclass names, an existing default ingressclass, or in a random order.
Anything else we need to know?:
Environment:
kubectl version
): v1.18cat /etc/os-release
): N/Auname -a
): N/AThe text was updated successfully, but these errors were encountered: