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

docs: add IngressClassParams KEP #2536

Merged
merged 47 commits into from
Jun 27, 2022
Merged

docs: add IngressClassParams KEP #2536

merged 47 commits into from
Jun 27, 2022

Conversation

pmalek
Copy link
Member

@pmalek pmalek commented Jun 3, 2022

What this PR does / why we need it:

It defines a KEP for #1131

Which issue this PR fixes:

addresses #1131

Special notes for your reviewer:

Related PR with proposal of implementation can be found in #2535

@pmalek pmalek added area/docs area/kep Enhancment Proposals labels Jun 3, 2022
@pmalek pmalek requested a review from a team as a code owner June 3, 2022 14:03
@pmalek pmalek self-assigned this Jun 3, 2022
@pmalek pmalek temporarily deployed to Configure ci June 3, 2022 14:03 Inactive
@pmalek pmalek temporarily deployed to Configure ci June 3, 2022 14:03 Inactive
@pmalek pmalek temporarily deployed to Configure ci June 3, 2022 14:25 Inactive
Copy link
Contributor

@shaneutt shaneutt left a comment

Choose a reason for hiding this comment

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

Mostly LGTM, my main comment is about the graduation criteria please see the comments below.

Also are we aware of any end-users that have asked for this feature? As far as I can tell this has been driven entirely by maintainers historically. That doesn't mean it shouldn't exist, but I do think it's worth talking about, and maybe reflecting in the KEPs motivations.

keps/0009-ingress-class-params-crd.md Outdated Show resolved Hide resolved
keps/0009-ingress-class-params-crd.md Outdated Show resolved Hide resolved
keps/0009-ingress-class-params-crd.md Outdated Show resolved Hide resolved
keps/0009-ingress-class-params-crd.md Outdated Show resolved Hide resolved
keps/0009-ingress-class-params-crd.md Outdated Show resolved Hide resolved
keps/0009-ingress-class-params-crd.md Outdated Show resolved Hide resolved
keps/0009-ingress-class-params-crd.md Outdated Show resolved Hide resolved
Copy link
Contributor

@rainest rainest left a comment

Choose a reason for hiding this comment

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

Are we deferring design of the linked CR to the implementable KEP stage? That seems like it's the main new thing to consider beyond what's in the existing issue.

@pmalek
Copy link
Member Author

pmalek commented Jun 6, 2022

Mostly LGTM, my main comment is about the graduation criteria please see the comments below.

Also are we aware of any end-users that have asked for this feature? As far as I can tell this has been driven entirely by maintainers historically. That doesn't mean it shouldn't exist, but I do think it's worth talking about, and maybe reflecting in the KEPs motivations.

Well my understanding was that there was #1586 which was proposed by our user but the issue (#1131) was brought up by one of maintainers.

@pmalek pmalek temporarily deployed to Configure ci June 6, 2022 12:19 Inactive
@pmalek pmalek temporarily deployed to Configure ci June 6, 2022 12:20 Inactive
@pmalek pmalek temporarily deployed to Configure ci June 6, 2022 12:24 Inactive
@pmalek pmalek temporarily deployed to Configure ci June 6, 2022 12:24 Inactive
@pmalek pmalek temporarily deployed to Configure ci June 6, 2022 12:26 Inactive
@pmalek pmalek temporarily deployed to Configure ci June 6, 2022 12:26 Inactive
@pmalek pmalek temporarily deployed to Configure ci June 6, 2022 12:26 Inactive
@pmalek pmalek temporarily deployed to Configure ci June 6, 2022 12:26 Inactive
@pmalek pmalek temporarily deployed to Configure ci June 6, 2022 12:27 Inactive
@pmalek pmalek temporarily deployed to Configure ci June 6, 2022 12:29 Inactive
@pmalek pmalek temporarily deployed to Configure ci June 6, 2022 12:31 Inactive
@pmalek pmalek temporarily deployed to Configure ci June 6, 2022 12:53 Inactive
@shaneutt
Copy link
Contributor

shaneutt commented Jun 6, 2022

Mostly LGTM, my main comment is about the graduation criteria please see the comments below.
Also are we aware of any end-users that have asked for this feature? As far as I can tell this has been driven entirely by maintainers historically. That doesn't mean it shouldn't exist, but I do think it's worth talking about, and maybe reflecting in the KEPs motivations.

Well my understanding was that there was #1586 which was proposed by our user but the issue (#1131) was brought up by one of maintainers.

