Skip to content

Conversation

@sjberman
Copy link
Collaborator

Problem: When hostNetwork was enabled, the data plane pod would connect and give us its hostname, which was the name of the node instead of the pod. This would cause issues internally with both tracking and acquiring the pod's owner so we know which config to send to it.

Solution: Add the owner's name and type via labels in the agent config. These labels are then sent to the control plane when an agent connects, and can be used to determine which configuration to send. Updated all trackin to use the UUID instead of pod name.

Testing: Enabling hostNetwork now allows the nginx pod to connect.

Closes #4426
Closes #4351

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

Release notes

If this PR introduces a change that affects users and needs to be mentioned in the release notes,
please add a brief note that summarizes the change.

Fix an issue where nginx pod could not connect to control plane when hostnetwork is enabled.

@github-actions github-actions bot added the bug Something isn't working label Dec 17, 2025
@codecov
Copy link

codecov bot commented Dec 17, 2025

Codecov Report

❌ Patch coverage is 80.16529% with 24 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.29%. Comparing base (e8f29de) to head (c23243b).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
internal/controller/nginx/agent/command.go 77.31% 19 Missing and 3 partials ⚠️
internal/controller/provisioner/provisioner.go 0.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4481      +/-   ##
==========================================
- Coverage   86.30%   86.29%   -0.02%     
==========================================
  Files         132      132              
  Lines       14849    14859      +10     
  Branches       35       35              
==========================================
+ Hits        12816    12823       +7     
- Misses       1810     1816       +6     
+ Partials      223      220       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@sjberman sjberman requested a review from Copilot December 17, 2025 17:51
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a connectivity issue where nginx pods using hostNetwork could not properly connect to the control plane. The problem occurred because pods reported their node hostname instead of pod name, breaking internal tracking and configuration routing.

Key changes:

  • Added owner name and type labels to agent configuration to identify the parent deployment/daemonset
  • Replaced pod name-based tracking with UUID-based tracking throughout the connection management system
  • Simplified pod owner resolution by extracting owner information directly from agent labels

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
internal/controller/provisioner/provisioner.go Initialize AgentLabels map to prevent nil pointer issues
internal/controller/provisioner/objects_test.go Add AgentLabels initialization to all test fixtures
internal/controller/provisioner/objects.go Populate agent labels with owner name and type, remove redundant nil check
internal/controller/nginx/types/types.go Define constants for agent owner labels and deployment types
internal/controller/nginx/agent/grpc/connections_test.go Update Connection struct usage to use ParentName instead of PodName
internal/controller/nginx/agent/grpc/connections.go Replace PodName field with ParentType and ParentName in Connection struct
internal/controller/nginx/agent/file_test.go Update tests to use ParentName and remove PodName references
internal/controller/nginx/agent/file.go Change connection validation to check InstanceID instead of PodName
internal/controller/nginx/agent/command_test.go Replace pod-based test fixtures with deployment-based ones, update all connection tracking to use UUID
internal/controller/nginx/agent/command.go Replace getPodOwner logic with agent label extraction, switch all pod name tracking to UUID-based tracking

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@sjberman sjberman marked this pull request as ready for review December 17, 2025 18:23
@sjberman sjberman requested a review from a team as a code owner December 17, 2025 18:23
Problem: When hostNetwork was enabled, the data plane pod would connect and give us its hostname, which was the name of the node instead of the pod. This would cause issues internally with both tracking and acquiring the pod's owner so we know which config to send to it.

Solution: Add the owner's name and type via labels in the agent config. These labels are then sent to the control plane when an agent connects, and can be used to determine which configuration to send. Updated all trackin to use the UUID instead of pod name.
@sjberman sjberman enabled auto-merge (squash) December 17, 2025 19:34
@sjberman sjberman merged commit e8ee7c1 into main Dec 17, 2025
60 of 61 checks passed
@sjberman sjberman deleted the fix/hostname branch December 17, 2025 20:04
@github-project-automation github-project-automation bot moved this from 🆕 New to ✅ Done in NGINX Gateway Fabric Dec 17, 2025
sjberman added a commit that referenced this pull request Dec 17, 2025
Problem: When hostNetwork was enabled, the data plane pod would connect and give us its hostname, which was the name of the node instead of the pod. This would cause issues internally with both tracking and acquiring the pod's owner so we know which config to send to it.

Solution: Add the owner's name and type via labels in the agent config. These labels are then sent to the control plane when an agent connects, and can be used to determine which configuration to send. Updated all trackin to use the UUID instead of pod name.
sjberman added a commit that referenced this pull request Dec 17, 2025
Problem: When hostNetwork was enabled, the data plane pod would connect and give us its hostname, which was the name of the node instead of the pod. This would cause issues internally with both tracking and acquiring the pod's owner so we know which config to send to it.

Solution: Add the owner's name and type via labels in the agent config. These labels are then sent to the control plane when an agent connects, and can be used to determine which configuration to send. Updated all trackin to use the UUID instead of pod name.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working release-notes

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Hostnetwork not allowing Pod to connect use hostnetwork mode can't connect to control plane

5 participants