ocm: added unit tests and increased coverage

Closes rh-ecosystem-edge#544 by adding unit tests for the list and validate functions in the OCM package and modifying a few existing test cases to improve coverage a bit. This PR also resets the resourceversion during force updates to preemptively address situations like in rh-ecosystem-edge#483.

Author: klaskosk

pkg/ocm/kac.go

@@ -180,6 +180,8 @@ func (builder *KACBuilder) Update(force bool) (*KACBuilder, error) {
 			glog.V(100).Infof(msg.FailToUpdateNotification("klusterletAddonConfig", builder.Definition.Name))
 
 			err := builder.Delete()
+			builder.Definition.ResourceVersion = ""
+
 			if err != nil {
 				glog.V(100).Infof(msg.FailToUpdateError("klusterletAddonConfig", builder.Definition.Name))
 

pkg/ocm/kac_test.go

@@ -238,6 +238,11 @@ func TestKACUpdate(t *testing.T) {
 			assert.True(t, kacBuilder.Exists())
 		}
 
+		// This causes an error while updating and allows us to test the code path for force updates.
+		if testCase.force {
+			kacBuilder.Definition.ResourceVersion = kacBuilder.Definition.Name
+		}
+
 		assert.NotNil(t, kacBuilder.Definition)
 		assert.False(t, kacBuilder.Definition.Spec.SearchCollectorConfig.Enabled)
 

pkg/ocm/managedcluster_test.go

@@ -191,6 +191,10 @@ func TestManagedClusterDelete(t *testing.T) {
 			testBuilder:   buildValidManagedClusterTestBuilder(buildTestClientWithDummyManagedCluster()),
 			expectedError: nil,
 		},
+		{
+			testBuilder:   buildValidManagedClusterTestBuilder(clients.GetTestClients(clients.TestClientParams{})),
+			expectedError: nil,
+		},
 		{
 			testBuilder:   buildInvalidManagedClusterTestBuilder(buildTestClientWithDummyManagedCluster()),
 			expectedError: fmt.Errorf("managedCluster 'name' cannot be empty"),
@@ -244,6 +248,82 @@ func TestManagedClusterUpdate(t *testing.T) {
 	}
 }
 
+func TestManagedClusterValidate(t *testing.T) {
+	testCases := []struct {
+		builderNil      bool
+		definitionNil   bool
+		apiClientNil    bool
+		builderErrorMsg string
+		expectedError   error
+	}{
+		{
+			builderNil:      false,
+			definitionNil:   false,
+			apiClientNil:    false,
+			builderErrorMsg: "",
+			expectedError:   nil,
+		},
+		{
+			builderNil:      true,
+			definitionNil:   false,
+			apiClientNil:    false,
+			builderErrorMsg: "",
+			expectedError:   fmt.Errorf("error: received nil managedCluster builder"),
+		},
+		{
+			builderNil:      false,
+			definitionNil:   true,
+			apiClientNil:    false,
+			builderErrorMsg: "",
+			expectedError:   fmt.Errorf("can not redefine the undefined managedCluster"),
+		},
+		{
+			builderNil:      false,
+			definitionNil:   false,
+			apiClientNil:    true,
+			builderErrorMsg: "",
+			expectedError:   fmt.Errorf("managedCluster builder cannot have nil apiClient"),
+		},
+		{
+			builderNil:      false,
+			definitionNil:   false,
+			apiClientNil:    false,
+			builderErrorMsg: "test error",
+			expectedError:   fmt.Errorf("test error"),
+		},
+	}
+
+	for _, testCase := range testCases {
+		managedClusterBuilder := buildValidManagedClusterTestBuilder(clients.GetTestClients(clients.TestClientParams{}))
+
+		if testCase.builderNil {
+			managedClusterBuilder = nil
+		}
+
+		if testCase.definitionNil {
+			managedClusterBuilder.Definition = nil
+		}
+
+		if testCase.apiClientNil {
+			managedClusterBuilder.apiClient = nil
+		}
+
+		if testCase.builderErrorMsg != "" {
+			managedClusterBuilder.errorMsg = testCase.builderErrorMsg
+		}
+
+		valid, err := managedClusterBuilder.validate()
+
+		if testCase.expectedError != nil {
+			assert.False(t, valid)
+			assert.Equal(t, testCase.expectedError, err)
+		} else {
+			assert.True(t, valid)
+			assert.Nil(t, err)
+		}
+	}
+}
+
 // buildDummyManagedCluster returns a ManagedCluster with the provided name.
 func buildDummyManagedCluster(name string) *clusterv1.ManagedCluster {
 	return &clusterv1.ManagedCluster{

pkg/ocm/ocm_test.go

@@ -204,15 +204,13 @@ func (builder *PlacementBindingBuilder) Update(force bool) (*PlacementBindingBui
 	if err != nil {
 		if force {
 			glog.V(100).Infof(
-				"Failed to update the placementBinding object %s. "+
-					"Note: Force flag set, executed delete/create methods instead", builder.Definition.Name)
+				msg.FailToUpdateNotification("placementBinding", builder.Definition.Name, builder.Definition.Namespace))
 
 			builder, err := builder.Delete()
+			builder.Definition.ResourceVersion = ""
 
 			if err != nil {
-				glog.V(100).Infof(
-					"Failed to update the placementBinding object %s, "+
-						"due to error in delete function", builder.Definition.Name)
+				glog.V(100).Infof(msg.FailToUpdateError("placementBinding", builder.Definition.Name, builder.Definition.Namespace))
 
 				return nil, err
 			}

pkg/ocm/placementbinding.go

@@ -14,6 +14,12 @@ import (
 func ListPlacementBindingsInAllNamespaces(apiClient *clients.Settings,
 	options ...runtimeclient.ListOptions) (
 	[]*PlacementBindingBuilder, error) {
+	if apiClient == nil {
+		glog.V(100).Info("PlacementBindings 'apiClient' parameter cannot be nil")
+
+		return nil, fmt.Errorf("failed to list placementBindings, 'apiClient' parameter is nil")
+	}
+
 	logMessage := string("Listing all placementBindings in all namespaces")
 	passedOptions := runtimeclient.ListOptions{}
 

pkg/ocm/placementbindingList.go

@@ -295,6 +295,11 @@ func TestPlacementBindingUpdate(t *testing.T) {
 			assert.Nil(t, err)
 		}
 
+		// This causes an error while updating and allows us to test the code path for force updates.
+		if testCase.force {
+			testBuilder.Definition.ResourceVersion = testBuilder.Definition.Name
+		}
+
 		assert.NotNil(t, testBuilder.Definition)
 		assert.Empty(t, testBuilder.Definition.SubFilter)
 
@@ -430,6 +435,82 @@ func TestValidateSubject(t *testing.T) {
 	}
 }
 
+func TestPlacementBindingValidate(t *testing.T) {
+	testCases := []struct {
+		builderNil      bool
+		definitionNil   bool
+		apiClientNil    bool
+		builderErrorMsg string
+		expectedError   error
+	}{
+		{
+			builderNil:      false,
+			definitionNil:   false,
+			apiClientNil:    false,
+			builderErrorMsg: "",
+			expectedError:   nil,
+		},
+		{
+			builderNil:      true,
+			definitionNil:   false,
+			apiClientNil:    false,
+			builderErrorMsg: "",
+			expectedError:   fmt.Errorf("error: received nil PlacementBinding builder"),
+		},
+		{
+			builderNil:      false,
+			definitionNil:   true,
+			apiClientNil:    false,
+			builderErrorMsg: "",
+			expectedError:   fmt.Errorf("can not redefine the undefined PlacementBinding"),
+		},
+		{
+			builderNil:      false,
+			definitionNil:   false,
+			apiClientNil:    true,
+			builderErrorMsg: "",
+			expectedError:   fmt.Errorf("PlacementBinding builder cannot have nil apiClient"),
+		},
+		{
+			builderNil:      false,
+			definitionNil:   false,
+			apiClientNil:    false,
+			builderErrorMsg: "test error",
+			expectedError:   fmt.Errorf("test error"),
+		},
+	}
+
+	for _, testCase := range testCases {
+		placementBindingBuilder := buildValidPlacementBindingTestBuilder(clients.GetTestClients(clients.TestClientParams{}))
+
+		if testCase.builderNil {
+			placementBindingBuilder = nil
+		}
+
+		if testCase.definitionNil {
+			placementBindingBuilder.Definition = nil
+		}
+
+		if testCase.apiClientNil {
+			placementBindingBuilder.apiClient = nil
+		}
+
+		if testCase.builderErrorMsg != "" {
+			placementBindingBuilder.errorMsg = testCase.builderErrorMsg
+		}
+
+		valid, err := placementBindingBuilder.validate()
+
+		if testCase.expectedError != nil {
+			assert.False(t, valid)
+			assert.Equal(t, testCase.expectedError, err)
+		} else {
+			assert.True(t, valid)
+			assert.Nil(t, err)
+		}
+	}
+}
+
 // buildDummyPlacementBinding returns a PlacementBinding with the provided name and namespace.
 func buildDummyPlacementBinding(name, nsname string) *policiesv1.PlacementBinding {
 	return &policiesv1.PlacementBinding{

pkg/ocm/placementbinding_test.go

@@ -0,0 +1,71 @@
+package ocm
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/openshift-kni/eco-goinfra/pkg/clients"
+	"github.com/stretchr/testify/assert"
+	"k8s.io/apimachinery/pkg/labels"
+	runtimeclient "sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+func TestListPlacementBindingsInAllNamespaces(t *testing.T) {
+	testCases := []struct {
+		placementBindings []*PlacementBindingBuilder
+		listOptions       []runtimeclient.ListOptions
+		expectedError     error
+		client            bool
+	}{
+		{
+			placementBindings: []*PlacementBindingBuilder{
+				buildValidPlacementBindingTestBuilder(buildTestClientWithDummyPlacementBinding()),
+			},
+			listOptions:   nil,
+			expectedError: nil,
+			client:        true,
+		},
+		{
+			placementBindings: []*PlacementBindingBuilder{
+				buildValidPlacementBindingTestBuilder(buildTestClientWithDummyPlacementBinding()),
+			},
+			listOptions:   []runtimeclient.ListOptions{{LabelSelector: labels.NewSelector()}},
+			expectedError: nil,
+			client:        true,
+		},
+		{
+			placementBindings: []*PlacementBindingBuilder{
+				buildValidPlacementBindingTestBuilder(buildTestClientWithDummyPlacementBinding()),
+			},
+			listOptions: []runtimeclient.ListOptions{
+				{LabelSelector: labels.NewSelector()},
+				{LabelSelector: labels.NewSelector()},
+			},
+			expectedError: fmt.Errorf("error: more than one ListOptions was passed"),
+			client:        true,
+		},
+		{
+			placementBindings: []*PlacementBindingBuilder{
+				buildValidPlacementBindingTestBuilder(buildTestClientWithDummyPlacementBinding()),
+			},
+			listOptions:   []runtimeclient.ListOptions{{LabelSelector: labels.NewSelector()}},
+			expectedError: fmt.Errorf("failed to list placementBindings, 'apiClient' parameter is nil"),
+			client:        false,
+		},
+	}
+
+	for _, testCase := range testCases {
+		var testSettings *clients.Settings
+
+		if testCase.client {
+			testSettings = buildTestClientWithDummyPlacementBinding()
+		}
+
+		builders, err := ListPlacementBindingsInAllNamespaces(testSettings, testCase.listOptions...)
+		assert.Equal(t, err, testCase.expectedError)
+
+		if testCase.expectedError == nil && len(testCase.listOptions) == 0 {
+			assert.Equal(t, len(testCase.placementBindings), len(builders))
+		}
+	}
+}

pkg/ocm/placementbindinglist_test.go

@@ -194,15 +194,13 @@ func (builder *PlacementRuleBuilder) Update(force bool) (*PlacementRuleBuilder,
 	if err != nil {
 		if force {
 			glog.V(100).Infof(
-				"Failed to update the placementrule object %s. "+
-					"Note: Force flag set, executed delete/create methods instead", builder.Definition.Name)
+				msg.FailToUpdateNotification("placementrule", builder.Definition.Name, builder.Definition.Namespace))
 
 			builder, err := builder.Delete()
+			builder.Definition.ResourceVersion = ""
 
 			if err != nil {
-				glog.V(100).Infof(
-					"Failed to update the placementrule object %s, "+
-						"due to error in delete function", builder.Definition.Name)
+				glog.V(100).Infof(msg.FailToUpdateError("placementrule", builder.Definition.Name, builder.Definition.Namespace))
 
 				return nil, err
 			}