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

Initial provider implementation #2

Merged
merged 22 commits into from
Oct 21, 2024
Merged

Initial provider implementation #2

merged 22 commits into from
Oct 21, 2024

Conversation

bastjan
Copy link
Contributor

@bastjan bastjan commented Oct 15, 2024

This PR contains the initial implementation of the provider for Cloudscale.

The provider has been observed to work until the Provisioned state on a local non-OCP test cluster, and to the Running state with node linking on our internal OCP lab, without any manual intervention. The steps for this are in the readme under Testing on a.... The CSRs for the new nodes are approved automatically, and a deployment for the upstream node-linker can be created with hack/deploy-nodelink-controller.sh.

The critical code is in pkg/machine/actuator.go.
controllers/machineset_controller.go adds some annotations to the MachineSet to tell autoscaler vCPUs and memory of a new node.

❯ kubectl get machine,node app-zwtl2       
NAME                                     PHASE     TYPE        REGION   ZONE   AGE
machine.machine.openshift.io/app-zwtl2   Running   flex-16-4   rma      rma1   9m1s

NAME             STATUS   ROLES        AGE     VERSION
node/app-zwtl2   Ready    app,worker   4m34s   v1.28.13+2ca1a23

Checklist

  • Categorize the PR by setting a good title and adding one of the labels:
    bug, enhancement, documentation, change, breaking, dependency
    as they show up in the changelog
  • Update tests.

@bastjan bastjan force-pushed the initial-implementation branch from 7b58254 to f7ef913 Compare October 15, 2024 13:46
@bastjan bastjan force-pushed the initial-implementation branch from 46e270b to 266f44e Compare October 16, 2024 15:13
@bastjan bastjan added the enhancement New feature or request label Oct 18, 2024
@bastjan bastjan marked this pull request as ready for review October 18, 2024 16:49
@bastjan bastjan changed the title Initial implementation Initial provider implementation Oct 18, 2024
@bastjan bastjan requested a review from a team October 18, 2024 16:50
Copy link
Member

@simu simu left a comment

Choose a reason for hiding this comment

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

Didn't fully check, but it feels like the flavour/flavor spelling isn't 100% identical across the code base, error messages and comments.

Overall looks good to me, the one thing that needs to be fixed is the flipped CPU/memory extraction from the cloudscale flavor string when annotating the machineset (cf. inline comments).

Note that I didn't review all the tests in detail.

controllers/machineset_controller.go Outdated Show resolved Hide resolved
controllers/machineset_controller_test.go Outdated Show resolved Hide resolved
pkg/machine/actuator_test.go Outdated Show resolved Hide resolved
@bastjan bastjan merged commit 5ef1a6e into master Oct 21, 2024
3 checks passed
@bastjan bastjan deleted the initial-implementation branch October 21, 2024 11:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants