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

Clear documentation for machine and cluster actuator interface #505

Closed
anfernee opened this issue Sep 19, 2018 · 6 comments · Fixed by #566
Closed

Clear documentation for machine and cluster actuator interface #505

anfernee opened this issue Sep 19, 2018 · 6 comments · Fixed by #566
Assignees

Comments

@anfernee
Copy link
Member

The machine actuator interface is not defined clearly here: https://github.com/kubernetes-sigs/cluster-api/blob/master/pkg/controller/machine/actuator.go#L25 It's confusing to the implementer of the interface about the expected behavior. The same applies to cluster actuator.

For instance:

Also, is it a good idea to put the idempotent burden to actuator implementer? especially for Create. Create does the whole lot of thing, and doing it idempotent isn't a trivial job. If the VM doesn't exist, it's fine. However, if the target VM exists, what should we do? Those logics are very common to all different actuators. Should we handle this in in controller? Actuator should just become a stateless cloud provider abstraction.

@roberthbailey
Copy link
Contributor

/cc @hardikdr

@dlipovetsky
Copy link
Contributor

These are great questions. Since what the methods do differs among providers (e.g. I'm implementing a provider that targets pre-existing machines, so Create only deploys software), I think the provider office hours could be a good place to discuss (any proposed changes would be presented at the working group meeting).

Here's a gdoc for jotting down thoughts: https://docs.google.com/document/d/1PTAWB8jC1PTx6K2-emvi7Mmo7ZVtLIE11rNEJJm2TJw/edit#

@dlipovetsky
Copy link
Contributor

Here's a relevant discussion about the Cluster actuator interface: #190 (comment)

@scruplelesswizard
Copy link

/assign chaosaffe

@k8s-ci-robot
Copy link
Contributor

@chaosaffe: GitHub didn't allow me to assign the following users: chaosaffe.

Note that only kubernetes-sigs members and repo collaborators can be assigned.
For more information please see the contributor guide

In response to this:

/assign chaosaffe

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.

@scruplelesswizard
Copy link

/assign chaosaffe

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 a pull request may close this issue.

5 participants