Skip to content
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

Support for Hetzner arm64 architecture instances #1816

Merged
merged 4 commits into from
Jun 27, 2024

Conversation

7oku
Copy link
Contributor

@7oku 7oku commented Jun 19, 2024

What this PR does / why we need it:
Hetzner provides CAX instances with arm64 architecture. An appropriate image has to be given to the hcloud module, which is suitable for arm64 instances.

The PR implements additional mechanisms to determine the new hcloud.ServerType.Architecture field and use the hcloud modules new method Image.GetForArchitecture() to determine the correct image for arm64 ServerTypes and allows machine-controller to successful deploy CAX instances.

Which issue(s) this PR fixes:
Fixes #

What type of PR is this?
/kind feature

Special notes for your reviewer:
I had to test the change with the latest release of MC (v1.59.2) as my test environment uses kubeone 1.8. The latest changes to MC break the kubernetes deployment, as a lot of flags (use-osm etc.) have been removed in main recently. Since I did not touch anything else other than the hetzner provider and make test-unit succeeded, it should work fine in upcoming releases. But feel free to test prior accepting the PR.

Does this PR introduce a user-facing change? Then add your Release Note here:

Support for Hetzner arm64 architecture instances

Documentation:

NONE

@kubermatic-bot kubermatic-bot added docs/none Denotes a PR that doesn't need documentation (changes). kind/feature Categorizes issue or PR as related to a new feature. release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: yes Denotes that all commits in the pull request have the valid DCO signoff message. sig/cluster-management Denotes a PR or issue as being assigned to SIG Cluster Management. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 19, 2024
@kubermatic-bot
Copy link
Contributor

Hi @7oku. Thanks for your PR.

I'm waiting for a kubermatic member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@kubermatic-bot kubermatic-bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jun 19, 2024
@embik
Copy link
Member

embik commented Jun 20, 2024

Looking good, thank you for the contribution! Let's see if it passes e2e tests.

/ok-to-test

@kubermatic-bot kubermatic-bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jun 20, 2024
@kubermatic-bot kubermatic-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 26, 2024
@kubermatic-bot kubermatic-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 26, 2024
@7oku
Copy link
Contributor Author

7oku commented Jun 26, 2024

@embik #1818 was prioritized in the meantime and changed base for my pr fundamentally. had to do some rework. retesting was successful, so it's ready to be merged again from my side now.

@P4sca1
Copy link
Contributor

P4sca1 commented Jun 26, 2024

Sorry for causing the merge conflicts. Your change looks good to me and is a useful addition!

@embik
Copy link
Member

embik commented Jun 26, 2024

Hey, thanks for updating the PR after the merge conflict. I'm currently checking if we have any concerns with the upgrade to the hcloud-go/v2 version, I'll make sure to get back to you ASAP!

@7oku
Copy link
Contributor Author

7oku commented Jun 26, 2024

Hey @embik no problem, we could potentially roll back to hcloud-go/v1 (1.56.0 or so), but according to hetznercloud/hcloud-go#263 (comment) v1 is deprecated since Sep 2023. Upgrading only took the effort to convert int to int64, so not a big deal.

Other than here in the Hetzner provider I did not find any other occurrence of hcloud module in machine-controller.

@P4sca1 all good, I'm happy to see development on the Hetzner provider.

Copy link
Member

@embik embik left a comment

Choose a reason for hiding this comment

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

/approve

Thank you for the contribution, it is greatly appreciated! 🎉

@kubermatic-bot kubermatic-bot added the lgtm Indicates that a PR is ready to be merged. label Jun 27, 2024
@kubermatic-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: embik

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubermatic-bot
Copy link
Contributor

LGTM label has been added.

Git tree hash: fe37d410e7e69454377d3eb04baf840cb31094ff

@kubermatic-bot kubermatic-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 27, 2024
@kubermatic-bot kubermatic-bot merged commit 8f3166a into kubermatic:main Jun 27, 2024
14 checks passed
@7oku 7oku deleted the feat/hetzner-arm branch June 28, 2024 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Denotes that all commits in the pull request have the valid DCO signoff message. docs/none Denotes a PR that doesn't need documentation (changes). kind/feature Categorizes issue or PR as related to a new feature. lgtm Indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/cluster-management Denotes a PR or issue as being assigned to SIG Cluster Management. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants