Skip to content

🌱 update controller runtime and cluster-api to newest version #1623

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 35 commits into
base: main
Choose a base branch
from

Conversation

cgroschupp
Copy link

@cgroschupp cgroschupp commented Jun 26, 2025

What this PR does / why we need it:
Updated controller-runtime to 0.21.0 and cluster-api to 1.10.3.

To get the newest from controller-runtime, for example the priority queue and support k8s 1.32 and 1.33

@guettli
Copy link
Collaborator

guettli commented Jul 10, 2025

@cgroschupp I created a PR based on yours. 🌱 update controller runtime and cluster-api to newest version by guettli · Pull Request #1628 · syself/cluster-api-provider-hetzner

I get a strange linting error in my PR

ERRO Running error: can't run linter goanalysis_metalinter
buildir: failed to load package conversion: could not load export data: no export data for "sigs.k8s.io/cluster-api/util/conversion" 

I guess we need to update from golangci-lint 1.x to 2.x

Does make lint work for you?

@guettli
Copy link
Collaborator

guettli commented Jul 10, 2025

@cgroschupp thank you for your PR. Currently the CI for Linting and Tests fail. I updated your PR in my PR (which is based on your version): 🌱 update controller runtime and cluster-api to newest version by guettli · Pull Request #1628 · syself/cluster-api-provider-hetzner

I was able to fix the golangci-lint error by syncing the dependencies to the version used by cluster-api (k8s and cel).

Currently we are unsure how to proceed. The linter complains about a lot of small things. Either we work on that, or you do it.

Do you have time during the next days to work on the PR?

@guettli
Copy link
Collaborator

guettli commented Jul 30, 2025

@cgroschupp thank you for your initial PR. I will continue with it here: #1628

@guettli guettli closed this Jul 30, 2025
@cgroschupp
Copy link
Author

cgroschupp commented Jul 30, 2025

@guettli Sorry, I was on holiday. Now 'make lint' is running.

see here: https://github.com/cgroschupp/cluster-api-provider-hetzner/tree/update-controller-runtime

@guettli
Copy link
Collaborator

guettli commented Jul 30, 2025

Have you merged the changes of my PR into yours?

@cgroschupp
Copy link
Author

cgroschupp commented Jul 30, 2025

I don't have write access to your branch. I added some commits to my branch.

@guettli
Copy link
Collaborator

guettli commented Jul 31, 2025

@cgroschupp please base on my branch. Tell me if there are reasons against doing it. Then please continue on your branch. Ping me, when you are ready. Then I will merge your branch into mine, so that we run the e2e tests.

I am sorry, it is a bit complicated, because currently e2e tests are not available for external PRs.

@guettli
Copy link
Collaborator

guettli commented Jul 31, 2025

reopening.

tdabasinskas and others added 3 commits July 31, 2025 10:51
…yself#1635)

build(tools): make helm and hcloud downloads platform-agnostic

Replace hardcoded `linux-amd64` architecture with dynamic `$(go env GOOS)-$(go env GOARCH)` to support multiple platforms and architectures during tool installation.

Signed-off-by: Tomas Dabašinskas <[email protected]>
…yself#1637)

* 🌱 Add SkipCreatingHetznerSecretInWorkloadCluster to hetznercluster_controller

SkipCreatingHetznerSecretInWorkloadCluster indicates whether the Hetzner secret should be created in the workload cluster. By default the secret gets created, so that the ccm (running in the wl-cluster) can use that secret. If you prefer to not reveal the secret in the workload cluster, you can set this to value to false, so that the secret is not created. Be sure to run the ccm outside of the workload cluster in that case, e.g. in the management cluster.

Closes syself#1636
@cgroschupp
Copy link
Author

@guettli rebase is done.

Two tests are currently failing. I need some time to find the problem. Perhaps you have an idea.

Summarizing 2 Failures:
  [FAIL] HetznerBareMetalMachineReconciler Tests with host Basic hbmm test [It] checks the hetznerBareMetalMachine status running phase
  /Users/christian/git/cluster-api-provider-hetzner/controllers/hetznerbaremetalmachine_controller_test.go:453
  [FAIL] HetznerBareMetalMachineReconciler Test EnqueueRequestsFromMapFunc BareMetalHostToBareMetalMachines [It] should enqueue the second HetznerBareMetalMachine, when the first gets deleted
  /Users/christian/git/cluster-api-provider-hetzner/controllers/hetznerbaremetalmachine_controller_test.go:1001

Ran 114 of 114 Specs in 50.358 seconds
FAIL! -- 112 Passed | 2 Failed | 0 Pending | 0 Skipped

@guettli guettli reopened this Jul 31, 2025
@guettli
Copy link
Collaborator

guettli commented Jul 31, 2025

@cgroschupp The good news: These two errors are stable. You have them, they are in CI, and I have them locally. I have a look at it now.

@guettli
Copy link
Collaborator

guettli commented Jul 31, 2025

@guettli
Copy link
Collaborator

guettli commented Jul 31, 2025

@cgroschupp it would be great, if you could have a look at this deprecation: remove "sigs.k8s.io/cluster-api/errors": Deprecate ancient errors/ package · Issue #10784 · kubernetes-sigs/cluster-api

guettli and others added 2 commits July 31, 2025 21:15
Context: kubernetes-sigs/cluster-api#10784
We'll maintain them here for the time being, until we have conditions replacing these errors.
@cgroschupp
Copy link
Author

@guettli 427cb76 this is only a temporary solution. In v1beta2, the FailureReason and FailureError fields should no longer be used; conditions should be used instead. We can probably remove the SetFailure calls completely, but this would constitute a breaking change and should be implemented when cluster-api-provider-hetzner switches to v1beta2.

See proposal: https://github.com/kubernetes-sigs/cluster-api/blob/main/docs/proposals/20240916-improve-status-in-CAPI-resources.md

@guettli
Copy link
Collaborator

guettli commented Aug 2, 2025

@cgroschupp thank you. I pulled your changes to my branch to trigger CI.

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.

3 participants