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.

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

Test different variants of K8s Services in conformance testing #1718

Closed
dprotaso opened this issue Feb 8, 2023 · 14 comments
Closed

Test different variants of K8s Services in conformance testing #1718

dprotaso opened this issue Feb 8, 2023 · 14 comments
Assignees
Labels
area/conformance kind/feature Categorizes issue or PR as related to a new feature. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.

Comments

@dprotaso
Copy link
Contributor

dprotaso commented Feb 8, 2023

What would you like to be added:

We should add conformance tests that ensure Gateway API works well with different variants of K8s Services, Endpoint, EndpointSlices

Short List Variants

  • Service - with selector
  • Service - with selector, endpoint slice mirror disabled
  • Headless Service - with manual endpoints (and their different types)
  • Headless Service - with manual endpoint slices (and their different types - ie. FQDN)

Why this is needed:

Different platforms that build on-top of K8s will utilize Services in different ways to suit their needs - it would be great to know Gateway conformance supports these variants. ie. some implementations currently don't track EndpointSlices which is pretty fundamental as they've been stable for numerous years

@dprotaso dprotaso added the kind/feature Categorizes issue or PR as related to a new feature. label Feb 8, 2023
@shaneutt shaneutt added this to the v0.6.2 milestone Feb 8, 2023
@shaneutt shaneutt added help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. labels Feb 15, 2023
@Xunzhuo
Copy link
Member

Xunzhuo commented Mar 8, 2023

/assign
/remove-help

@k8s-ci-robot k8s-ci-robot removed the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Mar 8, 2023
@shaneutt shaneutt moved this from Todo to In Progress in Gateway API: The Road to GA Mar 8, 2023
@shaneutt shaneutt modified the milestones: v0.6.2, v1.0.0 Mar 8, 2023
@dprotaso
Copy link
Contributor Author

dprotaso commented Mar 20, 2023

Related - #1816

@shaneutt Given ExternalName support is marked as SHOULD NOT - does it still warrant having an opt-in test? The purpose being for implementations (that support this type) having a common way to verify their support.

@shaneutt
Copy link
Member

Related - #1816

@shaneutt Given ExternalName support is marked as SHOULD NOT - does it still warrant having an opt-in test? The purpose being for implementations (that support this type) having a common way to verify their support.

Because we consider it Implementation-specific it sorta suggests the opposite of conformance: implementations can choose to implement that functionality in a way they deem appropriate and safe. I'm not saying that we should never have conformance tests that cover Implementation-specific behaviors, but I'm hesitant to say we need one here and wonder if that would make it feel more like "extended". What specifically did you have in mind?

@shaneutt
Copy link
Member

@Xunzhuo how are things going here? Need any assistance to move this one forward?

@shaneutt shaneutt removed this from the v1.0.0 milestone May 18, 2023
@dprotaso
Copy link
Contributor Author

@Xunzhuo are you still working on this?

@dprotaso
Copy link
Contributor Author

/unassign @Xunzhuo

@dprotaso
Copy link
Contributor Author

/assign @dprotaso

@dprotaso
Copy link
Contributor Author

dprotaso commented Feb 28, 2024

I've created a first pass of a test here - #2828

Currently the test exercises the following variants:

  1. Service - Headless (spec.clusterIP=None) with Selector
  2. Service - Headless no selector with a manual Endpoint (addressType: IPv4)
  3. Service - Headless no selector with a manual EndpointSlice (addressType: IPv4)
  4. Service - No selector with a manual Endpoint (addressType: IPv4)
  5. Service - No selector with a manual EndpointSlice (addressType: IPv4)

@dprotaso
Copy link
Contributor Author

dprotaso commented Feb 28, 2024

Ideally we can merge these variants and then discuss further tests to consider
eg.

  1. different address types - EndpointSlice:IP6
  2. Service - with selector, endpoint slice mirror disabled

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 28, 2024
@dprotaso
Copy link
Contributor Author

dprotaso commented Jun 4, 2024

Circling back what's done is

  1. Service with Manual EndpointSlices
  2. Headless K8s Services with Manual Endpointslices
  3. Headless Service

We dropped Endpoint testing (see: #2828 (comment)). New features will be added to EndpointSlice. Also Endpoints get mirrored to EndpointSlices.

The test works with both IP6 and IP4 clusters - it had to be conditional on the environment.

@dprotaso
Copy link
Contributor Author

dprotaso commented Jun 4, 2024

What's sorta left to discuss is what do we do with:

1. Service - with selector, endpoint slice mirror disabled See: #1718 (comment)
2. EndpointSlice with FQDN (using CoreDNS this becomes a CNAME - but it's sorta undefined) see: kubernetes/kubernetes#114210

@dprotaso
Copy link
Contributor Author

dprotaso commented Jun 4, 2024

re: 1 - @robscott's comment from the PR was

No, there's no reason for controllers to watch both Endpoints and EndpointSlice APIs. Gateway API is newer than the EndpointSlice API so hopefully all implementations are reading from EndpointSlices, but they definitely shouldn't be reading from both APIs.

Confirmed with @arkodg Envoy Gateway only watches endpoint slices
Confirmed with @sunjayBhatia that Contour watches either or but not both. Their default is endpointslices.

Probably best then to not add a test and I don't think it warrants any further API doc changes - watching endpointslices is now sorta implied via the conformance test.

@dprotaso
Copy link
Contributor Author

tl;dr From Gateway API Meeting Jun 10th

Support FQDN (similar to ExternalName) is too risky and can lead to confused deputy attacks. Hence it's not something the community wants to encourage.

I'm going to close out this issue since there's nothing else actionable here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/conformance kind/feature Categorizes issue or PR as related to a new feature. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
None yet
Development

No branches or pull requests

5 participants