Address return consistency

add argocd applications updates
openshift-kni · Jan 20, 2025 · 0d5077b · 0d5077b
1 parent 3234969
commit 0d5077b
Show file tree

Hide file tree

Showing 36 changed files with 118 additions and 102 deletions.
diff --git a/pkg/assisted/agent.go b/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)

diff --git a/pkg/assisted/agentclusterinstall.go b/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(

diff --git a/pkg/assisted/agentserviceconfig.go b/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 != "" {

diff --git a/pkg/assisted/infraenv.go b/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)

diff --git a/pkg/console/console.go b/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)

diff --git a/pkg/hive/clusterdeployment.go b/pkg/hive/clusterdeployment.go
@@ -30,6 +30,8 @@ type ClusterDeploymentAdditionalOptions func(builder *ClusterDeploymentBuilder)
 
 // 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 @@ func NewClusterDeploymentByInstallRefBuilder(
 	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
 	}

diff --git a/pkg/hive/clusterimageset.go b/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 != "" {

diff --git a/pkg/icsp/icsp.go b/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
 	}

diff --git a/pkg/lso/localvolumeset.go b/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.

diff --git a/pkg/metallb/bgpadvertisement.go b/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.

diff --git a/pkg/metallb/bgppeer.go b/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() {

diff --git a/pkg/metallb/l2advertisement.go b/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.

diff --git a/pkg/nad/builder.go b/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.

diff --git a/pkg/networkpolicy/multinetegressrule_test.go b/pkg/networkpolicy/multinetegressrule_test.go
@@ -114,7 +114,6 @@ func TestEgressWithPeerPodSelector(t *testing.T) {
 	assert.Equal(t, builder.definition.To[0].PodSelector.MatchLabels["app"], "nginx")
 
 	builder = NewEgressRuleBuilder()
-
 	//nolint:goconst
 	builder.errorMsg = "error"
 
@@ -210,6 +209,7 @@ func TestEgressWithPeerPodSelectorAndCIDR(t *testing.T) {
 	builder = NewEgressRuleBuilder()
 
 	// Test invalid CIDR
+	builder = NewEgressRuleBuilder()
 	builder.WithPeerPodSelectorAndCIDR(metav1.LabelSelector{
 		MatchLabels: map[string]string{
 			"app": "nginx",
@@ -221,6 +221,7 @@ func TestEgressWithPeerPodSelectorAndCIDR(t *testing.T) {
 	builder = NewEgressRuleBuilder()
 
 	// Test with exception
+	builder = NewEgressRuleBuilder()
 	builder.WithPeerPodSelectorAndCIDR(metav1.LabelSelector{
 		MatchLabels: map[string]string{
 			"app": "nginx",

diff --git a/pkg/networkpolicy/multinetingressrule_test.go b/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) {

diff --git a/pkg/networkpolicy/multinetworkpolicy.go b/pkg/networkpolicy/multinetworkpolicy.go
@@ -204,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{
@@ -236,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.

diff --git a/pkg/nfd/nodefeaturediscovery.go b/pkg/nfd/nodefeaturediscovery.go
@@ -46,6 +46,9 @@ func NewBuilderFromObjectString(apiClient *clients.Settings, almExample string)
 		return nil
 	}
 
+	glog.V(100).Infof(
+		"Initializing Builder definition to NodeFeatureDiscovery object")
+
 	builder := &Builder{
 		apiClient: apiClient,
 	}
@@ -69,7 +72,8 @@ func NewBuilderFromObjectString(apiClient *clients.Settings, almExample string)
 	if builder.Definition == nil {
 		glog.V(100).Infof("The NodeFeatureDiscovery object definition is nil")
 
-		builder.errorMsg = "nodeFeatureDiscovery definition is nil"
+		//nolint:lll
+		builder.errorMsg = "Error initializing NodeFeatureDiscovery from alm-examples: invalid character 'i' looking for beginning of object key string"
 
 		return builder
 	}
@@ -249,7 +253,7 @@ func (builder *Builder) Update(force bool) (*Builder, error) {
 		}
 	}
 
-	return builder, err
+	return builder, nil
 }
 
 // getNodeFeatureDiscoveryFromAlmExample extracts the NodeFeatureDiscovery from the alm-examples block.

diff --git a/pkg/ocm/placementrule.go b/pkg/ocm/placementrule.go
@@ -91,7 +91,7 @@ func PullPlacementRule(apiClient *clients.Settings, name, nsname string) (*Place
 		return nil, err
 	}
 
-	builder := PlacementRuleBuilder{
+	builder := &PlacementRuleBuilder{
 		apiClient: apiClient.Client,
 		Definition: &placementrulev1.PlacementRule{
 			ObjectMeta: metav1.ObjectMeta{
@@ -119,7 +119,7 @@ func PullPlacementRule(apiClient *clients.Settings, name, nsname string) (*Place
 
 	builder.Definition = builder.Object
 
-	return &builder, nil
+	return builder, nil
 }
 
 // Exists checks whether the given placementrule exists.

diff --git a/pkg/olm/clusterserviceversion.go b/pkg/olm/clusterserviceversion.go
@@ -45,7 +45,7 @@ func PullClusterServiceVersion(apiClient *clients.Settings, name, namespace stri
 		return nil, err
 	}
 
-	builder := ClusterServiceVersionBuilder{
+	builder := &ClusterServiceVersionBuilder{
 		apiClient: apiClient,
 		Definition: &oplmV1alpha1.ClusterServiceVersion{
 			ObjectMeta: metav1.ObjectMeta{
@@ -73,7 +73,7 @@ func PullClusterServiceVersion(apiClient *clients.Settings, name, namespace stri
 
 	builder.Definition = builder.Object
 
-	return &builder, nil
+	return builder, nil
 }
 
 // Get returns ClusterServiceVersion object if found.

diff --git a/pkg/pod/pod.go b/pkg/pod/pod.go
@@ -1068,6 +1068,10 @@ func (builder *Builder) WithSecurityContext(securityContext *corev1.PodSecurityC
 
 	builder.isMutationAllowed("SecurityContext")
 
+	if builder.errorMsg != "" {
+		return builder
+	}
+
 	builder.Definition.Spec.SecurityContext = securityContext
 
 	return builder

diff --git a/pkg/rbac/clusterrole_test.go b/pkg/rbac/clusterrole_test.go
@@ -31,7 +31,7 @@ func TestNewClusterRoleBuilder(t *testing.T) {
 			rule: rbacv1.PolicyRule{
 				Resources: []string{"pods"}, APIGroups: []string{"v1"}, Verbs: []string{"get"}},
 			client:            false,
-			expectedErrorText: "clusterRole builder cannot have nil apiClient",
+			expectedErrorText: "",
 		},
 		{
 			name: "",
-Original file line number
+Diff line change
@@ Expand Up @@
     		}
     	}
-    	return builder, err
+    	return builder, nil
     }
     // WithAggregationLength4 adds the specified AggregationLength to the BGPAdvertisement.
@@ Expand Down @@