Skip to content

Conversation

@lukasmetzner
Copy link
Contributor

@lukasmetzner lukasmetzner commented Nov 12, 2025

When merging our two binaries into one, the option to configure
the location on startup via KUBE_NODE_NAME or HCLOUD_SERVER_ID was not implemented.

Missing configuration options in docs:

2. The location is derived by querying a server specified by the `HCLOUD_SERVER_ID` variable.
3. If neither of the above is set, the `KUBE_NODE_NAME` environment variable defaults to the name of the node where the CSI controller is scheduled. This node name is then used to query the Hetzner API for a matching server and its location.

Fixes #1149

@lukasmetzner lukasmetzner requested a review from a team as a code owner November 12, 2025 09:36
@lukasmetzner lukasmetzner marked this pull request as draft November 12, 2025 09:36
@lukasmetzner lukasmetzner force-pushed the fix-missing-location-evaluation branch from feaa715 to 83ccec2 Compare November 12, 2025 09:42
@codecov
Copy link

codecov bot commented Nov 12, 2025

Codecov Report

❌ Patch coverage is 37.50000% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.64%. Comparing base (8048835) to head (3ea107e).

Files with missing lines Patch % Lines
cmd/main.go 25.00% 6 Missing and 3 partials ⚠️
internal/app/app.go 50.00% 3 Missing and 3 partials ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #1158       +/-   ##
===========================================
+ Coverage   37.62%   71.64%   +34.01%     
===========================================
  Files          17       17               
  Lines        1483     1481        -2     
===========================================
+ Hits          558     1061      +503     
+ Misses        905      336      -569     
- Partials       20       84       +64     
Flag Coverage Δ
e2e-controller 58.60% <37.50%> (?)
e2e-node 58.60% <37.50%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

@jooola
Copy link
Member

jooola commented Nov 12, 2025

Is there an easy way to test this code?

@lukasmetzner
Copy link
Contributor Author

Is there an easy way to test this code?

I could think of an integration test where we spin up a test server and a fake metadata server, to test all four paths of GetServerLocation.

@lukasmetzner lukasmetzner force-pushed the fix-missing-location-evaluation branch from 1fcd72a to a46ea57 Compare November 14, 2025 08:33
@lukasmetzner lukasmetzner marked this pull request as ready for review November 14, 2025 08:33
@lukasmetzner lukasmetzner force-pushed the fix-missing-location-evaluation branch from ff55460 to bb65065 Compare November 14, 2025 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(regression): merging controller and csi node broke app in ipv6 only setups

3 participants