Skip to content

Commit

Permalink
clienthelper test conversions
Browse files Browse the repository at this point in the history
  • Loading branch information
@hawkowl
hawkowl committed Mar 21, 2024
1 parent 3993afc commit 40629c1
Show file tree
Hide file tree
Showing 7 changed files with 192 additions and 78 deletions.
4 changes: 3 additions & 1 deletion pkg/operator/controllers/base/aro_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package base

import (
"context"
"fmt"

configv1 "github.com/openshift/api/config/v1"
operatorv1 "github.com/openshift/api/operator/v1"
Expand Down Expand Up @@ -111,7 +112,8 @@ func (c *AROController) IsClusterUpgrading(ctx context.Context) (bool, error) {
cv := &configv1.ClusterVersion{}
err := c.Client.Get(ctx, types.NamespacedName{Name: "version"}, cv)
if err != nil {
c.Log.Errorf("error getting the ClusterVersion: %v", err)
err = fmt.Errorf("error getting the ClusterVersion: %w", err)
c.Log.Error(err)
return false, err
}

Expand Down
18 changes: 7 additions & 11 deletions pkg/operator/controllers/dnsmasq/cluster_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,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/clienthelper"
"github.com/Azure/ARO-RP/pkg/util/dynamichelper"
)

Expand All @@ -29,17 +30,17 @@ const (

type ClusterReconciler struct {
base.AROController
dh dynamichelper.Interface
ch clienthelper.Interface
}

func NewClusterReconciler(log *logrus.Entry, client client.Client, dh dynamichelper.Interface) *ClusterReconciler {
func NewClusterReconciler(log *logrus.Entry, client client.Client, ch clienthelper.Interface) *ClusterReconciler {
return &ClusterReconciler{
AROController: base.AROController{
Log: log,
Client: client,
Name: ClusterControllerName,
},
dh: dh,
ch: ch,
}
}

Expand Down Expand Up @@ -68,11 +69,6 @@ func (r *ClusterReconciler) Reconcile(ctx context.Context, request ctrl.Request)
return reconcile.Result{}, err
}

if !allowReconcile {
r.Log.Debugf("skipping reconciliation of %s", r.Name)
return reconcile.Result{}, nil
}

r.Log.Debug("running")
mcps := &mcv1.MachineConfigPoolList{}
err = r.Client.List(ctx, mcps)
Expand All @@ -82,7 +78,7 @@ func (r *ClusterReconciler) Reconcile(ctx context.Context, request ctrl.Request)
return reconcile.Result{}, err
}

err = reconcileMachineConfigs(ctx, instance, r.dh, r.Client, allowReconcile, restartDnsmasq, mcps.Items...)
err = reconcileMachineConfigs(ctx, instance, r.ch, r.Client, allowReconcile, restartDnsmasq, mcps.Items...)
if err != nil {
r.Log.Error(err)
r.SetDegraded(ctx, err)
Expand All @@ -105,7 +101,7 @@ func (r *ClusterReconciler) SetupWithManager(mgr ctrl.Manager) error {
Complete(r)
}

func reconcileMachineConfigs(ctx context.Context, instance *arov1alpha1.Cluster, dh dynamichelper.Interface, c client.Client, allowReconcile bool, restartDnsmasq bool, mcps ...mcv1.MachineConfigPool) error {
func reconcileMachineConfigs(ctx context.Context, instance *arov1alpha1.Cluster, ch clienthelper.Interface, c client.Client, allowReconcile bool, restartDnsmasq bool, mcps ...mcv1.MachineConfigPool) error {
var resources []kruntime.Object
for _, mcp := range mcps {
resource, err := dnsmasqMachineConfig(instance.Spec.Domain, instance.Spec.APIIntIP, instance.Spec.IngressIP, mcp.Name, instance.Spec.GatewayDomains, instance.Spec.GatewayPrivateEndpointIP, restartDnsmasq)
Expand All @@ -130,7 +126,7 @@ func reconcileMachineConfigs(ctx context.Context, instance *arov1alpha1.Cluster,
// create or update. If we are not allowed to reconcile, we do not want to
// perform any updates, but we do want to perform initial configuration.
if allowReconcile {
return dh.Ensure(ctx, resources...)
return ch.Ensure(ctx, resources...)
} else {
for _, i := range resources {
err := c.Create(ctx, i.(client.Object))
Expand Down
182 changes: 146 additions & 36 deletions pkg/operator/controllers/dnsmasq/cluster_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"testing"
"time"

"github.com/golang/mock/gomock"
configv1 "github.com/openshift/api/config/v1"
operatorv1 "github.com/openshift/api/operator/v1"
mcv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1"
Expand All @@ -20,7 +19,8 @@ import (

"github.com/Azure/ARO-RP/pkg/operator"
arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
mock_dynamichelper "github.com/Azure/ARO-RP/pkg/util/mocks/dynamichelper"
"github.com/Azure/ARO-RP/pkg/util/clienthelper"
testclienthelper "github.com/Azure/ARO-RP/test/util/clienthelper"
utilconditions "github.com/Azure/ARO-RP/test/util/conditions"
utilerror "github.com/Azure/ARO-RP/test/util/error"
)
Expand All @@ -32,24 +32,22 @@ func TestClusterReconciler(t *testing.T) {
defaultDegraded := utilconditions.ControllerDefaultDegraded(ClusterControllerName)
defaultConditions := []operatorv1.OperatorCondition{defaultAvailable, defaultProgressing, defaultDegraded}

fakeDh := func(controller *gomock.Controller) *mock_dynamichelper.MockInterface {
return mock_dynamichelper.NewMockInterface(controller)
}

tests := []struct {
name string
objects []client.Object
mocks func(mdh *mock_dynamichelper.MockInterface)
wantCreated map[string]int
wantUpdated map[string]int
request ctrl.Request
wantErrMsg string
wantConditions []operatorv1.OperatorCondition
}{
{
name: "no cluster",
objects: []client.Object{},
mocks: func(mdh *mock_dynamichelper.MockInterface) {},
request: ctrl.Request{},
wantErrMsg: "clusters.aro.openshift.io \"cluster\" not found",
name: "no cluster",
objects: []client.Object{},
wantCreated: map[string]int{},
wantUpdated: map[string]int{},
request: ctrl.Request{},
wantErrMsg: "clusters.aro.openshift.io \"cluster\" not found",
},
{
name: "controller disabled",
Expand All @@ -67,7 +65,8 @@ func TestClusterReconciler(t *testing.T) {
},
},
},
mocks: func(mdh *mock_dynamichelper.MockInterface) {},
wantCreated: map[string]int{},
wantUpdated: map[string]int{},
request: ctrl.Request{},
wantErrMsg: "",
wantConditions: defaultConditions,
Expand Down Expand Up @@ -96,9 +95,8 @@ func TestClusterReconciler(t *testing.T) {
},
},
},
mocks: func(mdh *mock_dynamichelper.MockInterface) {
mdh.EXPECT().Ensure(gomock.Any()).Times(1)
},
wantCreated: map[string]int{},
wantUpdated: map[string]int{},
request: ctrl.Request{},
wantErrMsg: "",
wantConditions: defaultConditions,
Expand All @@ -124,9 +122,10 @@ func TestClusterReconciler(t *testing.T) {
Spec: mcv1.MachineConfigPoolSpec{},
},
},
mocks: func(mdh *mock_dynamichelper.MockInterface) {
mdh.EXPECT().Ensure(gomock.Any(), gomock.AssignableToTypeOf(&mcv1.MachineConfig{})).Times(1)
wantCreated: map[string]int{
"MachineConfig//99-master-aro-dns": 1,
},
wantUpdated: map[string]int{},
request: ctrl.Request{},
wantErrMsg: "",
wantConditions: defaultConditions,
Expand All @@ -151,20 +150,105 @@ func TestClusterReconciler(t *testing.T) {
Spec: mcv1.MachineConfigPoolSpec{},
},
},
mocks: func(mdh *mock_dynamichelper.MockInterface) {},
request: ctrl.Request{},
wantErrMsg: `clusterversions.config.openshift.io "version" not found`,
wantCreated: map[string]int{},
wantUpdated: map[string]int{},
request: ctrl.Request{},
wantErrMsg: `error getting the ClusterVersion: clusterversions.config.openshift.io "version" not found`,
wantConditions: []operatorv1.OperatorCondition{
defaultAvailable, defaultProgressing, {
Type: "DnsmasqClusterControllerDegraded",
Status: "True",
Message: `clusterversions.config.openshift.io "version" not found`,
Message: `error getting the ClusterVersion: clusterversions.config.openshift.io "version" not found`,
LastTransitionTime: transitionTime,
},
},
},
{
name: "valid MachineConfigPool, cluster not updating, not forced, does nothing",
name: "valid MachineConfigPool, cluster not updating, not forced, does create",
objects: []client.Object{
&arov1alpha1.Cluster{
ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
Status: arov1alpha1.ClusterStatus{
Conditions: defaultConditions,
},
Spec: arov1alpha1.ClusterSpec{
OperatorFlags: arov1alpha1.OperatorFlags{
operator.DnsmasqEnabled: operator.FlagTrue,
},
},
},
&mcv1.MachineConfigPool{
ObjectMeta: metav1.ObjectMeta{Name: "master"},
Status: mcv1.MachineConfigPoolStatus{},
Spec: mcv1.MachineConfigPoolSpec{},
},
&configv1.ClusterVersion{
ObjectMeta: metav1.ObjectMeta{
Name: "version",
},
Status: configv1.ClusterVersionStatus{
History: []configv1.UpdateHistory{
{
State: configv1.CompletedUpdate,
Version: "4.10.11",
},
},
},
},
},
wantCreated: map[string]int{
"MachineConfig//99-master-aro-dns": 1,
},
wantUpdated: map[string]int{},
request: ctrl.Request{},
wantErrMsg: "",
wantConditions: defaultConditions,
},
{
name: "valid MachineConfigPool, cluster not updating, not forced, does not update",
objects: []client.Object{
&arov1alpha1.Cluster{
ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
Status: arov1alpha1.ClusterStatus{
Conditions: defaultConditions,
},
Spec: arov1alpha1.ClusterSpec{
OperatorFlags: arov1alpha1.OperatorFlags{
operator.DnsmasqEnabled: operator.FlagTrue,
},
},
},
&mcv1.MachineConfigPool{
ObjectMeta: metav1.ObjectMeta{Name: "master"},
Status: mcv1.MachineConfigPoolStatus{},
Spec: mcv1.MachineConfigPoolSpec{},
},
&mcv1.MachineConfig{
ObjectMeta: metav1.ObjectMeta{Name: "99-master-aro-dns"},
Spec: mcv1.MachineConfigSpec{},
},
&configv1.ClusterVersion{
ObjectMeta: metav1.ObjectMeta{
Name: "version",
},
Status: configv1.ClusterVersionStatus{
History: []configv1.UpdateHistory{
{
State: configv1.CompletedUpdate,
Version: "4.10.11",
},
},
},
},
},
wantCreated: map[string]int{},
wantUpdated: map[string]int{},
request: ctrl.Request{},
wantErrMsg: "",
wantConditions: defaultConditions,
},
{
name: "valid MachineConfigPool, while cluster updating, updates existing ARO DNS MachineConfig",
objects: []client.Object{
&arov1alpha1.Cluster{
ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
Expand All @@ -182,10 +266,19 @@ func TestClusterReconciler(t *testing.T) {
Status: mcv1.MachineConfigPoolStatus{},
Spec: mcv1.MachineConfigPoolSpec{},
},
&mcv1.MachineConfig{
ObjectMeta: metav1.ObjectMeta{Name: "99-master-aro-dns"},
Spec: mcv1.MachineConfigSpec{},
},
&configv1.ClusterVersion{
ObjectMeta: metav1.ObjectMeta{
Name: "version",
},
Spec: configv1.ClusterVersionSpec{
DesiredUpdate: &configv1.Update{
Version: "4.10.12",
},
},
Status: configv1.ClusterVersionStatus{
History: []configv1.UpdateHistory{
{
Expand All @@ -196,8 +289,9 @@ func TestClusterReconciler(t *testing.T) {
},
},
},
mocks: func(mdh *mock_dynamichelper.MockInterface) {
mdh.EXPECT().Ensure(gomock.Any(), gomock.Any()).Times(0)
wantCreated: map[string]int{},
wantUpdated: map[string]int{
"MachineConfig//99-master-aro-dns": 1,
},
request: ctrl.Request{},
wantErrMsg: "",
Expand Down Expand Up @@ -241,10 +335,10 @@ func TestClusterReconciler(t *testing.T) {
},
},
},
mocks: func(mdh *mock_dynamichelper.MockInterface) {
mdh.EXPECT().Ensure(gomock.Any(), gomock.AssignableToTypeOf(&mcv1.MachineConfig{})).Times(1)
wantCreated: map[string]int{
"MachineConfig//99-master-aro-dns": 1,
},

wantUpdated: map[string]int{},
request: ctrl.Request{},
wantErrMsg: "",
wantConditions: defaultConditions,
Expand All @@ -253,26 +347,42 @@ func TestClusterReconciler(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
controller := gomock.NewController(t)
defer controller.Finish()
createTally := make(map[string]int)
updateTally := make(map[string]int)

client := ctrlfake.NewClientBuilder().
client := testclienthelper.NewHookingClient(ctrlfake.NewClientBuilder().
WithObjects(tt.objects...).
Build()
Build()).WithCreateHook(testclienthelper.TallyCountsAndKey(createTally)).WithUpdateHook(testclienthelper.TallyCountsAndKey(updateTally))

dh := fakeDh(controller)
tt.mocks(dh)
log := logrus.NewEntry(logrus.StandardLogger())
ch := clienthelper.NewWithClient(log, client)

r := NewClusterReconciler(
logrus.NewEntry(logrus.StandardLogger()),
log,
client,
dh,
ch,
)
ctx := context.Background()
_, err := r.Reconcile(ctx, tt.request)

utilerror.AssertErrorMessage(t, err, tt.wantErrMsg)
utilconditions.AssertControllerConditions(t, ctx, client, tt.wantConditions)

e, err := testclienthelper.CompareTally(tt.wantCreated, createTally)
if err != nil {
t.Errorf("create comparison: %v", err)
for _, i := range e {
t.Error(i)
}
}

e, err = testclienthelper.CompareTally(tt.wantUpdated, updateTally)
if err != nil {
t.Errorf("update comparison: %v", err)
for _, i := range e {
t.Error(i)
}
}
})
}
}
Loading

0 comments on commit 40629c1

Please sign in to comment.