Skip to content

fix(main): drop forced --allow-untagged-cloud requirement to silence deprecation warning#1244

Open
arbianshkodra wants to merge 3 commits into
hetznercloud:mainfrom
arbianshkodra:fix/has-cluster-id-true
Open

fix(main): drop forced --allow-untagged-cloud requirement to silence deprecation warning#1244
arbianshkodra wants to merge 3 commits into
hetznercloud:mainfrom
arbianshkodra:fix/has-cluster-id-true

Conversation

@arbianshkodra
Copy link
Copy Markdown

@arbianshkodra arbianshkodra commented May 17, 2026

What

Removes the fail-fast in cloudInitializer (main.go) that forced operators to pass --allow-untagged-cloud. HasClusterID() is left unchanged (still returns false).

Why

The kubernetes/cloud-provider library deprecated --allow-untagged-cloud and emits a startup warning whenever the flag is set:

Flag --allow-untagged-cloud has been deprecated, This flag is deprecated and will be removed in a future release. A cluster-id will be required on cloud instances.

For Hetzner, no ClusterID is ever consumed by the cloud provider — confirmed in #1119:

We don't use cluster-ids anywhere in the code. Neither has allow-untagged-cloud any relevance for us. ... You can safely ignore this message from the logs. We may want to update our code here and don't emit any messages.
@lukasmetzner

Because HasClusterID() returns false, cloudInitializer would klog.Fatalf on startup unless --allow-untagged-cloud was set. Operators were therefore forced to pass the flag — and passing the (deprecated) flag is exactly what triggered the upstream warning. The flag was functionally a no-op for Hetzner; its mere presence caused the warning.

By removing the fail-fast requirement, operators can omit the flag entirely and the deprecation warning disappears at its source (the upstream library only emits it when the flag is set).

Why not change HasClusterID()?

The initial approach changed HasClusterID() to return true. Per review feedback from @lukasmetzner, that value is part of the upstream cloud-provider interface and changing it could cause unexpected behavior if consumed upstream. This revision keeps the interface untouched and instead drops the locally-owned fail-fast in main.go.

Behavior change

Before After
Operator must set --allow-untagged-cloud or HCCM crashes on startup Operator can omit the flag
Deprecation warning fires when the flag is set (every install today) No warning fires when the flag is omitted
HasClusterID() returns false HasClusterID() returns false (unchanged)

Operators who keep --allow-untagged-cloud set will continue to see the deprecation warning (the upstream library still notices the flag) — but they no longer need the flag.

Testing

  • go build ./... passes
  • go test ./hcloud/ -run TestCloud passes

Closes #1119

…rning

The kubernetes/cloud-provider library deprecated --allow-untagged-cloud
and emits a startup warning whenever the flag is set:

    Flag --allow-untagged-cloud has been deprecated, This flag is
    deprecated and will be removed in a future release. A cluster-id
    will be required on cloud instances.

For Hetzner specifically, no ClusterID is ever consumed by the cloud
provider — issue hetznercloud#1119 closed Jan 2026 with maintainer confirmation:

    "We don't use cluster-ids anywhere in the code. Neither has
    allow-untagged-cloud any relevance for us. ... You can safely
    ignore this message from the logs."

Because HasClusterID() was hardcoded to return false, operators were
forced to pass --allow-untagged-cloud just to get past the fail-fast
check in main.go:73-80 — which then triggered the deprecation warning
from upstream. The flag is functionally a no-op for Hetzner, but its
presence is what causes the warning.

Returning true here states the truth (Hetzner does not use ClusterID,
treats every cluster as "tagged enough" for its purposes) and lets
operators omit the flag entirely, which silences the warning at its
source. Behavior is otherwise unchanged.

Closes hetznercloud#1119
@arbianshkodra arbianshkodra requested a review from a team as a code owner May 17, 2026 13:47
@lukasmetzner
Copy link
Copy Markdown
Contributor

Hey @arbianshkodra,

I'd prefer not to change the value returned by HasClusterID(), since it's part of the cloud provider interface we implement and could cause unexpected behavior if used upstream.

That said, I'm fine with removing the startup warning, as I don't see much upstream movement toward using cluster IDs.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 29, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 67.17%. Comparing base (547798d) to head (43b4e29).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1244      +/-   ##
==========================================
- Coverage   71.01%   67.17%   -3.84%     
==========================================
  Files          24       24              
  Lines        2677     2672       -5     
==========================================
- Hits         1901     1795     -106     
- Misses        604      706     +102     
+ Partials      172      171       -1     
Flag Coverage Δ
e2e ?
unit 67.17% <ø> (+0.12%) ⬆️

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.

@lukasmetzner
Copy link
Copy Markdown
Contributor

The e2e tests failing is expected here, due to our permissions.

@arbianshkodra
Copy link
Copy Markdown
Author

@lukasmetzner, understood. Should I update this?

@lukasmetzner
Copy link
Copy Markdown
Contributor

Yes, feel free update this PR.

Per review on hetznercloud#1244, keep HasClusterID() returning false (it is part of
the upstream cloud-provider interface) and instead remove the fail-fast
in cloudInitializer that forced operators to pass --allow-untagged-cloud.

That forced flag was the only thing that triggered the upstream pflag
deprecation warning. With the requirement gone, operators omit the flag
and the warning disappears at its source, without changing the interface.

This reverts the cloud.go/cloud_test.go changes from 6caccaa.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@arbianshkodra arbianshkodra changed the title fix(cloud): make HasClusterID() return true to silence deprecation warning fix(main): drop forced --allow-untagged-cloud requirement to silence deprecation warning May 29, 2026
@arbianshkodra
Copy link
Copy Markdown
Author

Done. Kept HasClusterID() as-is and dropped the forced --allow-untagged-cloud requirement in main.go instead. Warning's gone.

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.

HCCM: "--allow-untagged-cloud=false" and cluster labeling / tagging

2 participants