Good call, I was missing this piece of the puzzle. @larribas are you still interested in this feature, and would you have some time to weigh in here?

@pmalek pmalek temporarily deployed to Configure ci June 6, 2022 14:52 Inactive
dependabot bot and others added 11 commits June 24, 2022 22:37
Bumps [github.com/spf13/cobra](https:/spf13/cobra) from 1.4.0 to 1.5.0.
- [Release notes](https:/spf13/cobra/releases)
- [Commits](spf13/cobra@v1.4.0...v1.5.0)

---
updated-dependencies:
- dependency-name: github.com/spf13/cobra
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
If the IngressClass controller is disabled, also disable the additional
IngressClass watches in other controllers.
Increase the default proxy timeout to 30s.

Add a log indicating the increase timeout flag when the controller
exceeds the proxy timeout.
Bumps [google.golang.org/api](https:/googleapis/google-api-go-client) from 0.84.0 to 0.85.0.
- [Release notes](https:/googleapis/google-api-go-client/releases)
- [Changelog](https:/googleapis/google-api-go-client/blob/main/CHANGES.md)
- [Commits](googleapis/google-api-go-client@v0.84.0...v0.85.0)

---
updated-dependencies:
- dependency-name: google.golang.org/api
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Mattia Lavacca <[email protected]>
#2603)

Bumps [sigs.k8s.io/controller-runtime](https:/kubernetes-sigs/controller-runtime) from 0.12.0 to 0.12.2.
- [Release notes](https:/kubernetes-sigs/controller-runtime/releases)
- [Changelog](https:/kubernetes-sigs/controller-runtime/blob/master/RELEASE.md)
- [Commits](kubernetes-sigs/controller-runtime@v0.12.0...v0.12.2)

---
updated-dependencies:
- dependency-name: sigs.k8s.io/controller-runtime
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Rework managed Listener handling. The Gateway controller no longer
replaces user-provided Listeners with a set of Listeners derived from
the Kong proxy Service and listen configuration. It only checks to
ensure a Kong listen with the correct protocol is available for the
requested Listener. If there is none, it marks the Listener detached
because its protocol is unsupported, or because no protocol is
configured for the specific port.

Remove allowedRoutes merger. This is unnecessary with user-defined
Listeners preserved.

Remove utility functions used to compare Kong-derived and
Gateway-derived Listener sets. Comparisons against Kong configuration
now contribute to Listener compatibility checks on a per-Listen basis.

Remove any awareness of Listener history when calculating status other
than the attachedRoutes figure from Status. If any two Listeners result
in a conflict condition, mark all Listeners with that port or hostname
conflicted. Adding a new Listener in conflict with an existing Listener
will break the existing Listener and detach it.

Exclude ClusterIP addresses from Gateway address status. Only
LoadBalancer addresses, if present, will be advertised.
)

Bumps [github.com/stretchr/testify](https:/stretchr/testify) from 1.7.4 to 1.7.5.
- [Release notes](https:/stretchr/testify/releases)
- [Commits](stretchr/testify@v1.7.4...v1.7.5)

---
updated-dependencies:
- dependency-name: github.com/stretchr/testify
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@pmalek pmalek temporarily deployed to Configure ci June 24, 2022 20:41 Inactive
@pmalek pmalek temporarily deployed to Configure ci June 24, 2022 20:42 Inactive
@pmalek pmalek temporarily deployed to Configure ci June 24, 2022 20:43 Inactive
@pmalek pmalek temporarily deployed to Configure ci June 24, 2022 20:43 Inactive
@pmalek pmalek temporarily deployed to Configure ci June 24, 2022 20:44 Inactive
@pmalek pmalek temporarily deployed to Configure ci June 24, 2022 20:45 Inactive
@pmalek pmalek temporarily deployed to Configure ci June 24, 2022 20:45 Inactive
@pmalek pmalek requested a review from jrsmroz June 24, 2022 20:45
@pmalek pmalek temporarily deployed to Configure ci June 24, 2022 20:45 Inactive
@pmalek pmalek temporarily deployed to Configure ci June 25, 2022 00:11 Inactive
@pmalek pmalek temporarily deployed to Configure ci June 25, 2022 00:11 Inactive
@pmalek pmalek temporarily deployed to Configure ci June 25, 2022 00:30 Inactive
@pmalek pmalek merged commit ab4766a into main Jun 27, 2022
@pmalek pmalek deleted the kep-ingressclassparams branch June 27, 2022 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/docs area/kep Enhancment Proposals
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants