Work-in-progress test implementation.

RobbieMcKinstry · RobbieMcKinstry · commit 454245720ec9 · 2026-01-15T18:39:24.000-05:00
diff --git a/.claude/skills/development-loop/bug-reports/HTTPRouteSimpleSameNamespace.md b/.claude/skills/development-loop/bug-reports/HTTPRouteSimpleSameNamespace.md
@@ -0,0 +1,110 @@
+# Bug Report: HTTPRouteSimpleSameNamespace
+
+## Test Description
+This conformance test validates basic HTTP routing from an HTTPRoute to a backend service in the same namespace. It creates:
+- A Gateway named `same-namespace` in `gateway-conformance-infra` namespace
+- An HTTPRoute named `gateway-conformance-infra-test` that routes to `infra-backend-v1:8080`
+- Makes an HTTP request to the Gateway and expects a 200 response from the backend
+
+## Issues Found
+
+### Issue 1: Missing ResolvedRefs Condition on Gateway Listeners (FIXED)
+
+**Failure Message:**
+```
+gateway gateway-conformance-infra/same-namespace doesn't have ResolvedRefs condition set to True on http listener
+```
+
+**Root Cause:**
+Gateway listeners only had the `Accepted` condition set, but Gateway API requires listeners to also have `ResolvedRefs` and `Programmed` conditions.
+
+**Fix Applied:**
+Added `ResolvedRefs` and `Programmed` conditions to `GatewayStatusListeners` in `crates/controlplane/src/core/reconcile.rs:288-333`.
+
+### Issue 2: Missing observedGeneration on HTTPRoute Conditions (FIXED)
+
+**Failure Message:**
+```
+HTTPRoute expected observedGeneration to be updated to 1 for all conditions, only 0/2 were updated. stale conditions are: Accepted (generation 0), ResolvedRefs (generation 0)
+```
+
+**Root Cause:**
+HTTPRoute parent status conditions had `observed_generation: None` instead of the route's generation.
+
+**Fix Applied:**
+Updated `build_parent_status` function in `crates/controlplane/src/core/reconcile.rs:672-713` to accept and use the route's generation.
+
+### Issue 3: ConfigMap Server-Side Apply Conflict (IN PROGRESS)
+
+**Failure Message:**
+```
+Failed to execute upsert error=Kubernetes API error: ApiError: Apply failed with 1 conflict: conflict with "unknown" using v1: .data.config.json
+```
+
+**Root Cause Analysis:**
+The controller uses Server-Side Apply (SSA) to update ConfigMaps containing data plane configuration. There's a conflict with an "unknown" field manager on the `.data.config.json` field.
+
+**Investigation Findings:**
+
+1. **Original Code Issue:** The upsert functions in `executor.rs` used a "get-then-create" pattern:
+   - If resource exists: Update with SSA using field manager "multiway-controller"
+   - If resource doesn't exist: Create with `api.create()` using `PostParams::default()` (no field manager = "unknown")
+
+   This meant ConfigMaps created by our controller were tagged with field manager "unknown", causing conflicts on subsequent SSA updates.
+
+2. **Attempted Fix:** Changed all upsert functions to use pure SSA with `PatchParams::apply("multiway-controller").force()`:
+   - SSA handles both creation and updates idempotently
+   - The `.force()` flag should force the controller to take ownership of conflicting fields
+
+3. **Current Status:** Even with `.force()` enabled, the SSA conflict persists:
+   ```
+   Apply failed with 1 conflict: conflict with "unknown" using v1: .data.config.json
+   ```
+
+**Remaining Questions:**
+1. **Is `.force()` being properly passed to the API server?** The kube-rs `PatchParams::apply().force()` should set `force: true` in the patch request, but this needs verification.
+
+2. **Source of "unknown" field manager:** Something is creating/modifying ConfigMaps with field manager "unknown" before our SSA update runs. Possible sources:
+   - Another controller or admission webhook
+   - Race condition between Gateway and HTTPRoute reconciliation
+   - The conformance test setup process
+
+3. **kube-rs SSA behavior:** According to Kubernetes docs, SSA with force should always succeed by taking over field ownership. If `.force()` isn't working as expected, there may be:
+   - A bug in kube-rs's SSA implementation
+   - Missing TypeMeta (apiVersion/kind) in the ConfigMap object being applied
+   - Serialization issues with the patch request
+
+**Proposed Next Steps:**
+1. Add debug logging to capture the exact HTTP request being sent for SSA patches
+2. Verify the ConfigMap object has proper TypeMeta fields set for SSA
+3. Check if the kube-rs `PatchParams::force()` method is correctly setting the force flag
+4. Test with `kubectl apply --server-side --force-conflicts` to confirm the API server accepts force apply
+
+**Files Modified (for this issue):**
+- `crates/controlplane/src/shell/executor.rs`:
+  - Removed `PostParams` import (no longer using `api.create()`)
+  - Changed all `upsert_*` functions to use pure SSA with `.force()`
+  - Lines 178-197: `upsert_deployment` now uses SSA
+  - Lines 199-214: `upsert_service` now uses SSA
+  - Lines 216-231: `upsert_configmap` now uses SSA with force
+  - Lines 233-248: `upsert_serviceaccount` now uses SSA
+
+## Test Status
+
+The status condition fixes are correct and working:
+- Gateway listener ResolvedRefs condition: PASSING
+- HTTPRoute observedGeneration on conditions: PASSING
+
+The HTTP request test fails due to the ConfigMap SSA conflict:
+- Simple HTTP request should reach infra-backend: FAILING (timeout due to ConfigMap not being updated)
+
+## Files Modified
+
+1. `crates/controlplane/src/core/reconcile.rs`:
+   - Lines 288-334: Added ResolvedRefs and Programmed conditions to listener status
+   - Lines 639-646: Pass route generation to build_parent_status
+   - Lines 672-713: Updated build_parent_status to accept and use generation parameter
+   - Lines 385-405: Updated build_configmap to explicitly set all fields
+
+2. `crates/controlplane/src/shell/executor.rs`:
+   - Lines 178-248: Changed all upsert functions from get-then-create to pure SSA with force
diff --git a/.claude/skills/development-loop/test-tiers/tier-1-essential.csv b/.claude/skills/development-loop/test-tiers/tier-1-essential.csv
@@ -1,5 +1,5 @@
 test_name,description,implemented
