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 webconsole controller #1

Merged
merged 1 commit into from
Oct 1, 2020
Merged

refactor webconsole controller #1

merged 1 commit into from
Oct 1, 2020

Conversation

eguzki
Copy link

@eguzki eguzki commented Oct 1, 2020

I propose some refactor to follow better reconciliation patterns. Some relevant changes are:

  • pkg/controller/webconsole/webconsole_controller.go includes reconciliation logic which leverages inherited BaseReconciler
  • Checking consolelink support in the cluster is done in the same way as other CRD's are checked (for instance PodMonitors)
  • It aims to be extensible. When we implement consolelink for existing 3scale tenants, they should be easy to be added. They should be added in the main Reconcile method.
  • Reconciliation logic is reused and does not need to be re-implemented (DRY).
  • Reconciliation is based on the route object triggering reconciliation loop. List of routes is not fetched for every reconciliation loop (which happens for every single route object in the namespace). This change in the followed approach can be discussed. There are pros and cons, but I find it more appropriate for this usecase.
  • helper/webconsole only contributes with MasterConsole object definition and mutator. A mutator is what is called when actual reconciliation occurs. When consolelink exists, this is compared with the desired consolelink object and the mutator tells if the existing object needs changes and which changes.

let me know what you think

cc @miguelsorianod

@myeung18
Copy link
Owner

myeung18 commented Oct 1, 2020

I propose some refactor to follow better reconciliation patterns. Some relevant changes are:

  • pkg/controller/webconsole/webconsole_controller.go includes reconciliation logic which leverages inherited BaseReconciler
  • Checking consolelink support in the cluster is done in the same way as other CRD's are checked (for instance PodMonitors)
  • It aims to be extensible. When we implement consolelink for existing 3scale tenants, they should be easy to be added. They should be added in the main Reconcile method.
  • Reconciliation logic is reused and does not need to be re-implemented (DRY).
  • Reconciliation is based on the route object triggering reconciliation loop. List of routes is not fetched for every reconciliation loop (which happens for every single route object in the namespace). This change in the followed approach can be discussed. There are pros and cons, but I find it more appropriate for this usecase.
  • helper/webconsole only contributes with MasterConsole object definition and mutator. A mutator is what is called when actual reconciliation occurs. When consolelink exists, this is compared with the desired consolelink object and the mutator tells if the existing object needs changes and which changes.

let me know what you think

cc @miguelsorianod

Thank you for showing me the correct way of doing it.

@myeung18 myeung18 merged commit 86d393c into myeung18:consolelink Oct 1, 2020
myeung18 added a commit that referenced this pull request Oct 1, 2020
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 this pull request may close these issues.

2 participants