Address return consistency

pkg/assisted/agent.go

@@ -17,7 +17,7 @@ import (
 )
 
 const (
-	nonExistentMsg = "Cannot update non-existent agent"
+	nonExistentMsg = "cannot update non-existent agent"
 )
 
 // agentBuilder provides struct for the agent object containing connection to
@@ -300,11 +300,7 @@ func (builder *agentBuilder) Update() (*agentBuilder, error) {
 		glog.V(100).Infof("agent %s in namespace %s does not exist",
 			builder.Definition.Name, builder.Definition.Namespace)
 
-		builder.errorMsg = nonExistentMsg
-	}
-
-	if builder.errorMsg != "" {
-		return nil, fmt.Errorf(builder.errorMsg)
+		return builder, fmt.Errorf(nonExistentMsg)
 	}
 
 	err := builder.apiClient.Update(context.TODO(), builder.Definition)

pkg/assisted/agentclusterinstall.go

@@ -604,7 +604,6 @@ func (builder *AgentClusterInstallBuilder) Update(force bool) (*AgentClusterInst
 
 			err = builder.DeleteAndWait(time.Second * 10)
 			builder.Definition.ResourceVersion = ""
-			// fmt.Printf("agentclusterinstall exists: %v\n", builder.Exists())
 
 			if err != nil {
 				glog.V(100).Infof(

pkg/assisted/agentserviceconfig.go

@@ -157,9 +157,7 @@ func (builder *AgentServiceConfigBuilder) WithMirrorRegistryRef(configMapName st
 		glog.V(100).Infof("The configMapName is empty")
 
 		builder.errorMsg = "cannot add agentserviceconfig mirrorRegistryRef with empty configmap name"
-	}
 
-	if builder.errorMsg != "" {
 		return builder
 	}
 
@@ -263,7 +261,7 @@ func (builder *AgentServiceConfigBuilder) WaitUntilDeployed(timeout time.Duratio
 	if !builder.Exists() {
 		glog.V(100).Infof("The agentserviceconfig does not exist on the cluster")
 
-		return builder, fmt.Errorf("cannot wait for non-existent agentserviceconfig to be deployed")
+		return nil, fmt.Errorf("cannot wait for non-existent agentserviceconfig to be deployed")
 	}
 
 	// Polls every retryInterval to determine if agentserviceconfig is in desired state.
@@ -310,7 +308,7 @@ func PullAgentServiceConfig(apiClient *clients.Settings) (*AgentServiceConfigBui
 		return nil, fmt.Errorf("the apiClient is nil")
 	}
 
-	builder := AgentServiceConfigBuilder{
+	builder := &AgentServiceConfigBuilder{
 		apiClient: apiClient.Client,
 		Definition: &agentInstallV1Beta1.AgentServiceConfig{
 			ObjectMeta: metav1.ObjectMeta{
@@ -325,7 +323,7 @@ func PullAgentServiceConfig(apiClient *clients.Settings) (*AgentServiceConfigBui
 
 	builder.Definition = builder.Object
 
-	return &builder, nil
+	return builder, nil
 }
 
 // Get fetches the defined agentserviceconfig from the cluster.
@@ -383,7 +381,7 @@ func (builder *AgentServiceConfigBuilder) Update(force bool) (*AgentServiceConfi
 		glog.V(100).Infof("agentserviceconfig %s does not exist",
 			builder.Definition.Name)
 
-		return builder, fmt.Errorf("cannot update non-existent agentserviceconfig")
+		return nil, fmt.Errorf("cannot update non-existent agentserviceconfig")
 	}
 
 	err := builder.apiClient.Update(context.TODO(), builder.Definition)
@@ -524,13 +522,13 @@ func (builder *AgentServiceConfigBuilder) 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/assisted/infraenv.go

@@ -790,11 +790,7 @@ func (builder *InfraEnvBuilder) Update(force bool) (*InfraEnvBuilder, error) {
 		glog.V(100).Infof("infraenv %s in namespace %s does not exist",
 			builder.Definition.Name, builder.Definition.Namespace)
 
-		builder.errorMsg = "Cannot update non-existent infraenv"
-	}
-
-	if builder.errorMsg != "" {
-		return nil, fmt.Errorf(builder.errorMsg)
+		return nil, fmt.Errorf("cannot update non-existent infraenv")
 	}
 
 	err := builder.apiClient.Update(context.TODO(), builder.Definition)

pkg/console/console.go

@@ -63,7 +63,7 @@ func Pull(apiClient *clients.Settings, name string) (*Builder, error) {
 	if name == "" {
 		glog.V(100).Info("The name of the Console is empty")
 
-		return builder, fmt.Errorf("console 'name' cannot be empty")
+		return nil, fmt.Errorf("console 'name' cannot be empty")
 	}
 
 	glog.V(100).Infof("Pulling cluster console %s", name)

pkg/hive/clusterdeployment.go

@@ -30,6 +30,8 @@
 
 // NewABMClusterDeploymentBuilder creates a new instance of
 // ClusterDeploymentBuilder with platform type set to agentBareMetal.
+//
+//nolint:funlen
 func NewABMClusterDeploymentBuilder(
 	apiClient *clients.Settings,
 	name string,
@@ -135,7 +137,7 @@
 	if clusterInstallRef.Name == "" {
 		glog.V(100).Infof("The clusterInstallRef name of the clusterdeployment is empty")
 
-		builder.errorMsg = "clusterdeployment 'clusterInstallRef.name' cannot be empty"
+		builder.errorMsg = "clusterdeployment 'clusterInstallRef' cannot be empty"
 
 		return builder
 	}

pkg/hive/clusterimageset.go

@@ -297,13 +297,13 @@ func (builder *ClusterImageSetBuilder) 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/icsp/icsp.go

@@ -68,23 +68,23 @@ func NewICSPBuilder(apiClient *clients.Settings, name, source string, mirrors []
 	if name == "" {
 		glog.V(100).Infof("The name of the ImageContentSourcePolicy is empty")
 
-		icspBuilder.errorMsg = "imageContentSourcePolicy 'name' cannot be empty"
+		icspBuilder.errorMsg = "ImageContentSourcePolicy 'name' cannot be empty"
 
 		return icspBuilder
 	}
 
 	if source == "" {
 		glog.V(100).Infof("The Source of the ImageContentSourcePolicy is empty")
 
-		icspBuilder.errorMsg = "imageContentSourcePolicy 'source' cannot be empty"
+		icspBuilder.errorMsg = "ImageContentSourcePolicy 'source' cannot be empty"
 
 		return icspBuilder
 	}
 
 	if len(mirrors) == 0 {
 		glog.V(100).Infof("The mirrors of the ImageContentSourcePolicy are empty")
 
-		icspBuilder.errorMsg = "imageContentSourcePolicy 'mirrors' cannot be empty"
+		icspBuilder.errorMsg = "ImageContentSourcePolicy 'mirrors' cannot be empty"
 
 		return icspBuilder
 	}
@@ -254,15 +254,15 @@ func (builder *ICSPBuilder) WithRepositoryDigestMirror(source string, mirrors []
 	if source == "" {
 		glog.V(100).Infof("The source is empty")
 
-		builder.errorMsg = "imageContentSourcePolicy 'source' cannot be empty"
+		builder.errorMsg = "'source' cannot be empty"
 
 		return builder
 	}
 
 	if len(mirrors) == 0 {
 		glog.V(100).Infof("Mirrors is empty")
 
-		builder.errorMsg = "imageContentSourcePolicy 'mirrors' cannot be empty"
+		builder.errorMsg = "'mirrors' cannot be empty"
 
 		return builder
 	}

pkg/lso/localvolumeset.go

@@ -95,7 +95,7 @@ func PullLocalVolumeSet(apiClient *clients.Settings, name, nsname string) (*Loca
 		return nil, err
 	}
 
-	builder := LocalVolumeSetBuilder{
+	builder := &LocalVolumeSetBuilder{
 		apiClient: apiClient.Client,
 		Definition: &lsov1alpha1.LocalVolumeSet{
 			ObjectMeta: metav1.ObjectMeta{
@@ -123,7 +123,7 @@ func PullLocalVolumeSet(apiClient *clients.Settings, name, nsname string) (*Loca
 
 	builder.Definition = builder.Object
 
-	return &builder, nil
+	return builder, nil
 }
 
 // Get fetches existing localVolumeSet from cluster.

pkg/metallb/bgpadvertisement.go

@@ -254,7 +254,7 @@ func (builder *BGPAdvertisementBuilder) Update(force bool) (*BGPAdvertisementBui
 		}
 	}
 
-	return builder, err
+	return builder, nil
 }
 
 // WithAggregationLength4 adds the specified AggregationLength to the BGPAdvertisement.

pkg/metallb/bgppeer.go

@@ -156,13 +156,13 @@ func PullBGPPeer(apiClient *clients.Settings, name, nsname string) (*BGPPeerBuil
 	if name == "" {
 		glog.V(100).Infof("The name of the bgppeer is empty")
 
-		return nil, fmt.Errorf("bgppeer 'name' cannot be empty")
+		return nil, fmt.Errorf("bgppeer object name cannot be empty")
 	}
 
 	if nsname == "" {
 		glog.V(100).Infof("The namespace of the bgppeer is empty")
 
-		return nil, fmt.Errorf("bgppeer 'namespace' cannot be empty")
+		return nil, fmt.Errorf("bgppeer object namespace cannot be empty")
 	}
 
 	if !builder.Exists() {

pkg/metallb/l2advertisement.go

@@ -257,7 +257,7 @@ func (builder *L2AdvertisementBuilder) Update(force bool) (*L2AdvertisementBuild
 		}
 	}
 
-	return builder, err
+	return builder, nil
 }
 
 // WithNodeSelector adds the specified NodeSelectors to the L2Advertisement.

pkg/nad/builder.go

@@ -276,7 +276,7 @@ func (builder *Builder) GetString() (string, error) {
 		return "", err
 	}
 
-	return string(nadByte), err
+	return string(nadByte), nil
 }
 
 // fillConfigureString adds a configuration string to builder definition specs configuration if needed.

pkg/namespace/namespace.go

@@ -51,6 +51,8 @@ func NewBuilder(apiClient *clients.Settings, name string) *Builder {
 		glog.V(100).Infof("The name of the namespace is empty")
 
 		builder.errorMsg = "namespace 'name' cannot be empty"
+
+		return builder
 	}
 
 	return builder
@@ -199,7 +201,7 @@ func (builder *Builder) Delete() error {
 
 	builder.Object = nil
 
-	return err
+	return nil
 }
 
 // DeleteAndWait deletes a namespace and waits until it is removed from the cluster.

pkg/networkpolicy/multinetegressrule.go

@@ -126,10 +126,6 @@ func (builder *EgressRuleBuilder) WithOptions(options ...EgressAdditionalOptions
 func (builder *EgressRuleBuilder) WithPeerPodSelector(podSelector metav1.LabelSelector) *EgressRuleBuilder {
 	glog.V(100).Infof("Adding peer pod selector %v to EgressRule", podSelector)
 
-	if builder.errorMsg != "" {
-		return builder
-	}
-
 	builder.definition.To = append(builder.definition.To, v1beta1.MultiNetworkPolicyPeer{PodSelector: &podSelector})
 
 	return builder

pkg/networkpolicy/multinetegressrule_test.go

@@ -113,6 +113,7 @@ func TestEgressWithPeerPodSelector(t *testing.T) {
 	assert.Len(t, builder.definition.To, 1)
 	assert.Equal(t, builder.definition.To[0].PodSelector.MatchLabels["app"], "nginx")
 
+	builder = NewEgressRuleBuilder()
 	//nolint:goconst
 	builder.errorMsg = "error"
 
@@ -206,6 +207,7 @@ func TestEgressWithPeerPodSelectorAndCIDR(t *testing.T) {
 	assert.Equal(t, builder.definition.To[0].IPBlock.CIDR, "192.168.0.1/24")
 
 	// Test invalid CIDR
+	builder = NewEgressRuleBuilder()
 	builder.WithPeerPodSelectorAndCIDR(metav1.LabelSelector{
 		MatchLabels: map[string]string{
 			"app": "nginx",
@@ -215,6 +217,7 @@ func TestEgressWithPeerPodSelectorAndCIDR(t *testing.T) {
 	assert.Equal(t, builder.errorMsg, "Invalid CIDR argument 192.55.55.55")
 
 	// Test with exception
+	builder = NewEgressRuleBuilder()
 	builder.WithPeerPodSelectorAndCIDR(metav1.LabelSelector{
 		MatchLabels: map[string]string{
 			"app": "nginx",

pkg/networkpolicy/multinetingressrule.go

@@ -125,10 +125,6 @@ func (builder *IngressRuleBuilder) WithOptions(options ...IngressAdditionalOptio
 func (builder *IngressRuleBuilder) WithPeerPodSelector(podSelector metav1.LabelSelector) *IngressRuleBuilder {
 	glog.V(100).Infof("Adding peer pod selector %v to Ingress Rule", podSelector)
 
-	if builder.errorMsg != "" {
-		return builder
-	}
-
 	builder.definition.From = append(
 		builder.definition.From, v1beta1.MultiNetworkPolicyPeer{
 			PodSelector: &podSelector,
@@ -203,10 +199,6 @@ func (builder *IngressRuleBuilder) WithPeerPodSelectorAndCIDR(
 	podSelector metav1.LabelSelector, cidr string, except ...[]string) *IngressRuleBuilder {
 	glog.V(100).Infof("Adding peer pod selector %v and CIDR %s to IngressRule", podSelector, cidr)
 
-	if builder.errorMsg != "" {
-		return builder
-	}
-
 	builder.WithPeerPodSelector(podSelector)
 	builder.WithCIDR(cidr, except...)
 

pkg/networkpolicy/multinetingressrule_test.go

@@ -112,18 +112,6 @@ func TestIngressWithPeerPodSelector(t *testing.T) {
 
 	assert.Len(t, builder.definition.From, 1)
 	assert.Equal(t, builder.definition.From[0].PodSelector.MatchLabels["app"], "nginx")
-
-	builder = NewIngressRuleBuilder()
-
-	builder.errorMsg = "error"
-
-	builder.WithPeerPodSelector(metav1.LabelSelector{
-		MatchLabels: map[string]string{
-			"app": "nginx",
-		},
-	})
-
-	assert.Len(t, builder.definition.From, 0)
 }
 
 func TestIngressWithPeerNamespaceSelector(t *testing.T) {

pkg/networkpolicy/multinetworkpolicy.go

@@ -84,10 +84,6 @@ func (builder *MultiNetworkPolicyBuilder) WithPodSelector(podSelector metav1.Lab
 		"Creating MultiNetworkPolicy %s in %s namespace with the podSelector defined: %v",
 		builder.Definition.Name, builder.Definition.Namespace, podSelector)
 
-	if builder.errorMsg != "" {
-		return builder
-	}
-
 	builder.Definition.Spec.PodSelector = podSelector
 
 	return builder
@@ -208,7 +204,7 @@ func PullMultiNetworkPolicy(apiClient *clients.Settings, name, nsname string) (*
 		return nil, err
 	}
 
-	builder := MultiNetworkPolicyBuilder{
+	builder := &MultiNetworkPolicyBuilder{
 		apiClient: apiClient.Client,
 		Definition: &v1beta1.MultiNetworkPolicy{
 			ObjectMeta: metav1.ObjectMeta{
@@ -240,7 +236,7 @@ func PullMultiNetworkPolicy(apiClient *clients.Settings, name, nsname string) (*
 
 	builder.Definition = builder.Object
 
-	return &builder, nil
+	return builder, nil
 }
 
 // Get returns MultiNetworkPolicy object if found.