ocm: adjust return consistency (openshift-kni#852)

pkg/ocm/kac.go

@@ -186,7 +186,7 @@ func (builder *KACBuilder) Create() (*KACBuilder, error) {
 
 	builder.Object = builder.Definition
 
-	return builder, err
+	return builder, nil
 }
 
 // Update changes the existing KlusterletAddonConfig resource on the cluster, falling back to deleting and recreating if

pkg/ocm/managedcluster.go

@@ -223,15 +223,18 @@ func (builder *ManagedClusterBuilder) Create() (*ManagedClusterBuilder, error) {
 	glog.V(100).Infof("Creating the managedcluster %s",
 		builder.Definition.Name)
 
-	var err error
-	if !builder.Exists() {
-		err = builder.apiClient.Create(context.TODO(), builder.Definition)
-		if err == nil {
-			builder.Object = builder.Definition
-		}
+	if builder.Exists() {
+		return builder, nil
 	}
 
-	return builder, err
+	err := builder.apiClient.Create(context.TODO(), builder.Definition)
+	if err != nil {
+		return builder, err
+	}
+
+	builder.Object = builder.Definition
+
+	return builder, nil
 }
 
 // validate will check that the builder and builder definition are properly initialized before

pkg/ocm/placementbinding.go

@@ -49,7 +49,7 @@ func NewPlacementBindingBuilder(
 		return nil
 	}
 
-	builder := PlacementBindingBuilder{
+	builder := &PlacementBindingBuilder{
 		apiClient: apiClient.Client,
 		Definition: &policiesv1.PlacementBinding{
 			ObjectMeta: metav1.ObjectMeta{
@@ -63,21 +63,29 @@ func NewPlacementBindingBuilder(
 
 	if name == "" {
 		builder.errorMsg = "placementBinding's 'name' cannot be empty"
+
+		return builder
 	}
 
 	if nsname == "" {
 		builder.errorMsg = "placementBinding's 'nsname' cannot be empty"
+
+		return builder
 	}
 
 	if placementRefErr := validatePlacementRef(placementRef); placementRefErr != "" {
 		builder.errorMsg = placementRefErr
+
+		return builder
 	}
 
 	if subjectErr := validateSubject(subject); subjectErr != "" {
 		builder.errorMsg = subjectErr
+
+		return builder
 	}
 
-	return &builder
+	return builder
 }
 
 // PullPlacementBinding pulls existing placementBinding into Builder struct.
@@ -178,15 +186,18 @@ func (builder *PlacementBindingBuilder) Create() (*PlacementBindingBuilder, erro
 	glog.V(100).Infof("Creating the placementBinding %s in namespace %s",
 		builder.Definition.Name, builder.Definition.Namespace)
 
-	var err error
-	if !builder.Exists() {
-		err = builder.apiClient.Create(context.TODO(), builder.Definition)
-		if err == nil {
-			builder.Object = builder.Definition
-		}
+	if builder.Exists() {
+		return builder, nil
 	}
 
-	return builder, err
+	err := builder.apiClient.Create(context.TODO(), builder.Definition)
+	if err != nil {
+		return builder, err
+	}
+
+	builder.Object = builder.Definition
+
+	return builder, nil
 }
 
 // Delete removes a placementBinding from a cluster.

pkg/ocm/placementrule.go

@@ -45,7 +45,7 @@ func NewPlacementRuleBuilder(apiClient *clients.Settings, name, nsname string) *
 		return nil
 	}
 
-	builder := PlacementRuleBuilder{
+	builder := &PlacementRuleBuilder{
 		apiClient: apiClient.Client,
 		Definition: &placementrulev1.PlacementRule{
 			ObjectMeta: metav1.ObjectMeta{
@@ -59,15 +59,19 @@ func NewPlacementRuleBuilder(apiClient *clients.Settings, name, nsname string) *
 		glog.V(100).Info("The name of the PlacementRule is empty")
 
 		builder.errorMsg = "placementrule's 'name' cannot be empty"
+
+		return builder
 	}
 
 	if nsname == "" {
 		glog.V(100).Info("The namespace of the PlacementRule is empty")
 
 		builder.errorMsg = "placementrule's 'nsname' cannot be empty"
+
+		return builder
 	}
 
-	return &builder
+	return builder
 }
 
 // PullPlacementRule pulls existing placementrule into Builder struct.
@@ -156,7 +160,7 @@ func (builder *PlacementRuleBuilder) Get() (*placementrulev1.PlacementRule, erro
 		return nil, err
 	}
 
-	return placementRule, err
+	return placementRule, nil
 }
 
 // Create makes a placementrule in the cluster and stores the created object in struct.
@@ -168,15 +172,18 @@ func (builder *PlacementRuleBuilder) Create() (*PlacementRuleBuilder, error) {
 	glog.V(100).Infof("Creating the placementrule %s in namespace %s",
 		builder.Definition.Name, builder.Definition.Namespace)
 
-	var err error
-	if !builder.Exists() {
-		err = builder.apiClient.Create(context.TODO(), builder.Definition)
-		if err == nil {
-			builder.Object = builder.Definition
-		}
+	if builder.Exists() {
+		return builder, nil
 	}
 
-	return builder, err
+	err := builder.apiClient.Create(context.TODO(), builder.Definition)
+	if err != nil {
+		return builder, err
+	}
+
+	builder.Object = builder.Definition
+
+	return builder, nil
 }
 
 // Delete removes a placementrule from a cluster.
@@ -246,7 +253,7 @@ func (builder *PlacementRuleBuilder) Update(force bool) (*PlacementRuleBuilder,
 
 	builder.Object = builder.Definition
 
-	return builder, err
+	return builder, nil
 }
 
 // validate will check that the builder and builder definition are properly initialized before
@@ -263,13 +270,13 @@ func (builder *PlacementRuleBuilder) validate() (bool, error) {
 	if builder.Definition == nil {
 		glog.V(100).Infof("The %s is undefined", resourceCRD)
 
-		builder.errorMsg = msg.UndefinedCrdObjectErrString(resourceCRD)
+		return false, fmt.Errorf(msg.UndefinedCrdObjectErrString(resourceCRD))
 	}
 
 	if builder.apiClient == nil {
 		glog.V(100).Infof("The %s builder apiclient is nil", resourceCRD)
 
-		builder.errorMsg = fmt.Sprintf("%s builder cannot have nil apiClient", resourceCRD)
+		return false, fmt.Errorf("%s builder cannot have nil apiClient", resourceCRD)
 	}
 
 	if builder.errorMsg != "" {

pkg/ocm/policy.go

@@ -49,7 +49,7 @@ func NewPolicyBuilder(
 		return nil
 	}
 
-	builder := PolicyBuilder{
+	builder := &PolicyBuilder{
 		apiClient: apiClient.Client,
 		Definition: &policiesv1.Policy{
 			ObjectMeta: metav1.ObjectMeta{
@@ -66,21 +66,27 @@ func NewPolicyBuilder(
 		glog.V(100).Info("The name of the Policy is empty")
 
 		builder.errorMsg = "policy 'name' cannot be empty"
+
+		return builder
 	}
 
 	if nsname == "" {
 		glog.V(100).Info("The namespace of the Policy is empty")
 
 		builder.errorMsg = "policy 'nsname' cannot be empty"
+
+		return builder
 	}
 
 	if template == nil {
 		glog.V(100).Info("The PolicyTemplate of the Policy is nil")
 
 		builder.errorMsg = "policy 'template' cannot be nil"
+
+		return builder
 	}
 
-	return &builder
+	return builder
 }
 
 // PullPolicy pulls existing policy into Builder struct.
@@ -100,7 +106,7 @@ func PullPolicy(apiClient *clients.Settings, name, nsname string) (*PolicyBuilde
 		return nil, err
 	}
 
-	builder := PolicyBuilder{
+	builder := &PolicyBuilder{
 		apiClient: apiClient.Client,
 		Definition: &policiesv1.Policy{
 			ObjectMeta: metav1.ObjectMeta{
@@ -128,7 +134,7 @@ func PullPolicy(apiClient *clients.Settings, name, nsname string) (*PolicyBuilde
 
 	builder.Definition = builder.Object
 
-	return &builder, nil
+	return builder, nil
 }
 
 // Exists checks whether the given policy exists.
@@ -169,7 +175,7 @@ func (builder *PolicyBuilder) Get() (*policiesv1.Policy, error) {
 		return nil, err
 	}
 
-	return policy, err
+	return policy, nil
 }
 
 // Create makes a policy in the cluster and stores the created object in struct.
@@ -181,15 +187,18 @@ func (builder *PolicyBuilder) Create() (*PolicyBuilder, error) {
 	glog.V(100).Infof("Creating the policy %s in namespace %s",
 		builder.Definition.Name, builder.Definition.Namespace)
 
-	var err error
-	if !builder.Exists() {
-		err = builder.apiClient.Create(context.TODO(), builder.Definition)
-		if err == nil {
-			builder.Object = builder.Definition
-		}
+	if builder.Exists() {
+		return builder, nil
+	}
+
+	err := builder.apiClient.Create(context.TODO(), builder.Definition)
+	if err != nil {
+		return builder, err
 	}
 
-	return builder, err
+	builder.Object = builder.Definition
+
+	return builder, nil
 }
 
 // Delete removes a policy from a cluster.
@@ -260,7 +269,7 @@ func (builder *PolicyBuilder) Update(force bool) (*PolicyBuilder, error) {
 
 	builder.Object = builder.Definition
 
-	return builder, err
+	return builder, nil
 }
 
 // WithRemediationAction sets a RemediationAction in the policy definition.
@@ -426,13 +435,13 @@ func (builder *PolicyBuilder) validate() (bool, error) {
 	if builder.Definition == nil {
 		glog.V(100).Infof("The %s is undefined", resourceCRD)
 
-		builder.errorMsg = msg.UndefinedCrdObjectErrString(resourceCRD)
+		return false, fmt.Errorf(msg.UndefinedCrdObjectErrString(resourceCRD))
 	}
 
 	if builder.apiClient == nil {
 		glog.V(100).Infof("The %s builder apiclient is nil", resourceCRD)
 
-		builder.errorMsg = fmt.Sprintf("%s builder cannot have nil apiClient", resourceCRD)
+		return false, fmt.Errorf("%s builder cannot have nil apiClient", resourceCRD)
 	}
 
 	if builder.errorMsg != "" {