Cluster-api-provider-azure: Move Service interface outside of azure (cloud/ directory) package

Created on 11 Dec 2020  路  8Comments  路  Source: kubernetes-sigs/cluster-api-provider-azure

/kind cleanup

Describe the solution you'd like
We have the service interface which describes the behavior of a reconciliation capz service. There are two kinds of services:

  1. Individual service that reconciles one specific azure component (eg. virtualmachines.Service which only cares about vm reconciliation). These live inside cloud/services directory.
  2. Composite Service (aka reconcilers) that reconciles more than one individual components (eg. azuremachineReconciler which reconciles all services required for an azuremachine). These are mostly inside controllers/

Right now, the Service interface is defined inside cloud/ directory which makes it awkward for services that are outside of cloud/services to implement.

We'd like to move Service interface outside of cloud/ to a common package accessible for all implementors.

Anything else you would like to add:

More discussion on this topic here

Environment:

  • cluster-api-provider-azure version:
  • Kubernetes version: (use kubectl version):
  • OS (e.g. from /etc/os-release):
kincleanup

Most helpful comment

I also noticed we have an OldService interface which is only used in machine pool. Not sure about the history here, but should we also migrate it to implement the new service interface?

Yep that's leftover TODO from https://github.com/kubernetes-sigs/cluster-api-provider-azure/issues/757

As soon as all the services have been refactored we can get rid of OldService

All 8 comments

@shysank
Can i take this up if its unassigned?

Sure @surajrana93 , although I'm not entirely sure about where we want to move Service to? Should we create a new package?

/cc @CecileRobertMichon @devigned

Maybe we just rename azure.Service to azure.Reconciler. Thoughts?

Related to this, does anyone else find that the cloud directory contains the azure package a bit odd? Should it be the azure directory or the cloud package? I prefer the former.

Maybe we just rename azure.Service to azure.Reconciler. Thoughts?

+1 for the rename

Related to this, does anyone else find that the cloud directory contains the azure package a bit odd? Should it be the azure directory or the cloud package? I prefer the former.

Yep, I think this slipped into the v1alpha2 revamp and never got corrected. +1 for renaming the directory to azure

+1 for renaming both interface, and directory. I also noticed we have an OldService interface which is only used in machine pool. Not sure about the history here, but should we also migrate it to implement the new service interface?

I also noticed we have an OldService interface which is only used in machine pool. Not sure about the history here, but should we also migrate it to implement the new service interface?

Yep that's leftover TODO from https://github.com/kubernetes-sigs/cluster-api-provider-azure/issues/757

As soon as all the services have been refactored we can get rid of OldService

/assign

@CecileRobertMichon
Please take a look when you get time https://github.com/kubernetes-sigs/cluster-api-provider-azure/pull/1094

Was this page helpful?
0 / 5 - 0 ratings

Related issues

voor picture voor  路  6Comments

devigned picture devigned  路  4Comments

CecileRobertMichon picture CecileRobertMichon  路  7Comments

mboersma picture mboersma  路  9Comments

devigned picture devigned  路  9Comments