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

Jump to bottom

Degraded mode e2e #2066

Merged
merged 1 commit into from
May 23, 2023
Merged

Degraded mode e2e #2066

merged 1 commit into from
May 23, 2023

Conversation

sawsa307
Copy link
Contributor

@sawsa307 sawsa307 commented Apr 4, 2023 •
edited
Loading

Create e2e test for degraded mode.
When an endpointslice contains endpoint with missing or empty nodeName, this should be filled during degraded mode. When an endpointslice contains endpoint with invalid pod information, or the endpoint has an IP that doesn't correspond to any podIP, this endpoint should be filtered out.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 4, 2023
@k8s-ci-robot
Copy link
Contributor

Hi @sawsa307. 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 /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/test-infra repository.

@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Apr 4, 2023
@sawsa307
Copy link
Contributor Author

sawsa307 commented Apr 4, 2023

/assign @swetharepakula

@sawsa307
Copy link
Contributor Author

sawsa307 commented Apr 4, 2023

cc @gauravkghildiyal

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 7, 2023
@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Apr 11, 2023
@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Apr 12, 2023
@sawsa307 sawsa307 force-pushed the degraded-mode-e2e branch 3 times, most recently from e192904 to 341c6ed Compare April 12, 2023 22:10
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 17, 2023
@k8s-ci-robot k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Apr 17, 2023
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 18, 2023
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 21, 2023
@sawsa307 sawsa307 force-pushed the degraded-mode-e2e branch 2 times, most recently from d86a0cf to 87de629 Compare May 10, 2023 21:59
@sawsa307
Copy link
Contributor Author

/retest

bowei
bowei reviewed May 12, 2023
View reviewed changes
cmd/e2e-test/neg_test.go Outdated
epsName = "custom-endpointslice"
svcName = "service-1"
replicas = int32(2)
endpointAttach = int32(2)
Copy link
Member

Choose a reason for hiding this comment

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

these are still type int32? Is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

bowei
bowei reviewed May 12, 2023
View reviewed changes
cmd/e2e-test/neg_test.go Outdated
// deployment with labels not matched to the service so the desired
// number of podIPs are created and not used by the existing
// endpoint slice.
if err := e2e.EnsureEchoDeployment(s, epsName, endpointAttach, e2e.NoopModify); err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

Can we do this setup before running through all of the test cases?
E.g. do we need to redploy new Pods every time through the test case or just recreate the endpointslices?

E.g. setup the Pods before we start.
Then we can make the expected not just be a count but more explicitly the Pods we expect to show up in the slice based on what has been created.

Copy link
Member

Choose a reason for hiding this comment

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

Let's pull the setup pods out of the for loop for the test cases

Copy link
Contributor Author

@sawsa307 sawsa307 May 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Done. Thank you!

@sawsa307 sawsa307 requested a review from bowei May 15, 2023 20:42
@sawsa307 sawsa307 force-pushed the degraded-mode-e2e branch 6 times, most recently from bf1cef7 to 95a9de9 Compare May 16, 2023 11:05
@sawsa307
Copy link
Contributor Author

/retest

@sawsa307 sawsa307 force-pushed the degraded-mode-e2e branch 5 times, most recently from 2bcf3da to 2762336 Compare May 18, 2023 01:33
swetharepakula
swetharepakula reviewed May 19, 2023
View reviewed changes
cmd/e2e-test/neg_test.go Show resolved Hide resolved
pkg/e2e/helpers.go Show resolved Hide resolved
@sawsa307
Create e2e test for degraded mode
045cc03 
Create e2e test for degraded mode. When an endpointslice contains
endpoint with missing or empty nodeName, this should be filled during
degraded mode. When an endpointslice contains endpoint with invalid pod
information, or the endpoint has an IP that doesn't correspond to any
podIP, this endpoint should be filtered out.
swetharepakula
swetharepakula reviewed May 23, 2023
View reviewed changes
Copy link
Member

@swetharepakula swetharepakula left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 23, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sawsa307, swetharepakula

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 May 23, 2023
@k8s-ci-robot k8s-ci-robot merged commit 0988f33 into kubernetes:master May 23, 2023
@sawsa307 sawsa307 deleted the degraded-mode-e2e branch September 2, 2023 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@swetharepakula swetharepakula swetharepakula left review comments

@aojea aojea Awaiting requested review from aojea

@code-elinka code-elinka Awaiting requested review from code-elinka

@bowei bowei Awaiting requested review from bowei

Assignees

@swetharepakula swetharepakula

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@sawsa307 @k8s-ci-robot @swetharepakula @bowei