Skip to content

Commit

Permalink
operator: Add reconcile methods to AROReconciler interface
Browse files Browse the repository at this point in the history 
This introduces two new methods in AROReconciler:

  ReconcileEnabled(ctx, request, cluster) -> (result, error)
  ReconcileDisabled(ctx, request, cluster) -> (result, error)

One or the other method will be called based on the controller's
"enabled" operator flag.

AROController itself now implements the Reconcile method in order
to consistently handle disabled controllers.  Subtypes need only
implement the ReconcileEnabled method, called when the controller
is enabled.

The Reconcile method uses the "template method" design pattern
from object-oriented programming.  This warrants some explanation
since the pattern looks a little odd in Golang.

Some background reading:
https://golangbyexample.com/template-method-design-pattern-golang/

AROController provides ReconcileDisabled but leaves ReconcileEnabled
unimplemented, so ReconcileEnabled is effectively an abstract method.
Therefore AROController itself is not an AROReconciler according to
Golang's interface semantics, but its subtypes are.

Because AROController's Reconcile method calls ReconcileEnabled,
which is abstract, it needs a way to cast itself to an AROReconciler.

This is the purpose of the Reconciler field in AROController.  The
field is set during subtype instantiation:

   r := ExampleReconciler{
       AROController: base.AROController {
           Log:         ...
           Client:      ...
           Name:        ...
           EnabledFlag: ...
       }
   }
   r.Reconciler = r
   return r

The Reconciler field is essentially a virtual method table like
those inherent in polymorphic languages like C++ and Python.  It
allows AROController to call abstract AROReconciler methods that
are implemented by a subtype.
  • Loading branch information
Matthew Barnes committed Nov 9, 2023
1 parent f1b40b0 commit 499680a
Show file tree
Hide file tree
Showing 12 changed files with 84 additions and 141 deletions.
26 changes: 26 additions & 0 deletions pkg/operator/controllers/base/aro_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,21 @@ import (
"k8s.io/apimachinery/pkg/types"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
)

type AROReconciler interface {
reconcile.Reconciler
GetName() string
SetupWithManager(ctrl.Manager) error
ReconcileEnabled(context.Context, ctrl.Request, *arov1alpha1.Cluster) (ctrl.Result, error)
ReconcileDisabled(context.Context, ctrl.Request, *arov1alpha1.Cluster) (ctrl.Result, error)
}

type AROController struct {
Reconciler AROReconciler // virtual method table
Log *logrus.Entry
Client client.Client
Name string
Expand All @@ -33,6 +38,27 @@ func (c *AROController) GetName() string {
return c.Name
}

func (c *AROController) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.Result, error) {
cluster, err := c.GetCluster(ctx)
if err != nil {
return reconcile.Result{}, err
}

// Controller can be disabled if it defines an "enabled" operator flag.
if c.EnabledFlag != "" && !cluster.Spec.OperatorFlags.GetSimpleBoolean(c.EnabledFlag) {
c.Log.Debug("controller is disabled")
return c.Reconciler.ReconcileDisabled(ctx, request, cluster)
}

c.Log.Debug("running")
return c.Reconciler.ReconcileEnabled(ctx, request, cluster)
}

func (c *AROController) ReconcileDisabled(ctx context.Context, request ctrl.Request, cluster *arov1alpha1.Cluster) (ctrl.Result, error) {
c.ClearConditions(ctx)
return reconcile.Result{}, nil
}

