Skip to content

Commit

Permalink
Add Unit tests for cloud/scope/vpc.go & Refactor some other unit tests (
Browse files Browse the repository at this point in the history 
#182)


---------

Co-authored-by: Khaja Omer <[email protected]>
  • Loading branch information
@komer3
komer3 and komer3 authored Mar 12, 2024
1 parent 77e6078 commit dd587ad
Show file tree
Hide file tree
Showing 13 changed files with 482 additions and 251 deletions.
5 changes: 0 additions & 5 deletions cloud/scope/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"

"github.com/linode/linodego"
"sigs.k8s.io/cluster-api/util/patch"
"sigs.k8s.io/controller-runtime/pkg/client"
)

Expand All @@ -27,7 +26,3 @@ func CreateLinodeObjectStorageClient(apiKey string) (LinodeObjectStorageClient,
type k8sClient interface {
client.Client
}

type PatchHelper interface {
Patch(ctx context.Context, obj client.Object, opts ...patch.Option) error
}
7 changes: 4 additions & 3 deletions cloud/scope/cluster.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (

"github.com/linode/linodego"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
"sigs.k8s.io/cluster-api/util/patch"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"

infrav1alpha1 "github.com/linode/cluster-api-provider-linode/api/v1alpha1"
Expand All @@ -48,7 +49,7 @@ func validateClusterScopeParams(params ClusterScopeParams) error {

// NewClusterScope creates a new Scope from the supplied parameters.
// This is meant to be called for each reconcile iteration.
func NewClusterScope(ctx context.Context, apiKey string, params ClusterScopeParams, patchNewHelper patchNewHelper) (*ClusterScope, error) {
func NewClusterScope(ctx context.Context, apiKey string, params ClusterScopeParams) (*ClusterScope, error) {
if err := validateClusterScopeParams(params); err != nil {
return nil, err
}
Expand All @@ -66,7 +67,7 @@ func NewClusterScope(ctx context.Context, apiKey string, params ClusterScopePara
return nil, fmt.Errorf("failed to create linode client: %w", err)
}

helper, err := patchNewHelper(params.LinodeCluster, params.Client)
helper, err := patch.NewHelper(params.LinodeCluster, params.Client)
if err != nil {
return nil, fmt.Errorf("failed to init patch helper: %w", err)
}
Expand All @@ -83,7 +84,7 @@ func NewClusterScope(ctx context.Context, apiKey string, params ClusterScopePara
// ClusterScope defines the basic context for an actuator to operate upon.
type ClusterScope struct {
client k8sClient
PatchHelper PatchHelper
PatchHelper *patch.Helper
LinodeClient *linodego.Client
Cluster *clusterv1.Cluster
LinodeCluster *infrav1alpha1.LinodeCluster
Expand Down
130 changes: 62 additions & 68 deletions cloud/scope/cluster_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ import (
"go.uber.org/mock/gomock"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
"sigs.k8s.io/cluster-api/util/patch"
"sigs.k8s.io/controller-runtime/pkg/client"

infrav1alpha1 "github.com/linode/cluster-api-provider-linode/api/v1alpha1"
Expand Down Expand Up @@ -100,9 +100,9 @@ func TestClusterScopeMethods(t *testing.T) {
}

tests := []struct {
name string
fields fields
patchErr error
name string
fields fields
expects func(mock *mock.Mockk8sClient)
}{
{
name: "Success - finalizer should be added to the Linode Cluster object",
Expand All @@ -114,7 +114,14 @@ func TestClusterScopeMethods(t *testing.T) {
},
},
},
patchErr: nil,
expects: func(mock *mock.Mockk8sClient) {
mock.EXPECT().Scheme().DoAndReturn(func() *runtime.Scheme {
s := runtime.NewScheme()
infrav1alpha1.AddToScheme(s)
return s
}).Times(2)
mock.EXPECT().Patch(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
},
},
{
name: "AddFinalizer error - finalizer should not be added to the Linode Cluster object. Function returns nil since it was already present",
Expand All @@ -127,7 +134,13 @@ func TestClusterScopeMethods(t *testing.T) {
},
},
},
patchErr: nil,
expects: func(mock *mock.Mockk8sClient) {
mock.EXPECT().Scheme().DoAndReturn(func() *runtime.Scheme {
s := runtime.NewScheme()
infrav1alpha1.AddToScheme(s)
return s
}).Times(1)
},
},
}
for _, tt := range tests {
Expand All @@ -138,24 +151,20 @@ func TestClusterScopeMethods(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

mockPatchHelper := mock.NewMockPatchHelper(ctrl)
mockK8sClient := mock.NewMockk8sClient(ctrl)

lClient, err := CreateLinodeClient("test-key")
if err != nil {
t.Errorf("createLinodeClient() error = %v", err)
}

cScope := &ClusterScope{
client: mockK8sClient,
PatchHelper: mockPatchHelper,
LinodeClient: lClient,
Cluster: testcase.fields.Cluster,
LinodeCluster: testcase.fields.LinodeCluster,
}
testcase.expects(mockK8sClient)

if cScope.LinodeCluster.Finalizers == nil {
mockPatchHelper.EXPECT().Patch(gomock.Any(), gomock.Any()).Return(testcase.patchErr)
cScope, err := NewClusterScope(
context.Background(),
"test-key",
ClusterScopeParams{
Cluster: testcase.fields.Cluster,
LinodeCluster: testcase.fields.LinodeCluster,
Client: mockK8sClient,
})
if err != nil {
t.Errorf("ClusterScope() error = %v", err)
}

if err := cScope.AddFinalizer(context.Background()); err != nil {
Expand All @@ -179,8 +188,7 @@ func TestNewClusterScope(t *testing.T) {
name string
args args
expectedError error
getFunc func(ctx context.Context, key types.NamespacedName, obj *corev1.Secret, opts ...client.GetOption) error
patchFunc func(obj client.Object, crClient client.Client) (*patch.Helper, error)
expects func(mock *mock.Mockk8sClient)
}{
{
name: "Success - Pass in valid args and get a valid ClusterScope",
Expand All @@ -192,11 +200,12 @@ func TestNewClusterScope(t *testing.T) {
},
},
expectedError: nil,
getFunc: func(ctx context.Context, key types.NamespacedName, obj *corev1.Secret, opts ...client.GetOption) error {
return nil
},
patchFunc: func(obj client.Object, crClient client.Client) (*patch.Helper, error) {
return &patch.Helper{}, nil
expects: func(mock *mock.Mockk8sClient) {
mock.EXPECT().Scheme().DoAndReturn(func() *runtime.Scheme {
s := runtime.NewScheme()
infrav1alpha1.AddToScheme(s)
return s
})
},
},
{
Expand All @@ -217,18 +226,21 @@ func TestNewClusterScope(t *testing.T) {
},
},
expectedError: nil,
getFunc: func(ctx context.Context, key types.NamespacedName, obj *corev1.Secret, opts ...client.GetOption) error {
cred := corev1.Secret{
Data: map[string][]byte{
"apiToken": []byte("example"),
},
}
*obj = cred

return nil
},
patchFunc: func(obj client.Object, crClient client.Client) (*patch.Helper, error) {
return &patch.Helper{}, nil
expects: func(mock *mock.Mockk8sClient) {
mock.EXPECT().Scheme().DoAndReturn(func() *runtime.Scheme {
s := runtime.NewScheme()
infrav1alpha1.AddToScheme(s)
return s
})
mock.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(func(ctx context.Context, key types.NamespacedName, obj *corev1.Secret, opts ...client.GetOption) error {
cred := corev1.Secret{
Data: map[string][]byte{
"apiToken": []byte("example"),
},
}
*obj = cred
return nil
})
},
},
{
Expand All @@ -238,12 +250,7 @@ func TestNewClusterScope(t *testing.T) {
params: ClusterScopeParams{},
},
expectedError: fmt.Errorf("cluster is required when creating a ClusterScope"),
getFunc: func(ctx context.Context, key types.NamespacedName, obj *corev1.Secret, opts ...client.GetOption) error {
return nil
},
patchFunc: func(obj client.Object, crClient client.Client) (*patch.Helper, error) {
return &patch.Helper{}, nil
},
expects: func(mock *mock.Mockk8sClient) {},
},
{
name: "Error - patchHelper returns error. Checking error handle for when new patchHelper is invoked",
Expand All @@ -254,12 +261,9 @@ func TestNewClusterScope(t *testing.T) {
LinodeCluster: &infrav1alpha1.LinodeCluster{},
},
},
expectedError: fmt.Errorf("failed to init patch helper: obj is nil"),
getFunc: func(ctx context.Context, key types.NamespacedName, obj *corev1.Secret, opts ...client.GetOption) error {
return nil
},
patchFunc: func(obj client.Object, crClient client.Client) (*patch.Helper, error) {
return nil, fmt.Errorf("obj is nil")
expectedError: fmt.Errorf("failed to init patch helper:"),
expects: func(mock *mock.Mockk8sClient) {
mock.EXPECT().Scheme().Return(runtime.NewScheme())
},
},
{
Expand All @@ -280,11 +284,8 @@ func TestNewClusterScope(t *testing.T) {
},
},
expectedError: fmt.Errorf("credentials from secret ref: get credentials secret test/example: failed to get secret"),
getFunc: func(ctx context.Context, key types.NamespacedName, obj *corev1.Secret, opts ...client.GetOption) error {
return fmt.Errorf("failed to get secret")
},
patchFunc: func(obj client.Object, crClient client.Client) (*patch.Helper, error) {
return &patch.Helper{}, nil
expects: func(mock *mock.Mockk8sClient) {
mock.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).Return(fmt.Errorf("failed to get secret"))
},
},
{
Expand All @@ -297,12 +298,7 @@ func TestNewClusterScope(t *testing.T) {
},
},
expectedError: fmt.Errorf("failed to create linode client: missing Linode API key"),
getFunc: func(ctx context.Context, key types.NamespacedName, obj *corev1.Secret, opts ...client.GetOption) error {
return nil
},
patchFunc: func(obj client.Object, crClient client.Client) (*patch.Helper, error) {
return &patch.Helper{}, nil
},
expects: func(mock *mock.Mockk8sClient) {},
},
}

Expand All @@ -316,16 +312,14 @@ func TestNewClusterScope(t *testing.T) {

mockK8sClient := mock.NewMockk8sClient(ctrl)

if testcase.args.params.LinodeCluster != nil && testcase.args.params.LinodeCluster.Spec.CredentialsRef != nil {
mockK8sClient.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(testcase.getFunc)
}
testcase.expects(mockK8sClient)

testcase.args.params.Client = mockK8sClient

got, err := NewClusterScope(context.Background(), testcase.args.apiKey, testcase.args.params, testcase.patchFunc)
got, err := NewClusterScope(context.Background(), testcase.args.apiKey, testcase.args.params)

if testcase.expectedError != nil {
assert.EqualError(t, err, testcase.expectedError.Error())
assert.ErrorContains(t, err, testcase.expectedError.Error())
} else {
assert.NotEmpty(t, got)
}
Expand Down
3 changes: 0 additions & 3 deletions cloud/scope/common.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,11 @@ import (
"github.com/linode/linodego"
"golang.org/x/oauth2"
corev1 "k8s.io/api/core/v1"
"sigs.k8s.io/cluster-api/util/patch"
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/linode/cluster-api-provider-linode/version"
)

type patchNewHelper func(obj client.Object, crClient client.Client) (*patch.Helper, error)

func CreateLinodeClient(apiKey string) (*linodego.Client, error) {
if apiKey == "" {
return nil, errors.New("missing Linode API key")
Expand Down
10 changes: 5 additions & 5 deletions cloud/scope/machine.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@ import (
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/types"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/cluster-api/util/patch"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"

infrav1alpha1 "github.com/linode/cluster-api-provider-linode/api/v1alpha1"
)

type MachineScopeParams struct {
Client client.Client
Client k8sClient
Cluster *clusterv1.Cluster
Machine *clusterv1.Machine
LinodeCluster *infrav1alpha1.LinodeCluster
Expand All @@ -26,7 +26,7 @@ type MachineScopeParams struct {
type MachineScope struct {
client k8sClient

PatchHelper PatchHelper
PatchHelper *patch.Helper
Cluster *clusterv1.Cluster
Machine *clusterv1.Machine
LinodeClient *linodego.Client
Expand All @@ -51,7 +51,7 @@ func validateMachineScopeParams(params MachineScopeParams) error {
return nil
}

func NewMachineScope(ctx context.Context, apiKey string, params MachineScopeParams, patchNewHelper patchNewHelper) (*MachineScope, error) {
func NewMachineScope(ctx context.Context, apiKey string, params MachineScopeParams) (*MachineScope, error) {
if err := validateMachineScopeParams(params); err != nil {
return nil, err
}
Expand Down Expand Up @@ -88,7 +88,7 @@ func NewMachineScope(ctx context.Context, apiKey string, params MachineScopePara
return nil, fmt.Errorf("failed to create linode client: %w", err)
}

helper, err := patchNewHelper(params.LinodeMachine, params.Client)
helper, err := patch.NewHelper(params.LinodeMachine, params.Client)
if err != nil {
return nil, fmt.Errorf("failed to init patch helper: %w", err)
}
Expand Down
Loading

0 comments on commit dd587ad

Please sign in to comment.