-HTTPRouteSimpleSameNamespace,Basic HTTP routing from a route to a backend service in the same namespace. Foundation of all routing.,false
+HTTPRouteSimpleSameNamespace,Basic HTTP routing from a route to a backend service in the same namespace. Foundation of all routing.,in-progress
 HTTPRouteMatching,Path and header matching for routing requests to different backends based on request criteria.,false
 HTTPRouteExactPathMatching,Exact path matching where /foo matches only /foo and not /foo/bar.,false
 HTTPRouteWeight,Traffic distribution across multiple backends based on specified weights for load balancing.,false
diff --git a/crates/controlplane/src/core/reconcile.rs b/crates/controlplane/src/core/reconcile.rs
@@ -289,14 +289,48 @@ fn build_gateway_status_with_routes(
                 name: l.name.clone(),
                 attached_routes,
                 supported_kinds,
-                conditions: vec![Condition {
-                    type_: "Accepted".to_string(),
-                    status: status_value.to_string(),
-                    observed_generation: gateway.metadata.generation,
-                    last_transition_time: time.clone(),
-                    reason: reason.to_string(),
-                    message: message.to_string(),
-                }],
+                conditions: vec![
+                    Condition {
+                        type_: "Accepted".to_string(),
+                        status: status_value.to_string(),
+                        observed_generation: gateway.metadata.generation,
+                        last_transition_time: time.clone(),
+                        reason: reason.to_string(),
+                        message: message.to_string(),
+                    },
+                    Condition {
+                        type_: "ResolvedRefs".to_string(),
+                        status: status_value.to_string(),
+                        observed_generation: gateway.metadata.generation,
+                        last_transition_time: time.clone(),
+                        reason: if accepted {
+                            "ResolvedRefs".to_string()
+                        } else {
+                            reason.to_string()
+                        },
+                        message: if accepted {
+                            "All references resolved".to_string()
+                        } else {
+                            message.to_string()
+                        },
+                    },
+                    Condition {
+                        type_: "Programmed".to_string(),
+                        status: status_value.to_string(),
+                        observed_generation: gateway.metadata.generation,
+                        last_transition_time: time.clone(),
+                        reason: if accepted {
+                            "Programmed".to_string()
+                        } else {
+                            "Invalid".to_string()
+                        },
+                        message: if accepted {
+                            "Listener is programmed".to_string()
+                        } else {
+                            message.to_string()
+                        },
+                    },
+                ],
             }
         })
         .collect();
