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

Support setting a default GatewayClass #262

Closed
robscott opened this issue Aug 12, 2020 · 11 comments
Closed

Support setting a default GatewayClass #262

robscott opened this issue Aug 12, 2020 · 11 comments
Labels
area/webhook kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.

Comments

@robscott
Copy link
Member

robscott commented Aug 12, 2020

This should mirror IngressClass behavior:

You can mark a particular IngressClass as default for your cluster. Setting the ingressclass.kubernetes.io/is-default-class annotation to true on an IngressClass resource will ensure that new Ingresses without an ingressClassName field specified will be assigned this default IngressClass.

Unfortunately this would mean that we'd need a mutating webhook to implement this.

@robscott robscott added the kind/feature Categorizes issue or PR as related to a new feature. label Aug 12, 2020
@hbagdi
Copy link
Contributor

hbagdi commented Aug 12, 2020

Don't you also need a mutating webhook to inject the default if a class is not set?

@robscott
Copy link
Member Author

🤦 Thanks! That's what I meant to say, updated the original comment so it's more clear now.

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/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 Dec 24, 2020
@hbagdi
Copy link
Contributor

hbagdi commented Dec 28, 2020

/remove-lifecycle stale
/lifecycle frozen

@k8s-ci-robot k8s-ci-robot added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Dec 28, 2020
@hbagdi
Copy link
Contributor

hbagdi commented Aug 23, 2021

@robscott Is adding a default considered a breaking change? In some cases yes, here I think we should be good. Want to confirm.

@robscott
Copy link
Member Author

Yeah I don't think this would be a breaking change. This would require:

  1. Supporting a new annotation on GatewayClass to mark it as default.
  2. Making GatewayClassName on Gateway optional

When GatewayClassName was unset, a webhook would fill it in with a default value. If there was no default, maybe we'd have to reject the Gateway entirely? I'm not exactly sure on the specifics here, but it does not seem like it would be a breaking change to me.

@shaneutt
Copy link
Member

Just out of curiosity is the onus to implement this based on a specific implementation's needs or is it an effort towards further feature parity with Ingress?

@shaneutt
Copy link
Member

While grooming we saw that this one was open for a long period of time without anyone with a strong use case to champion it. We're going to close this as we don't expect anyone's ready to drive it forward, but if you still want this feature and have a strong use case we will be happy to reconsider this and re-open.

@shaneutt shaneutt closed this as not planned Won't fix, can't repro, duplicate, stale Jul 21, 2022
@james-callahan
Copy link

I'd still love to see this feature.

It shouldn't be up to the ClusterOperator to know the name of the GatewayClass chosen by the Infrastructure Providers.

In the more general case, it means there's no sensible default for any Gateway specification: if it's part of a kustomize application or helm template then it has to be fully plumbed through and a value provided.

@shaneutt
Copy link
Member

It shouldn't be up to the ClusterOperator to know the name of the GatewayClass chosen by the Infrastructure Providers.

An understandable position (and it might be worth noting that we're discussing a similar topic about roles, and which should have to know about what over in #3385).

There's a proposal in there to simply provide a default. Would that potentially help here as well? Definitely something worth thinking about 🤔

@youngnick
Copy link
Contributor

GatewayClass processing is more complicated than the Gateway parentRef, because it's cluster-scoped.

That said, it might be possible to change this, but would require replicating some functionality like IngressClass: having a way to mark a GatewayClass as the default for the cluster (with rules for which wins when multiple set it), and then making Gateway's 'spec.gatewayClassName` field optional (with the understanding that not specifying a GatewayClass when there is no default will do nothing).

Again, the reason why we didn't do this before is it is risky. If Ian the infra admin changes the default GatewayClass, then all non-specified Gateways will need to be re-provisioned, which probably would involve an outage. That makes changing that field a very risky operation.

Basically, every default we add adds magic, and magic adds risk. Explicitly specifying things removes risk, because the owner of a resource needs to take definite action for a change to occur.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/webhook kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.
Projects
None yet
Development

No branches or pull requests

7 participants