Skip to content

Adding e2e tests covering EndpointSlice and Endpoints Controllers#84208

Merged
k8s-ci-robot merged 1 commit intokubernetes:masterfrom
robscott:endpointslice-e2e
Nov 13, 2019
Merged

Adding e2e tests covering EndpointSlice and Endpoints Controllers#84208
k8s-ci-robot merged 1 commit intokubernetes:masterfrom
robscott:endpointslice-e2e

Conversation

@robscott
Copy link
Copy Markdown
Member

@robscott robscott commented Oct 22, 2019

What type of PR is this?
/kind feature

What this PR does / why we need it:
Since EndpointSlices and Endpoints are expected to be enabled by default for some time, it's important to have tests covering the functionality of both controllers. With some consumers relying on Endpoints and others relying on EndpointSlices, it's important to ensure both resources are generated in a timely manner with consistent attributes.

Does this PR introduce a user-facing change?:

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

/sig network
/priority important-longterm

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/feature Categorizes issue or PR as related to a new feature. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. sig/network Categorizes an issue or PR as relevant to SIG Network. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. area/test sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Oct 22, 2019
@irvifa
Copy link
Copy Markdown
Member

irvifa commented Oct 23, 2019

/retest

@freehan
Copy link
Copy Markdown
Contributor

freehan commented Oct 23, 2019

/assign freehan

@robscott
Copy link
Copy Markdown
Member Author

robscott commented Oct 23, 2019

/retest

@robscott
Copy link
Copy Markdown
Member Author

robscott commented Oct 24, 2019

The integration test appears to be a flaky failure, but the other e2e tests are failing due to EndpointSlices not being enabled by default. At this point I think that's intended behavior and will be resolved by an upcoming PR to graduate EndpointSlices to beta and enable them by default.

@robscott robscott mentioned this pull request Oct 25, 2019
Merged
8 tasks
@robscott
Copy link
Copy Markdown
Member Author

robscott commented Oct 28, 2019

/retest

freehan
freehan reviewed Oct 29, 2019
View reviewed changes
test/e2e/network/endpointslice.go Outdated
Copy link
Copy Markdown
Contributor

@freehan freehan Oct 29, 2019

Choose a reason for hiding this comment

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

consider moving this into expectEndpointsAndSlices

Copy link
Copy Markdown
Member Author

@robscott robscott Oct 30, 2019

Choose a reason for hiding this comment

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

I ended up leaving it where it is since those functions are relying on the same set of pods existing, whereas they are each looking for unique sets of Endpoints and EndpointSlices. Hopefully with how things have been restructured now the separation makes a bit more sense, but happy to move things around if not.

test/e2e/network/endpointslice.go Outdated
Copy link
Copy Markdown
Contributor

@freehan freehan Oct 29, 2019

Choose a reason for hiding this comment

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

Consider having a overarching poll, instead of waiting per object.
This may be more fault tolerant.

wait.Poll(
     pod is ready?
     getEndpoints
     getEndpointSlice
     getMatchingEndpoint
     parse 
)

Copy link
Copy Markdown
Member Author

@robscott robscott Oct 30, 2019

Choose a reason for hiding this comment

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

Good idea. Mostly did this, except split it up into one for both pods, and one for Endpoints and EndpointSlices for each test case, hopefully that makes sense.

@robscott
Copy link
Copy Markdown
Member Author

robscott commented Oct 30, 2019

/retest

freehan
freehan reviewed Nov 8, 2019
View reviewed changes
test/e2e/network/endpointslice.go Outdated
Copy link
Copy Markdown
Contributor

@freehan freehan Nov 8, 2019

Choose a reason for hiding this comment

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

is this needed?

@freehan
Copy link
Copy Markdown
Contributor

freehan commented Nov 8, 2019

LGTM overall

@freehan
Copy link
Copy Markdown
Contributor

freehan commented Nov 11, 2019

/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 Nov 11, 2019
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Nov 11, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: freehan, robscott

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details 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 Nov 11, 2019
@fejta-bot
Copy link
Copy Markdown

fejta-bot commented Nov 12, 2019

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@fejta-bot
Copy link
Copy Markdown

fejta-bot commented Nov 12, 2019

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@robscott
Adding e2e tests covering EndpointSlice and Endpoints Controllers
6d2fc09 
Since EndpointSlices and Endpoints are expected to be enabled by default
for some time, it's important to have tests covering the functionality
of both controllers. With some consumers relying on Endpoints and others
relying on EndpointSlices, it's important to ensure both resources are
generated in a timely manner with consistent attributes.
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 12, 2019
@robscott
Copy link
Copy Markdown
Member Author

robscott commented Nov 12, 2019

/retest

1 similar comment
@robscott
Copy link
Copy Markdown
Member Author

robscott commented Nov 12, 2019

/retest

@mrbobbytables mrbobbytables mentioned this pull request Nov 13, 2019
Closed
@MrHohn
Copy link
Copy Markdown
Member

MrHohn commented Nov 13, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 13, 2019
@k8s-ci-robot k8s-ci-robot merged commit 8eab6aa into kubernetes:master Nov 13, 2019
@k8s-ci-robot k8s-ci-robot added this to the v1.17 milestone Nov 13, 2019
@robscott robscott deleted the endpointslice-e2e branch March 11, 2021 04:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dnardo dnardo Awaiting requested review from dnardo

1 more reviewer

@freehan freehan freehan left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@freehan freehan

@MrHohn MrHohn

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/test cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. release-note-none Denotes a PR that doesn't merit a release note. sig/network Categorizes an issue or PR as relevant to SIG Network. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

v1.17

Development

Successfully merging this pull request may close these issues.

6 participants

@robscott @irvifa @freehan @k8s-ci-robot @fejta-bot @MrHohn