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

Refactor actuators around a context #305

Closed
vincepri opened this issue Oct 23, 2018 · 5 comments
Closed

Refactor actuators around a context #305

vincepri opened this issue Oct 23, 2018 · 5 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/active Indicates that an issue or PR is actively being worked on by a contributor. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Milestone

Comments

@vincepri
Copy link
Member

/kind feature

Describe the solution you'd like
In the current state of things, the machine and cluster actuator are structured in a way which effectively operates around either a cluster, a machine, or both. Longer term, we should think about refactoring the actuators so they instantiate throw-away objects from a context. During this refactor, a lot of business logic currently present under services/ec2 could potentially move up one level, keeping a clear separation between the ec2 layer and the actuator operations.

CC @detiber @ashish-amarnath @chuckha @randomvariable

@k8s-ci-robot k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Oct 23, 2018
@randomvariable
Copy link
Member

Do you mean literally a context.Context, or something more typed? Worry with the former is losing some compile time checks. On the other hand, for decent structured logging, particularly when we consider multi-cluster, many-node, it'd be good to pass a context at least for logging.

I generally agree with separating the actual service calls and actuator operations in a refactor now that we've got to a working state.

@vincepri
Copy link
Member Author

I was suggesting of something more typed, I don't think "context.Context" is useful in this context 😛

This was referenced Oct 29, 2018
@detiber detiber added this to the 20181122 milestone Oct 29, 2018
@detiber
Copy link
Member

detiber commented Oct 29, 2018

/priority important-soon

@k8s-ci-robot k8s-ci-robot added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Oct 29, 2018
This was referenced Nov 14, 2018
@chuckha chuckha added the lifecycle/active Indicates that an issue or PR is actively being worked on by a contributor. label Nov 15, 2018
@vincepri
Copy link
Member Author

/close

@k8s-ci-robot
Copy link
Contributor

@vincepri: Closing this issue.

In response to this:

/close

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.

enxebre pushed a commit to enxebre/cluster-api-provider-aws-2 that referenced this issue Apr 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/active Indicates that an issue or PR is actively being worked on by a contributor. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
None yet
Development

No branches or pull requests

5 participants