-
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
Move e2e fromManifest funcs to manifest package #43637
Conversation
Hi @wongma7. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with 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. I understand the commands that are listed here. |
Darn, #43066 throws a wrench into this :) the non-statefulset parts & general idea of this PR can still be reviewed though... |
@foxish, here https://github.com/kubernetes/kubernetes/pull/43637/files#diff-20a9335743239e362032f5839bb81f98L106 why do we use |
/approve |
@wongma7 The change in that PR from fix from |
@erictune can you review or assign another reviewer, thanks! this is purely a mechanical & refactoring PR, no major additions. |
@erictune ping please reassign |
@k8s-bot ok to test |
/cc @kubernetes/sig-testing-pr-reviews |
Just added package to "hack/.linted_packages" to fix jenkins verification |
@k8s-bot gce etcd3 e2e test this "Ginkgo timed out waiting for all parallel nodes to report back!" |
/approve |
/lgtm |
@k8s-bot pull-kubernetes-federation-e2e-gce test this |
@wongma7: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. I understand the commands that are listed here. |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mikedanese, spiffxp, spxtr, wongma7 Associated issue: 37007 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
Automatic merge from submit-queue |
This was broken back in kubernetes#43637 when the logic in `(*StatefulSetTester).CreateStatefulSet` switched from using `generated.ReadOrDie` to read the entire service.yaml file and pass it to kubectl to using `manifest.SvcFromManifest`, which assumes that the file contains only a single service. Fixes kubernetes#52750
Automatic merge from submit-queue (batch tested with PRs 51759, 53001, 52806). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.. Fix broken statefulset e2e test **What this PR does / why we need it**: Fixes the CockroachDB statefulset e2e test. This was broken back in #43637 when the logic in `(*StatefulSetTester).CreateStatefulSet` switched from using `generated.ReadOrDie` to read the entire service.yaml file and pass it to kubectl to using `manifest.SvcFromManifest`, which assumes that the file contains only a single service. To fix the test, just remove the second service, which isn't needed to test the Statefulset functionality. **Which issue this PR fixes**: Fixes #52750 **Special notes for your reviewer**: N/A **Release note**: ```release-note NONE ```
fixes #37007 again
The goal here is to remove e2e/framework's dependence on e2e/generated so that external projects can import e2e/framework without issue. I reorganize & try to make all the 'fromManifest' funcs consistent (to all use generated, return err) to prevent any reintroduction of generated into framework