func (c *AROController) SetConditions(ctx context.Context, cnds ...*operatorv1.OperatorCondition) {
cluster, err := c.GetCluster(ctx)
if err != nil {
Expand Down
17 changes: 4 additions & 13 deletions pkg/operator/controllers/cloudproviderconfig/cloudproviderconfig_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,31 +133,22 @@ type CloudProviderConfigReconciler struct {
}

func NewReconciler(log *logrus.Entry, client client.Client) *CloudProviderConfigReconciler {
return &CloudProviderConfigReconciler{
r := &CloudProviderConfigReconciler{
AROController: base.AROController{
Log: log.WithField("controller", controllerName),
Client: client,
Name: controllerName,
EnabledFlag: controllerEnabled,
},
}
r.Reconciler = r
return r
}

// Reconcile makes sure that the cloud-provider-config is healthy
func (r *CloudProviderConfigReconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.Result, error) {
func (r *CloudProviderConfigReconciler) ReconcileEnabled(ctx context.Context, request ctrl.Request, instance *arov1alpha1.Cluster) (ctrl.Result, error) {
r.Log.Debug("reconcile ConfigMap openshift-config/cloud-provider-config")

instance, err := r.GetCluster(ctx)
if err != nil {
return reconcile.Result{}, err
}

if !instance.Spec.OperatorFlags.GetSimpleBoolean(r.EnabledFlag) {
r.Log.Debug("controller is disabled")
return reconcile.Result{}, nil
}

r.Log.Debug("running")
return reconcile.Result{}, r.updateCloudProviderConfig(ctx)
}

Expand Down
17 changes: 5 additions & 12 deletions pkg/operator/controllers/dnsmasq/cluster_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ type ClusterReconciler struct {
}

func NewClusterReconciler(log *logrus.Entry, client client.Client, dh dynamichelper.Interface) *ClusterReconciler {
return &ClusterReconciler{
r := &ClusterReconciler{
AROController: base.AROController{
Log: log.WithField("controller", clusterControllerName),
Client: client,
Expand All @@ -42,27 +42,20 @@ func NewClusterReconciler(log *logrus.Entry, client client.Client, dh dynamichel
},
dh: dh,
}
r.Reconciler = r
return r
}

// Reconcile watches the ARO object, and if it changes, reconciles all the
// 99-%s-aro-dns machineconfigs
func (r *ClusterReconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.Result, error) {
instance, err := r.GetCluster(ctx)
if err != nil {
return reconcile.Result{}, err
}

if !instance.Spec.OperatorFlags.GetSimpleBoolean(r.EnabledFlag) {
r.Log.Debug("controller is disabled")
return reconcile.Result{}, nil
}
func (r *ClusterReconciler) ReconcileEnabled(ctx context.Context, request ctrl.Request, instance *arov1alpha1.Cluster) (ctrl.Result, error) {
var err error

restartDnsmasq := instance.Spec.OperatorFlags.GetSimpleBoolean(restartDnsmasqEnabled)
if restartDnsmasq {
r.Log.Debug("restartDnsmasq is enabled")
}

r.Log.Debug("running")
mcps := &mcv1.MachineConfigPoolList{}
err = r.Client.List(ctx, mcps)
if err != nil {
Expand Down
18 changes: 6 additions & 12 deletions pkg/operator/controllers/dnsmasq/machineconfig_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
"github.com/Azure/ARO-RP/pkg/operator/controllers/base"
"github.com/Azure/ARO-RP/pkg/util/dynamichelper"
)
Expand All @@ -32,7 +33,7 @@ type MachineConfigReconciler struct {
var rxARODNS = regexp.MustCompile("^99-(.*)-aro-dns$")

func NewMachineConfigReconciler(log *logrus.Entry, client client.Client, dh dynamichelper.Interface) *MachineConfigReconciler {
return &MachineConfigReconciler{
r := &MachineConfigReconciler{
AROController: base.AROController{
Log: log.WithField("controller", machineConfigControllerName),
Client: client,
Expand All @@ -41,26 +42,19 @@ func NewMachineConfigReconciler(log *logrus.Entry, client client.Client, dh dyna
},
dh: dh,
}
r.Reconciler = r
return r
}

// Reconcile watches ARO DNS MachineConfig objects, and if any changes,
// reconciles it
func (r *MachineConfigReconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.Result, error) {
instance, err := r.GetCluster(ctx)
if err != nil {
return reconcile.Result{}, err
}

if !instance.Spec.OperatorFlags.GetSimpleBoolean(r.EnabledFlag) {
r.Log.Debug("controller is disabled")
return reconcile.Result{}, nil
}
func (r *MachineConfigReconciler) ReconcileEnabled(ctx context.Context, request ctrl.Request, instance *arov1alpha1.Cluster) (ctrl.Result, error) {
var err error

if instance.Spec.OperatorFlags.GetSimpleBoolean(restartDnsmasqEnabled) {
r.Log.Debug("restart dnsmasq machineconfig enabled")
}

r.Log.Debug("running")
m := rxARODNS.FindStringSubmatch(request.Name)
if m == nil {
return reconcile.Result{}, nil
Expand Down
18 changes: 6 additions & 12 deletions pkg/operator/controllers/dnsmasq/machineconfigpool_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
"github.com/Azure/ARO-RP/pkg/operator/controllers/base"
"github.com/Azure/ARO-RP/pkg/util/dynamichelper"
)
Expand All @@ -29,7 +30,7 @@ type MachineConfigPoolReconciler struct {
}

func NewMachineConfigPoolReconciler(log *logrus.Entry, client client.Client, dh dynamichelper.Interface) *MachineConfigPoolReconciler {
return &MachineConfigPoolReconciler{
r := &MachineConfigPoolReconciler{
AROController: base.AROController{
Log: log.WithField("controller", machineConfigPoolControllerName),
Client: client,
Expand All @@ -38,27 +39,20 @@ func NewMachineConfigPoolReconciler(log *logrus.Entry, client client.Client, dh
},
dh: dh,
}
r.Reconciler = r
return r
}

// Reconcile watches MachineConfigPool objects, and if any changes,
// reconciles the associated ARO DNS MachineConfig object
func (r *MachineConfigPoolReconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.Result, error) {
instance, err := r.GetCluster(ctx)
if err != nil {
return reconcile.Result{}, err
}

if !instance.Spec.OperatorFlags.GetSimpleBoolean(r.EnabledFlag) {
r.Log.Debug("controller is disabled")
return reconcile.Result{}, nil
}
func (r *MachineConfigPoolReconciler) ReconcileEnabled(ctx context.Context, request ctrl.Request, instance *arov1alpha1.Cluster) (ctrl.Result, error) {
var err error

restartDnsmasq := instance.Spec.OperatorFlags.GetSimpleBoolean(restartDnsmasqEnabled)
if restartDnsmasq {
r.Log.Debug("restart dnsmasq machineconfig enabled")
}

r.Log.Debug("running")
mcp := &mcv1.MachineConfigPool{}
err = r.Client.Get(ctx, types.NamespacedName{Name: request.Name}, mcp)
if kerrors.IsNotFound(err) {
Expand Down
20 changes: 5 additions & 15 deletions pkg/operator/controllers/genevalogging/genevalogging_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ type Reconciler struct {
}

func NewReconciler(log *logrus.Entry, client client.Client, dh dynamichelper.Interface) *Reconciler {
return &Reconciler{
r := &Reconciler{
AROController: base.AROController{
Log: log.WithField("controller", controllerName),
Client: client,
Expand All @@ -49,6 +49,8 @@ func NewReconciler(log *logrus.Entry, client client.Client, dh dynamichelper.Int
},
dh: dh,
}
r.Reconciler = r
return r
}

func (r *Reconciler) ensureResources(ctx context.Context, instance *arov1alpha1.Cluster) error {
Expand Down Expand Up @@ -86,20 +88,8 @@ func (r *Reconciler) ensureResources(ctx context.Context, instance *arov1alpha1.
}

// Reconcile the genevalogging deployment.
func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.Result, error) {
instance, err := r.GetCluster(ctx)
if err != nil {
r.Log.Error(err)
return reconcile.Result{}, err
}

if !instance.Spec.OperatorFlags.GetSimpleBoolean(r.EnabledFlag) {
r.Log.Debug("controller is disabled")
return reconcile.Result{}, nil
}

r.Log.Debug("running")
err = r.ensureResources(ctx, instance)
func (r *Reconciler) ReconcileEnabled(ctx context.Context, request ctrl.Request, instance *arov1alpha1.Cluster) (ctrl.Result, error) {
err := r.ensureResources(ctx, instance)
if err != nil {
r.Log.Error(err)
r.SetDegraded(ctx, err)
Expand Down
17 changes: 5 additions & 12 deletions pkg/operator/controllers/imageconfig/image_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,32 +37,25 @@ type Reconciler struct {
}

func NewReconciler(log *logrus.Entry, client client.Client) *Reconciler {
return &Reconciler{
r := &Reconciler{
AROController: base.AROController{
Log: log.WithField("controller", controllerName),
Client: client,
Name: controllerName,
EnabledFlag: controllerEnabled,
},
}
r.Reconciler = r
return r
}

// watches the ARO object for changes and reconciles image.config.openshift.io/cluster object.
// - If blockedRegistries is not nil, makes sure required registries are not added
// - If AllowedRegistries is not nil, makes sure required registries are added
// - Fails fast if both are not nil, unsupported
func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.Result, error) {
instance, err := r.GetCluster(ctx)
if err != nil {
return reconcile.Result{}, err
}

if !instance.Spec.OperatorFlags.GetSimpleBoolean(r.EnabledFlag) {
r.Log.Debug("controller is disabled")
return reconcile.Result{}, nil
}
func (r *Reconciler) ReconcileEnabled(ctx context.Context, request ctrl.Request, instance *arov1alpha1.Cluster) (ctrl.Result, error) {
var err error

r.Log.Debug("running")
requiredRegistries, err := GetCloudAwareRegistries(instance)
if err != nil {
// Not returning error as it will requeue again
Expand Down
17 changes: 5 additions & 12 deletions pkg/operator/controllers/ingress/ingress_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,28 +35,21 @@ type Reconciler struct {
}

func NewReconciler(log *logrus.Entry, client client.Client) *Reconciler {
return &Reconciler{
r := &Reconciler{
AROController: base.AROController{
Log: log.WithField("controller", controllerName),
Client: client,
Name: controllerName,
EnabledFlag: controllerEnabled,
},
}
r.Reconciler = r
return r
}

func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.Result, error) {
instance, err := r.GetCluster(ctx)
if err != nil {
return reconcile.Result{}, err
}

if !instance.Spec.OperatorFlags.GetSimpleBoolean(r.EnabledFlag) {
r.Log.Debug("controller is disabled")
return reconcile.Result{}, nil
}
func (r *Reconciler) ReconcileEnabled(ctx context.Context, request ctrl.Request, instance *arov1alpha1.Cluster) (ctrl.Result, error) {
var err error

r.Log.Debug("running")
ingress := &operatorv1.IngressController{}
err = r.Client.Get(ctx, types.NamespacedName{Namespace: openshiftIngressControllerNamespace, Name: openshiftIngressControllerName}, ingress)
if err != nil {
Expand Down
20 changes: 6 additions & 14 deletions pkg/operator/controllers/machinehealthcheck/machinehealthcheck_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ type Reconciler struct {
}

func NewReconciler(log *logrus.Entry, client client.Client, dh dynamichelper.Interface) *Reconciler {
return &Reconciler{
r := &Reconciler{
AROController: base.AROController{
Log: log.WithField("controller", controllerName),
Client: client,
Expand All @@ -58,25 +58,17 @@ func NewReconciler(log *logrus.Entry, client client.Client, dh dynamichelper.Int
},
dh: dh,
}
r.Reconciler = r
return r
}

// Reconcile watches MachineHealthCheck objects, and if any changes,
// reconciles the associated ARO MachineHealthCheck object
func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.Result, error) {
instance, err := r.GetCluster(ctx)
func (r *Reconciler) ReconcileEnabled(ctx context.Context, request ctrl.Request, instance *arov1alpha1.Cluster) (ctrl.Result, error) {
var err error

if err != nil {
return reconcile.Result{}, err
}

if !instance.Spec.OperatorFlags.GetSimpleBoolean(r.EnabledFlag) {
r.Log.Debug("controller is disabled")
return reconcile.Result{}, nil
}

r.Log.Debug("running")
if !instance.Spec.OperatorFlags.GetSimpleBoolean(controllerManaged) {
err := r.dh.EnsureDeleted(ctx, "MachineHealthCheck", "openshift-machine-api", "aro-machinehealthcheck")
err = r.dh.EnsureDeleted(ctx, "MachineHealthCheck", "openshift-machine-api", "aro-machinehealthcheck")
if err != nil {
r.Log.Error(err)
r.SetDegraded(ctx, err)
Expand Down
Loading

0 comments on commit 499680a

Please sign in to comment.