@@ -364,7 +398,9 @@ fn build_configmap(names: &DataPlaneNames, config: &GatewayConfig, gateway: &Gat
             ..Default::default()
         },
         data: Some(data),
-        ..Default::default()
+        // Explicitly set immutable to None to avoid SSA conflicts with unmanaged fields
+        immutable: None,
+        binary_data: None,
     }
 }
 
@@ -608,6 +644,7 @@ pub fn reconcile_httproute(
             validation.accepted,
             validation.reason,
             &validation.message,
+            httproute.metadata.generation,
         );
         parent_statuses.push(parent_status);
 
@@ -641,6 +678,7 @@ fn build_parent_status(
     accepted: bool,
     reason: &str,
     message: &str,
+    generation: Option<i64>,
 ) -> HttpRouteStatusParents {
     let time = Time(now);
     let status_value = if accepted { "True" } else { "False" };
@@ -659,15 +697,15 @@ fn build_parent_status(
             Condition {
                 type_: "Accepted".to_string(),
                 status: status_value.to_string(),
-                observed_generation: None,
+                observed_generation: generation,
                 last_transition_time: time.clone(),
                 reason: reason.to_string(),
                 message: message.to_string(),
             },
             Condition {
                 type_: "ResolvedRefs".to_string(),
                 status: status_value.to_string(),
-                observed_generation: None,
+                observed_generation: generation,
                 last_transition_time: time,
                 reason: if accepted { "ResolvedRefs" } else { reason }.to_string(),
                 message: message.to_string(),
diff --git a/crates/controlplane/src/shell/executor.rs b/crates/controlplane/src/shell/executor.rs
@@ -9,7 +9,7 @@ use std::time::Duration;
 use gateway_crds::{Gateway, GatewayClass, HTTPRoute};
 use k8s_openapi::api::apps::v1::Deployment;
 use k8s_openapi::api::core::v1::{ConfigMap, Service, ServiceAccount};
-use kube::api::{DeleteParams, Patch, PatchParams, PostParams};
+use kube::api::{DeleteParams, Patch, PatchParams};
 use kube::runtime::controller::Action;
 use kube::{Api, Client};
 use tracing::{debug, error, info, warn};
@@ -175,7 +175,7 @@ impl ReconcileExecutor {
         Ok(())
     }
 
-    /// Upsert a Deployment
+    /// Upsert a Deployment using Server-Side Apply
     async fn upsert_deployment(&self, deployment: &Deployment) -> Result<()> {
         let namespace = deployment
             .metadata
@@ -185,100 +185,64 @@ impl ReconcileExecutor {
         let name = deployment.metadata.name.as_deref().unwrap();
         let api: Api<Deployment> = Api::namespaced(self.client.clone(), namespace);
 
-        match api.get(name).await {
-            Ok(_) => {
-                debug!(namespace, name, "Updating Deployment");
-                api.patch(
-                    name,
-                    &PatchParams::apply("multiway-controller"),
-                    &Patch::Apply(deployment),
-                )
-                .await?;
-            }
-            Err(kube::Error::Api(err)) if err.code == 404 => {
-                info!(namespace, name, "Creating Deployment");
-                api.create(&PostParams::default(), deployment).await?;
-            }
-            Err(e) => return Err(e.into()),
-        }
+        debug!(namespace, name, "Applying Deployment");
+        api.patch(
+            name,
+            &PatchParams::apply("multiway-controller").force(),
+            &Patch::Apply(deployment),
+        )
+        .await?;
 
         Ok(())
     }
 
-    /// Upsert a Service
+    /// Upsert a Service using Server-Side Apply
     async fn upsert_service(&self, service: &Service) -> Result<()> {
         let namespace = service.metadata.namespace.as_deref().unwrap_or("default");
         let name = service.metadata.name.as_deref().unwrap();
         let api: Api<Service> = Api::namespaced(self.client.clone(), namespace);
 
-        match api.get(name).await {
-            Ok(_) => {
-                debug!(namespace, name, "Updating Service");
-                api.patch(
-                    name,
-                    &PatchParams::apply("multiway-controller"),
-                    &Patch::Apply(service),
-                )
-                .await?;
-            }
-            Err(kube::Error::Api(err)) if err.code == 404 => {
-                info!(namespace, name, "Creating Service");
-                api.create(&PostParams::default(), service).await?;
-            }
-            Err(e) => return Err(e.into()),
-        }
+        debug!(namespace, name, "Applying Service");
+        api.patch(
+            name,
+            &PatchParams::apply("multiway-controller").force(),
+            &Patch::Apply(service),
+        )
+        .await?;
 
         Ok(())
     }
 
-    /// Upsert a ConfigMap
+    /// Upsert a ConfigMap using Server-Side Apply
     async fn upsert_configmap(&self, configmap: &ConfigMap) -> Result<()> {
         let namespace = configmap.metadata.namespace.as_deref().unwrap_or("default");
         let name = configmap.metadata.name.as_deref().unwrap();
         let api: Api<ConfigMap> = Api::namespaced(self.client.clone(), namespace);
 
-        match api.get(name).await {
-            Ok(_) => {
-                debug!(namespace, name, "Updating ConfigMap");
-                api.patch(
-                    name,
-                    &PatchParams::apply("multiway-controller"),
-                    &Patch::Apply(configmap),
-                )
-                .await?;
-            }
-            Err(kube::Error::Api(err)) if err.code == 404 => {
-                info!(namespace, name, "Creating ConfigMap");
-                api.create(&PostParams::default(), configmap).await?;
-            }
-            Err(e) => return Err(e.into()),
-        }
+        debug!(namespace, name, "Applying ConfigMap");
+        api.patch(
+            name,
+            &PatchParams::apply("multiway-controller").force(),
+            &Patch::Apply(configmap),
+        )
+        .await?;
 
         Ok(())
     }
 
-    /// Upsert a ServiceAccount
+    /// Upsert a ServiceAccount using Server-Side Apply
     async fn upsert_serviceaccount(&self, sa: &ServiceAccount) -> Result<()> {
         let namespace = sa.metadata.namespace.as_deref().unwrap_or("default");
         let name = sa.metadata.name.as_deref().unwrap();
         let api: Api<ServiceAccount> = Api::namespaced(self.client.clone(), namespace);
 
-        match api.get(name).await {
-            Ok(_) => {
-                debug!(namespace, name, "Updating ServiceAccount");
-                api.patch(
-                    name,
-                    &PatchParams::apply("multiway-controller"),
-                    &Patch::Apply(sa),
-                )
-                .await?;
-            }
-            Err(kube::Error::Api(err)) if err.code == 404 => {
-                info!(namespace, name, "Creating ServiceAccount");
-                api.create(&PostParams::default(), sa).await?;
-            }
-            Err(e) => return Err(e.into()),
-        }
+        debug!(namespace, name, "Applying ServiceAccount");
+        api.patch(
+            name,
+            &PatchParams::apply("multiway-controller").force(),
+            &Patch::Apply(sa),
+        )
+        .await?;
 
         Ok(())
     }
