sriov: adjust return consistency (#866)

pkg/sriov/network.go

@@ -52,7 +52,7 @@ func NewNetworkBuilder(
 		return nil
 	}
 
-	builder := NetworkBuilder{
+	builder := &NetworkBuilder{
 		apiClient: apiClient.Client,
 		Definition: &srIovV1.SriovNetwork{
 			ObjectMeta: metav1.ObjectMeta{
@@ -68,21 +68,29 @@ func NewNetworkBuilder(
 
 	if name == "" {
 		builder.errorMsg = "SrIovNetwork 'name' cannot be empty"
+
+		return builder
 	}
 
 	if nsname == "" {
 		builder.errorMsg = "SrIovNetwork 'nsname' cannot be empty"
+
+		return builder
 	}
 
 	if targetNsname == "" {
 		builder.errorMsg = "SrIovNetwork 'targetNsname' cannot be empty"
+
+		return builder
 	}
 
 	if resName == "" {
 		builder.errorMsg = "SrIovNetwork 'resName' cannot be empty"
+
+		return builder
 	}
 
-	return &builder
+	return builder
 }
 
 // WithVLAN sets vlan id in the SrIovNetwork definition. Allowed vlanId range is between 0-4094.
@@ -93,9 +101,7 @@ func (builder *NetworkBuilder) WithVLAN(vlanID uint16) *NetworkBuilder {
 
 	if vlanID > 4094 {
 		builder.errorMsg = "invalid vlanID, allowed vlanID values are between 0-4094"
-	}
 
-	if builder.errorMsg != "" {
 		return builder
 	}
 
@@ -145,10 +151,6 @@ func (builder *NetworkBuilder) WithMetaPluginAllMultiFlag(allMultiFlag bool) *Ne
 
 	builder.Definition.Spec.MetaPluginsConfig = fmt.Sprintf(`{ "type": "tuning", "allmulti": %t }`, allMultiFlag)
 
-	if builder.errorMsg != "" {
-		return builder
-	}
-
 	return builder
 }
 
@@ -173,9 +175,7 @@ func (builder *NetworkBuilder) WithLinkState(linkState string) *NetworkBuilder {
 
 	if !slices.Contains(allowedLinkStates, linkState) {
 		builder.errorMsg = "invalid 'linkState' parameters"
-	}
 
-	if builder.errorMsg != "" {
 		return builder
 	}
 
@@ -232,9 +232,7 @@ func (builder *NetworkBuilder) WithVlanQoS(qoSClass uint16) *NetworkBuilder {
 
 	if qoSClass > 7 {
 		builder.errorMsg = "Invalid QoS class. Supported vlan QoS class values are between 0...7"
-	}
 
-	if builder.errorMsg != "" {
 		return builder
 	}
 
@@ -322,7 +320,7 @@ func PullNetwork(apiClient *clients.Settings, name, nsname string) (*NetworkBuil
 		return nil, err
 	}
 
-	builder := NetworkBuilder{
+	builder := &NetworkBuilder{
 		apiClient: apiClient.Client,
 		Definition: &srIovV1.SriovNetwork{
 			ObjectMeta: metav1.ObjectMeta{
@@ -350,7 +348,7 @@ func PullNetwork(apiClient *clients.Settings, name, nsname string) (*NetworkBuil
 
 	builder.Definition = builder.Object
 
-	return &builder, nil
+	return builder, nil
 }
 
 // Get returns CatalogSource object if found.
@@ -559,9 +557,7 @@ func (builder *NetworkBuilder) withIpam(ipamType string) *NetworkBuilder {
 		glog.V(100).Infof("sriov network 'ipamType' parameter can not be empty")
 
 		builder.errorMsg = "failed to configure IPAM, 'ipamType' parameter is empty"
-	}
 
-	if builder.errorMsg != "" {
 		return builder
 	}
 
@@ -584,13 +580,13 @@ func (builder *NetworkBuilder) 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/sriov/networknodestate.go

@@ -57,12 +57,16 @@ func NewNetworkNodeStateBuilder(apiClient *clients.Settings, nodeName, nsname st
 		glog.V(100).Infof("The name of the nodeName is empty")
 
 		builder.errorMsg = "SriovNetworkNodeState 'nodeName' is empty"
+
+		return builder
 	}
 
 	if nsname == "" {
 		glog.V(100).Infof("The namespace of the SriovNetworkNodeState is empty")
 
 		builder.errorMsg = "SriovNetworkNodeState 'nsname' is empty"
+
+		return builder
 	}
 
 	return builder
@@ -226,16 +230,16 @@ func (builder *NetworkNodeStateBuilder) findInterfaceByName(sriovInterfaceName s
 		glog.V(100).Infof("Error to discover sriov network node state for node %s", builder.nodeName)
 
 		builder.errorMsg = "failed to discover sriov network node state"
+
+		return nil, err
 	}
 
 	if sriovInterfaceName == "" {
 		glog.V(100).Infof("The sriovInterface can not be empty string")
 
 		builder.errorMsg = "the sriovInterface is an empty sting"
-	}
 
-	if builder.errorMsg != "" {
-		return nil, fmt.Errorf(builder.errorMsg)
+		return nil, fmt.Errorf("sriovInterface can not be empty string")
 	}
 
 	for _, interf := range builder.Objects.Status.Interfaces {
@@ -261,7 +265,7 @@ func (builder *NetworkNodeStateBuilder) validate() (bool, error) {
 	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/sriov/operatorconfig.go

@@ -69,6 +69,8 @@ func NewOperatorConfigBuilder(apiClient *clients.Settings, nsname string) *Opera
 		glog.V(100).Infof("The namespace of the SriovOperatorConfig is empty")
 
 		builder.errorMsg = "SriovOperatorConfig 'nsname' is empty"
+
+		return builder
 	}
 
 	return builder
@@ -114,7 +116,7 @@ func PullOperatorConfig(apiClient *clients.Settings, nsname string) (*OperatorCo
 		return nil, err
 	}
 
-	builder := OperatorConfigBuilder{
+	builder := &OperatorConfigBuilder{
 		apiClient: apiClient.Client,
 		Definition: &srIovV1.SriovOperatorConfig{
 			ObjectMeta: metaV1.ObjectMeta{
@@ -137,7 +139,7 @@ func PullOperatorConfig(apiClient *clients.Settings, nsname string) (*OperatorCo
 
 	builder.Definition = builder.Object
 
-	return &builder, nil
+	return builder, nil
 }
 
 // Get returns CatalogSource object if found.

pkg/sriov/policy.go

@@ -55,7 +55,7 @@ func NewPolicyBuilder(
 		return nil
 	}
 
-	builder := PolicyBuilder{
+	builder := &PolicyBuilder{
 		apiClient: apiClient.Client,
 		Definition: &srIovV1.SriovNetworkNodePolicy{
 			ObjectMeta: metav1.ObjectMeta{
@@ -76,29 +76,41 @@ func NewPolicyBuilder(
 
 	if name == "" {
 		builder.errorMsg = "SriovNetworkNodePolicy 'name' cannot be empty"
+
+		return builder
 	}
 
 	if nsname == "" {
 		builder.errorMsg = "SriovNetworkNodePolicy 'nsname' cannot be empty"
+
+		return builder
 	}
 
 	if resName == "" {
 		builder.errorMsg = "SriovNetworkNodePolicy 'resName' cannot be empty"
+
+		return builder
 	}
 
 	if len(nicNames) == 0 {
 		builder.errorMsg = "SriovNetworkNodePolicy 'nicNames' cannot be empty list"
+
+		return builder
 	}
 
 	if len(nodeSelector) == 0 {
 		builder.errorMsg = "SriovNetworkNodePolicy 'nodeSelector' cannot be empty map"
+
+		return builder
 	}
 
 	if vfsNumber <= 0 {
 		builder.errorMsg = "SriovNetworkNodePolicy 'vfsNumber' cannot be zero of negative"
+
+		return builder
 	}
 
-	return &builder
+	return builder
 }
 
 // WithDevType sets device type in the SriovNetworkNodePolicy definition. Allowed devTypes are vfio-pci and netdevice.
@@ -128,17 +140,19 @@ func (builder *PolicyBuilder) WithVFRange(firstVF, lastVF int) *PolicyBuilder {
 
 	if firstVF < 0 || lastVF < 0 {
 		builder.errorMsg = "firstPF or lastVF can not be less than 0"
+
+		return builder
 	}
 
 	if firstVF > lastVF {
 		builder.errorMsg = "firstPF argument can not be greater than lastPF"
+
+		return builder
 	}
 
 	if lastVF > 63 {
 		builder.errorMsg = "lastVF can not be greater than 63"
-	}
 
-	if builder.errorMsg != "" {
 		return builder
 	}
 
@@ -160,9 +174,7 @@ func (builder *PolicyBuilder) WithMTU(mtu int) *PolicyBuilder {
 
 	if 1 > mtu || mtu > 9192 {
 		builder.errorMsg = fmt.Sprintf("invalid mtu size %d allowed mtu should be in range 1...9192", mtu)
-	}
 
-	if builder.errorMsg != "" {
 		return builder
 	}
 
@@ -252,7 +264,7 @@ func PullPolicy(apiClient *clients.Settings, name, nsname string) (*PolicyBuilde
 		return nil, err
 	}
 
-	builder := PolicyBuilder{
+	builder := &PolicyBuilder{
 		apiClient: apiClient.Client,
 		Definition: &srIovV1.SriovNetworkNodePolicy{
 			ObjectMeta: metav1.ObjectMeta{
@@ -280,7 +292,7 @@ func PullPolicy(apiClient *clients.Settings, name, nsname string) (*PolicyBuilde
 
 	builder.Definition = builder.Object
 
-	return &builder, nil
+	return builder, nil
 }
 
 // Get returns CatalogSource object if found.
@@ -382,13 +394,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 != "" {

pkg/sriov/poolconfig.go

@@ -50,7 +50,7 @@ func NewPoolConfigBuilder(apiClient *clients.Settings, name, nsname string) *Poo
 		return nil
 	}
 
-	builder := PoolConfigBuilder{
+	builder := &PoolConfigBuilder{
 		apiClient: apiClient.Client,
 		Definition: &srIovV1.SriovNetworkPoolConfig{
 			ObjectMeta: metav1.ObjectMeta{
@@ -62,16 +62,16 @@ func NewPoolConfigBuilder(apiClient *clients.Settings, name, nsname string) *Poo
 	if name == "" {
 		builder.errorMsg = "SriovNetworkPoolConfig 'name' cannot be empty"
 
-		return &builder
+		return builder
 	}
 
 	if nsname == "" {
 		builder.errorMsg = "SriovNetworkPoolConfig 'nsname' cannot be empty"
 
-		return &builder
+		return builder
 	}
 
-	return &builder
+	return builder
 }
 
 // Create generates an SriovNetworkPoolConfig in the cluster and stores the created object in struct.
@@ -344,13 +344,13 @@ func (builder *PoolConfigBuilder) 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 != "" {