Conditions

Author: sjberman

docs/proposals/gateway-settings.md

@@ -79,7 +79,7 @@ type GatewaySettingsSpec struct {
 type Telemetry struct {
 	// Exporter specifies OpenTelemetry export parameters.
 	// +optional
-	Exporter *Exporter `json:"exporter,omitempty"`
+	Exporter *TelemetryExporter `json:"exporter,omitempty"`
 
 	// ServiceName is the "service.name" attribute of the OpenTelemetry resource.
 	// Default is 'nginx-gateway-fabric:<gateway-name>'.
@@ -91,8 +91,8 @@ type Telemetry struct {
 	SpanAttributes map[string]string `json:"spanAttributes,omitempty"`
 }
 
-// Exporter specifies OpenTelemetry export parameters.
-type Exporter struct {
+// TelemetryExporter specifies OpenTelemetry export parameters.
+type TelemetryExporter struct {
 	// Interval is the maximum interval between two exports, by default is 5 seconds.
 	// +optional
 	Interval *Duration `json:"interval,omitempty"`
@@ -122,7 +122,7 @@ type Duration string
 
 > Note: This is subject to change, will not merge until ironed out.
 
-In order to be consistent with other resources with object refs, we should add the `ResolvedRefs` Condition to the `GatewayClass` when the `parametersRef` is in use. If the `GatewaySettings` config is invalid, then the condition is set to `False` with a reason why.
+There are two Conditions on the GatewayClass status to consider when using the `parametersRef`. The first is a `ResolvedRefs` Condition. If the `GatewaySettings` reference cannot be found, then this Condition is set to `False`.
 
 NGINX Gateway Fabric must set this Condition on the GatewayClass affected by a `GatewaySettings`.
 Below is an example of what this Condition may look like:
@@ -142,6 +142,12 @@ Some additional rules:
 - When the `GatewaySettings` affecting that object is removed, the Condition should be removed.
 - The Observed Generation is the generation of the GatewayClass, not the generation of the `GatewaySettings`.
 
+The other condition is the existing `Accepted` condition on the GatewayClass. There is an existing reason for this Condition, `InvalidParameters`, that sets `Accepted` to `False`  when the `parametersRef` fields are invalid. However, this could lead to downstream problems. For example, if a GatewayClass is `Accepted`, and then `GatewaySettings` are updated to something invalid, then marking the GatewayClass as `not Accepted` would result in the entire downstream configuration tree being nullified. This is a large disruption.
+
+The proposition is to instead keep the GatewayClass as `Accepted` even if the `GatewaySettings` are invalid, but still set the reason to `InvalidParameters` and include a detailed message about the issue. In this case, default values will be reverted to for the settings, and downstream configuration will remain intact. There may be impact by reverting to defaults, but this impact is likely less than the impact of completely nullifying all Gateways/Routes/etc. that live under the GatewayClass if we instead marked it as `not Accepted`.
+
+If this scenario occurs, we must be clear about what happened. Log an error, an event, and write the status.
+
 ## Use Cases
 
 - As a Cluster Operator, I want to set global settings that will apply to all Gateways that are a part of a GatewayClass. These settings should not be overriden at a lower level.