search

syself / cluster-api-provider-hetzner

Cluster API Provider Hetzner :rocket: The best way to manage Kubernetes clusters on Hetzner, fully declarative, Kubernetes-native and with self-healing capabilities
https://caph.syself.com
Apache License 2.0
589 stars 57 forks source link

avoid warnings around finalizers #1248

Open kranurag7 opened 4 months ago

kranurag7 commented 4 months ago

/kind feature

Describe the solution you'd like [A clear and concise description of what you want to happen.]

We get the following warnings in the logs. This issue tracks fixing the same and removing this warning. 

{"level":"INFO","time":"2024-04-04T18:35:42.381Z","logger":"KubeAPIWarningLogger","file":"log/warning_handler.go:65","message":"metadata.finalizers: \"hetznerba
remetalhost.infrastructure.cluster.x-k8s.io\": prefer a domain-qualified finalizer name to avoid accidental conflicts with other finalizer writers"}

Anything else you would like to add: [Miscellaneous information that will assist in solving the issue.]

Environment:

janiskemper commented 4 months ago

can you take care of this one @kranurag7 ?

guettli commented 1 week ago

I agree, this message is still there.

but why is this not valid: "hetznerbaremetalhost.infrastructure.cluster.x-k8s.io" ?

These are 52 chars. Afaik 63 are allowed. Why does this message appear? ....

I found the answer.

it should be foo.example.com/bar. The /bar part is missing. Docs

We used the naming pattern which was used by CAPI. There is an open issue:

https://github.com/kubernetes-sigs/cluster-api/issues/10914

This is a lot of effort with little benefit.

Changing this does not feel productive. My opinion (posted on upstream issue here)

This is a lot of effort and only a little benefit. Maybe we could update the upstream code which generates the error message? I don't think the current name (without a slash) is a real problem.

kranurag7 commented 1 week ago

closing this reading the linked issue above.

guettli commented 1 week ago

capi maintainers replied to my message.

CAPI will do the change, and then we will do it, too.