Skip to content
This repository has been archived by the owner on Apr 7, 2020. It is now read-only.

Refactor controlplane controllers to eliminate code duplication #76

Conversation

stoyanr
Copy link
Contributor

@stoyanr stoyanr commented May 7, 2019

What this PR does / why we need it:
Refactors the AWS and GCP controlplane actuators into a generic actuator and provider-specific "value providers" in order to eliminate code duplication.

Special notes for your reviewer:

  • The chosen approach has been agreed with @rfranzke. It allows for significantly reducing the code duplication between the current controlplane controllers, while at the same time keeping the flexibility to provide a very different implementation in the future if needed.
  • The generic actuator is just one possible implementation of the Actuator interface. Providers might choose to use the generic actuator and plug into it a ValuesProvider implementation, or they might choose to provide their own Actuator if they need to do something different from the flow that the generic actuator implements.
  • Charts and image vectors are still provider-specific, they are very similar right now but we expect they might diverge quickly.
  • Tests for the GCP ValuesProvider have been added.
  • Depends on Add GCP controlplane controller #69 which should be merged first.

Release note:

Refactor controlplane controllers to eliminate code duplication

@stoyanr stoyanr requested a review from a team as a code owner May 7, 2019 14:43
@stoyanr stoyanr force-pushed the refactor-controlplane-controllers-for-reuse branch from 6898ca4 to 6b6412e Compare May 7, 2019 14:46
@rfranzke rfranzke added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label May 8, 2019
@gardener-robot-ci-1 gardener-robot-ci-1 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels May 8, 2019
Copy link
Contributor

@rfranzke rfranzke left a comment

Choose a reason for hiding this comment

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

/lgtm

@rfranzke rfranzke merged commit c556a7a into gardener-attic:master May 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants