gateway-api icon indicating copy to clipboard operation
gateway-api copied to clipboard

GatewayClass finalizer conformance test

Open mlavacca opened this issue 3 years ago • 2 comments

What would you like to be added:

The GatewayClass doc states that

// Whenever one or more Gateways are using a GatewayClass, implementations MUST
// add the `gateway-exists-finalizer.gateway.networking.k8s.io` finalizer on the
// associated GatewayClass. This ensures that a GatewayClass associated with a
// Gateway is not deleted while in use.

Since this is described as a MUST in the API, it should be checked by a conformance test that creates a GatewayClass and a Gateway that uses it. Once the Gateway is created, the existence of such a finalizer should be checked on the GatewayClass.

Why this is needed:

This is needed because the existence of such a finalizer is stated as mandatory.

mlavacca avatar Sep 27 '22 13:09 mlavacca

Can we consider changing the spec here? This is a bad idea IMO

howardjohn avatar Sep 27 '22 13:09 howardjohn

This has been the source of quite a bit of discussion before, even fairly recently. Probably would be good to check back in on it cc @robscott & @youngnick

shaneutt avatar Sep 27 '22 14:09 shaneutt

Just found this in my notifications, sorry for the delay.

@howardjohn could you explain more about why you think this is a bad idea? Currently, the GatewayClass is a required resource, so without a GatewayClass, Gateways must not be processed. So, removing the GatewayClass should also remove associated Gateways.

youngnick avatar Nov 30 '22 08:11 youngnick

I don't think finalizers are a good design in general; there are very rare cases they are useful but normally they just lead to frustration (see https://stackoverflow.com/questions/52369247/namespace-stuck-as-terminating-how-i-removed-it, one of the most upvoted questions for kubernetes). Its maybe a bit better in this case since its not a namespace scoped resource

howardjohn avatar Nov 30 '22 15:11 howardjohn

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-triage-robot avatar Feb 28 '23 16:02 k8s-triage-robot

The Kubernetes project currently lacks enough active 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 rotten
  • Close this issue with /close
  • Offer to help out with Issue Triage

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

/lifecycle rotten

k8s-triage-robot avatar Mar 30 '23 16:03 k8s-triage-robot

We must resolve this before GA, but it may be better to make this a SHOULD instead of a MUST as the way to resolve this, as saying that an implementation must implement a finalizer is not great. We would appreciate having a contributor help make the final decision and then make the update.

/help

shaneutt avatar Apr 05 '23 22:04 shaneutt

@shaneutt: This request has been marked as needing help from a contributor.

Guidelines

Please ensure that the issue body includes answers to the following questions:

  • Why are we solving this issue?
  • To address this issue, are there any code changes? If there are code changes, what needs to be done in the code and what places can the assignee treat as reference points?
  • Does this issue have zero to low barrier of entry?
  • How can the assignee reach out to you for help?

For more details on the requirements of such an issue, please see here and ensure that they are met.

If this request no longer meets these requirements, the label can be removed by commenting with the /remove-help command.

In response to this:

We must resolve this before GA, but it may be better to make this a SHOULD instead of a MUST as the way to resolve this, as saying that an implementation must implement a finalizer is not great. We would appreciate having a contributor help make the final decision and then make the update.

/help

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

k8s-ci-robot avatar Apr 05 '23 22:04 k8s-ci-robot