Skip to content

[GEP-1651]: Updated Types & Move to Experimental #2157

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
Merged
merged 19 commits into from
Jul 11, 2023
Merged

[GEP-1651]: Updated Types & Move to Experimental #2157

merged 19 commits into from
Jul 11, 2023

Conversation

@dprotaso
Copy link
Contributor

@dprotaso dprotaso commented Jun 28, 2023 •
edited
Loading

This expects the following to be merged first: #2144

See just the commit for just this PR - f9d4bbf

What type of PR is this?
/kind gep
/kind feature

What this PR does / why we need it:

  • This updates the v1beta1 types to allow implementations to start experimenting with GEP-1651
  • Update GEP-1651 to status Experimental

Which issue(s) this PR fixes:

Part of #1651

Does this PR introduce a user-facing change?:

Add Routability support to v1beta1 API types

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/gep PRs related to Gateway Enhancement Proposal(GEP) kind/feature Categorizes issue or PR as related to a new feature. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jun 28, 2023
@k8s-ci-robot k8s-ci-robot requested review from bowei and robscott June 28, 2023 19:44
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jun 28, 2023
robscott
robscott reviewed Jun 29, 2023
View reviewed changes
Copy link
Member

@robscott robscott left a comment

Choose a reason for hiding this comment

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

Thanks @dprotaso!

robscott

This comment was marked as duplicate.

Sign in to view

@dprotaso
Copy link
Contributor Author

dprotaso commented Jun 29, 2023 •
edited
Loading

/hold

need to add the inline validation and drop the godoc linking

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 29, 2023
@robscott robscott mentioned this pull request Jun 30, 2023
Closed
robscott
robscott reviewed Jun 30, 2023
View reviewed changes
Copy link
Member

@robscott robscott left a comment

Choose a reason for hiding this comment

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

Thanks @dprotaso!

config/crd/standard/gateway.networking.k8s.io_gateways.yaml Outdated
Comment on lines 1184 to 1190
from a reserved pool. \n Implementations that support GatewayRoutability
MUST include an address that has the same routable semantics as
defined in the Gateway spec. \n Implementations MAY add additional
addresses in status, but they MUST be semantically less than the
scope of the requested scope. For example if a user requests a `Private`
routable Gateway then an additional address MAY have a routability
of `Cluster` but MUST NOT include `Private`."
Copy link
Member

@robscott robscott Jun 30, 2023

Choose a reason for hiding this comment

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

Not a blocker for this PR, but unfortunate that we don't have a way to limit spec changes like this to experimental CRDs yet: #2160.

Copy link
Contributor Author

@dprotaso dprotaso Jun 30, 2023

Choose a reason for hiding this comment

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

Related: kubernetes-sigs/controller-tools#705

We could prune fields based on markers

Copy link
Contributor Author

@dprotaso dprotaso Jun 30, 2023

Choose a reason for hiding this comment

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

Though controller-tools folks don't want to maintain such a feature - so you could do some post-processing of the CRDs

Copy link
Member

@robscott robscott Jun 30, 2023

Choose a reason for hiding this comment

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

Yep, we are already doing some custom generation here (see #2160) we just need to extend it further to support this use case.

@robscott robscott added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Jun 30, 2023
@robscott
Copy link
Member

robscott commented Jun 30, 2023

Thanks @dprotaso! Will leave hold in place for someone else to review + approve, but LGTM.

/approve

@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented Jun 30, 2023

[APPROVALNOTIFIER] This PR is APPROVED

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

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 Jun 30, 2023
@dprotaso
Copy link
Contributor Author

dprotaso commented Jun 30, 2023

FYI - you can configure prow to squash everything by default so you don't need to add a label

@dprotaso dprotaso mentioned this pull request Jul 7, 2023
Merged
@robscott
Copy link
Member

robscott commented Jul 11, 2023

Thanks @dprotaso! Please remove the hold when #2144 merges.

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 11, 2023
@robscott
Copy link
Member

robscott commented Jul 11, 2023

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 11, 2023
@k8s-ci-robot k8s-ci-robot merged commit 7c4f74c into kubernetes-sigs:main Jul 11, 2023
@dprotaso dprotaso deleted the gep-1651 branch July 11, 2023 18:05
@shaneutt shaneutt mentioned this pull request Jul 24, 2023
Closed
@shaneutt shaneutt mentioned this pull request Aug 7, 2023
Closed
@shaneutt shaneutt mentioned this pull request Sep 18, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@robscott robscott robscott left review comments

@bowei bowei Awaiting requested review from bowei

Assignees

@robscott robscott

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. kind/feature Categorizes issue or PR as related to a new feature. kind/gep PRs related to Gateway Enhancement Proposal(GEP) lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@dprotaso @robscott @k8s-ci-robot