Skip to content

Commit

Permalink
Propagate errors of GenevaLogging controller (Azure#3221)
Browse files Browse the repository at this point in the history 
* genevalogging: Use AROController as base type
* genevalogging: Split off business logic for uniform error handling
* genevalogging: Add condition for controller status
* genevalogging: Check status conditions in unit tests

---------

Co-authored-by: Matthew Barnes <[email protected]>
  • Loading branch information
@mbarnes @ventifus
2 people authored and ventifus committed Feb 7, 2024
1 parent c34ec2e commit 561e3e2
Show file tree
Hide file tree
Showing 3 changed files with 125 additions and 33 deletions.
2 changes: 1 addition & 1 deletion pkg/operator/controllers/genevalogging/genevalogging.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (

func (r *Reconciler) securityContextConstraints(ctx context.Context, name, serviceAccountName string) (*securityv1.SecurityContextConstraints, error) {
scc := &securityv1.SecurityContextConstraints{}
err := r.client.Get(ctx, types.NamespacedName{Name: "privileged"}, scc)
err := r.Client.Get(ctx, types.NamespacedName{Name: "privileged"}, scc)
if err != nil {
return nil, err
}
Expand Down
72 changes: 43 additions & 29 deletions pkg/operator/controllers/genevalogging/genevalogging_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (

"github.com/Azure/ARO-RP/pkg/operator"
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 @@ -34,65 +35,78 @@ const (

// Reconciler reconciles a Cluster object
type Reconciler struct {
log *logrus.Entry
base.AROController

dh dynamichelper.Interface
client client.Client
dh dynamichelper.Interface
}

func NewReconciler(log *logrus.Entry, client client.Client, dh dynamichelper.Interface) *Reconciler {
return &Reconciler{
log: log,

dh: dh,
client: client,
AROController: base.AROController{
Log: log,
Client: client,
Name: ControllerName,
},
dh: dh,
}
}

// Reconcile the genevalogging deployment.
func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.Result, error) {
instance := &arov1alpha1.Cluster{}
err := r.client.Get(ctx, types.NamespacedName{Name: arov1alpha1.SingletonClusterName}, instance)
func (r *Reconciler) ensureResources(ctx context.Context, instance *arov1alpha1.Cluster) error {
operatorSecret := &corev1.Secret{}
operatorSecretName := types.NamespacedName{
Namespace: operator.Namespace,
Name: operator.SecretName,
}
err := r.Client.Get(ctx, operatorSecretName, operatorSecret)
if err != nil {
return reconcile.Result{}, err
return err
}

if !instance.Spec.OperatorFlags.GetSimpleBoolean(controllerEnabled) {
r.log.Debug("controller is disabled")
return reconcile.Result{}, nil
resources, err := r.resources(ctx, instance, operatorSecret.Data[GenevaCertName], operatorSecret.Data[GenevaKeyName])
if err != nil {
return err
}

r.log.Debug("running")
operatorSecret := &corev1.Secret{}
err = r.client.Get(ctx, types.NamespacedName{Namespace: operator.Namespace, Name: operator.SecretName}, operatorSecret)
err = dynamichelper.SetControllerReferences(resources, instance)
if err != nil {
return reconcile.Result{}, err
return err
}

resources, err := r.resources(ctx, instance, operatorSecret.Data[GenevaCertName], operatorSecret.Data[GenevaKeyName])
err = dynamichelper.Prepare(resources)
if err != nil {
r.log.Error(err)
return reconcile.Result{}, err
return err
}

err = dynamichelper.SetControllerReferences(resources, instance)
err = r.dh.Ensure(ctx, resources...)
if err != nil {
r.log.Error(err)
return reconcile.Result{}, err
return err
}

err = dynamichelper.Prepare(resources)
return nil
}

// 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)
r.Log.Error(err)
return reconcile.Result{}, err
}

err = r.dh.Ensure(ctx, resources...)
if !instance.Spec.OperatorFlags.GetSimpleBoolean(controllerEnabled) {
r.Log.Debug("controller is disabled")
return reconcile.Result{}, nil
}

r.Log.Debug("running")
err = r.ensureResources(ctx, instance)
if err != nil {
r.log.Error(err)
r.Log.Error(err)
r.SetDegraded(ctx, err)
return reconcile.Result{}, err
}

r.ClearConditions(ctx)
return reconcile.Result{}, nil
}

Expand Down
84 changes: 81 additions & 3 deletions pkg/operator/controllers/genevalogging/genevalogging_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,32 @@ package genevalogging
// Licensed under the Apache License 2.0.

import (
"context"
"errors"
"fmt"
"testing"

"github.com/go-test/deep"
"github.com/golang/mock/gomock"
operatorv1 "github.com/openshift/api/operator/v1"
securityv1 "github.com/openshift/api/security/v1"
"github.com/sirupsen/logrus"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
ctrlfake "sigs.k8s.io/controller-runtime/pkg/client/fake"

"github.com/Azure/ARO-RP/pkg/operator"
arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
"github.com/Azure/ARO-RP/pkg/operator/controllers/base"
mock_dynamichelper "github.com/Azure/ARO-RP/pkg/util/mocks/dynamichelper"
_ "github.com/Azure/ARO-RP/pkg/util/scheme"
"github.com/Azure/ARO-RP/pkg/util/version"
testdatabase "github.com/Azure/ARO-RP/test/database"
utilconditions "github.com/Azure/ARO-RP/test/util/conditions"
utilerror "github.com/Azure/ARO-RP/test/util/error"
)

func getContainer(d *appsv1.DaemonSet, containerName string) (corev1.Container, error) {
Expand All @@ -33,11 +42,32 @@ func getContainer(d *appsv1.DaemonSet, containerName string) (corev1.Container,
}

func TestGenevaLoggingDaemonset(t *testing.T) {
nominalMocks := func(mockDh *mock_dynamichelper.MockInterface) {
mockDh.EXPECT().Ensure(
gomock.Any(),
gomock.AssignableToTypeOf(&securityv1.SecurityContextConstraints{}),
gomock.AssignableToTypeOf(&corev1.Namespace{}),
gomock.AssignableToTypeOf(&corev1.ConfigMap{}),
gomock.AssignableToTypeOf(&corev1.Secret{}),
gomock.AssignableToTypeOf(&corev1.ServiceAccount{}),
gomock.AssignableToTypeOf(&appsv1.DaemonSet{}),
).Times(1)
}

defaultConditions := []operatorv1.OperatorCondition{
utilconditions.ControllerDefaultAvailable(ControllerName),
utilconditions.ControllerDefaultProgressing(ControllerName),
utilconditions.ControllerDefaultDegraded(ControllerName),
}

tests := []struct {
name string
request ctrl.Request
operatorFlags arov1alpha1.OperatorFlags
validateDaemonset func(*appsv1.DaemonSet) []error
mocks func(mockDh *mock_dynamichelper.MockInterface)
wantErrMsg string
wantConditions []operatorv1.OperatorCondition
}{
{
name: "no flags given",
Expand Down Expand Up @@ -71,6 +101,9 @@ func TestGenevaLoggingDaemonset(t *testing.T) {

return
},
mocks: nominalMocks,
wantErrMsg: "",
wantConditions: defaultConditions,
},
{
name: "fluentbit changed",
Expand Down Expand Up @@ -105,6 +138,9 @@ func TestGenevaLoggingDaemonset(t *testing.T) {

return
},
mocks: nominalMocks,
wantErrMsg: "",
wantConditions: defaultConditions,
},
{
name: "mdsd changed",
Expand Down Expand Up @@ -139,23 +175,58 @@ func TestGenevaLoggingDaemonset(t *testing.T) {

return
},
mocks: nominalMocks,
wantErrMsg: "",
wantConditions: []operatorv1.OperatorCondition{
utilconditions.ControllerDefaultAvailable(ControllerName),
utilconditions.ControllerDefaultProgressing(ControllerName),
utilconditions.ControllerDefaultDegraded(ControllerName),
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
controller := gomock.NewController(t)
defer controller.Finish()

instance := &arov1alpha1.Cluster{
ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
Status: arov1alpha1.ClusterStatus{Conditions: []operatorv1.OperatorCondition{}},
Status: arov1alpha1.ClusterStatus{Conditions: defaultConditions},
Spec: arov1alpha1.ClusterSpec{
ResourceID: testdatabase.GetResourcePath("00000000-0000-0000-0000-000000000000", "testcluster"),
OperatorFlags: tt.operatorFlags,
ACRDomain: "acrDomain",
},
}

resources := []client.Object{
instance,
&corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Namespace: operator.Namespace,
Name: operator.SecretName,
},
Data: map[string][]byte{
GenevaCertName: {},
GenevaKeyName: {},
},
},
&securityv1.SecurityContextConstraints{
ObjectMeta: metav1.ObjectMeta{
Name: "privileged",
},
},
}

mockDh := mock_dynamichelper.NewMockInterface(controller)

r := &Reconciler{
log: logrus.NewEntry(logrus.StandardLogger()),
client: ctrlfake.NewClientBuilder().WithObjects(instance).Build(),
AROController: base.AROController{
Log: logrus.NewEntry(logrus.StandardLogger()),
Client: ctrlfake.NewClientBuilder().WithObjects(resources...).Build(),
Name: ControllerName,
},
dh: mockDh,
}

daemonset, err := r.daemonset(instance)
Expand All @@ -167,6 +238,13 @@ func TestGenevaLoggingDaemonset(t *testing.T) {
for _, err := range errs {
t.Error(err)
}

tt.mocks(mockDh)
ctx := context.Background()
_, err = r.Reconcile(ctx, tt.request)

utilerror.AssertErrorMessage(t, err, tt.wantErrMsg)
utilconditions.AssertControllerConditions(t, ctx, r.Client, tt.wantConditions)
})
}
}

0 comments on commit 561e3e2

Please sign in to comment.