From 136a5a99af74190cad616db5e42224b7194ff956 Mon Sep 17 00:00:00 2001 From: Brandon Palm Date: Thu, 18 Apr 2024 15:46:14 -0500 Subject: [PATCH] Address return consistency add argocd applications updates --- pkg/assisted/agent.go | 8 +--- pkg/assisted/agentclusterinstall.go | 1 - pkg/assisted/agentserviceconfig.go | 14 +++--- pkg/assisted/infraenv.go | 6 +-- pkg/console/console.go | 2 +- pkg/hive/clusterdeployment.go | 4 +- pkg/hive/clusterimageset.go | 4 +- pkg/icsp/icsp.go | 10 ++-- pkg/kmm/module.go | 2 + pkg/lca/imagebasedupgrade.go | 6 +-- pkg/lso/localvolumeset.go | 4 +- pkg/metallb/addresspool.go | 7 +-- pkg/metallb/bfdprofile.go | 4 ++ pkg/metallb/bgpadvertisement.go | 7 ++- pkg/metallb/bgppeer.go | 9 ++-- pkg/metallb/l2advertisement.go | 2 +- pkg/metallb/metallb.go | 5 +- pkg/nad/builder.go | 2 +- pkg/namespace/namespace.go | 20 ++++---- pkg/network/operator.go | 4 +- pkg/networkpolicy/multinetegressrule.go | 4 -- pkg/networkpolicy/multinetegressrule_test.go | 3 ++ pkg/networkpolicy/multinetingressrule.go | 8 ---- pkg/networkpolicy/multinetingressrule_test.go | 12 ----- pkg/networkpolicy/multinetworkpolicy.go | 8 +--- pkg/nfd/nodefeaturediscovery.go | 15 ++++-- pkg/nfd/nodefeaturediscovery_test.go | 9 ++-- pkg/nmstate/nmstate.go | 16 ++++--- pkg/nmstate/nodenetworkstate.go | 4 +- pkg/nmstate/policy.go | 24 +++++----- pkg/nodes/node.go | 10 ++-- pkg/nto/performanceprofile.go | 16 +++---- pkg/nvidiagpu/clusterpolicy.go | 10 ++-- pkg/ocm/placementbinding.go | 20 +++++--- pkg/ocm/placementrule.go | 18 +++++--- pkg/ocm/policy.go | 20 +++++--- pkg/ocm/policyset.go | 20 +++++--- pkg/olm/catalogsource.go | 16 ++++--- pkg/olm/clusterserviceversion.go | 8 ++-- pkg/olm/installplan.go | 12 +++-- pkg/olm/operatorgroup.go | 8 +++- pkg/olm/packagemanifest.go | 4 +- pkg/olm/subscription.go | 20 ++++---- pkg/pod/container.go | 36 ++++++++------- pkg/pod/container_test.go | 4 +- pkg/pod/pod.go | 36 +++++++-------- pkg/rbac/clusterrole.go | 18 ++++---- pkg/rbac/clusterrole_test.go | 2 +- pkg/rbac/clusterrolebinding.go | 24 +++++----- pkg/rbac/role.go | 14 +++--- pkg/rbac/role_test.go | 8 ++-- pkg/rbac/rolebinding.go | 32 +++++++------ pkg/route/route.go | 24 +++++----- pkg/scc/scc.go | 8 +++- pkg/secret/secret.go | 6 +-- pkg/service/service.go | 12 ++--- pkg/servicemesh/controlplane.go | 46 ++++++++++--------- pkg/servicemesh/memberroll.go | 14 +++--- pkg/sriov-fec/nodeconfig.go | 4 +- pkg/sriov/network.go | 34 +++++++------- pkg/sriov/networknodestate.go | 14 +++--- pkg/sriov/operatorconfig.go | 6 ++- pkg/sriov/policy.go | 44 ++++++++++++++---- pkg/sriov/poolconfig.go | 12 ++--- pkg/statefulset/statefulset.go | 28 +++++++---- pkg/storage/pvc.go | 45 ++++++++---------- pkg/storage/pvc_test.go | 10 ++-- pkg/velero/backup.go | 4 +- pkg/velero/restore.go | 20 +++++--- pkg/webhook/mutatingwebhook.go | 8 ++-- pkg/webhook/validatingwebhook.go | 8 ++-- 71 files changed, 496 insertions(+), 431 deletions(-) diff --git a/pkg/assisted/agent.go b/pkg/assisted/agent.go index 906b55880..7fe2fecb9 100644 --- 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 index 462b3994d..ff0b0d500 100644 --- 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 index 90dbb063c..3aeb63f55 100644 --- 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 index 5d28803cb..52de23504 100644 --- 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 index 00f8d1752..db10b1868 100644 --- 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 index 5376b80ce..5b440b00d 100644 --- 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 index ee10448e9..62e7f04b7 100644 --- 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 index 051d10252..804d5ea4f 100644 --- a/pkg/icsp/icsp.go +++ b/pkg/icsp/icsp.go @@ -68,7 +68,7 @@ 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 } @@ -76,7 +76,7 @@ func NewICSPBuilder(apiClient *clients.Settings, name, source string, mirrors [] 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 } @@ -84,7 +84,7 @@ func NewICSPBuilder(apiClient *clients.Settings, name, source string, mirrors [] 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,7 +254,7 @@ 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 } @@ -262,7 +262,7 @@ func (builder *ICSPBuilder) WithRepositoryDigestMirror(source string, mirrors [] 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/kmm/module.go b/pkg/kmm/module.go index 132ed8c25..e789caad0 100644 --- a/pkg/kmm/module.go +++ b/pkg/kmm/module.go @@ -433,6 +433,8 @@ func (builder *ModuleBuilder) withServiceAccount(srvAccountName string, accountT builder.Definition.Spec.DevicePlugin.ServiceAccountName = srvAccountName default: builder.errorMsg = "invalid account type parameter. Supported parameters are: 'module', 'device'" + + return builder } return builder diff --git a/pkg/lca/imagebasedupgrade.go b/pkg/lca/imagebasedupgrade.go index ea833b652..2f818807e 100644 --- a/pkg/lca/imagebasedupgrade.go +++ b/pkg/lca/imagebasedupgrade.go @@ -121,11 +121,7 @@ func (builder *ImageBasedUpgradeBuilder) Update() (*ImageBasedUpgradeBuilder, er glog.V(100).Infof("imagebasedupgrade %s does not exist", builder.Definition.Name) - builder.errorMsg = "Unable to update non-existing imagebasedupgrade" - } - - if builder.errorMsg != "" { - return nil, fmt.Errorf(builder.errorMsg) + return nil, fmt.Errorf("unable to update non-existing imagebasedupgrade") } err := builder.apiClient.Update(context.TODO(), builder.Definition) diff --git a/pkg/lso/localvolumeset.go b/pkg/lso/localvolumeset.go index 3aa67c70a..d4cbbaa46 100644 --- 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/addresspool.go b/pkg/metallb/addresspool.go index 26b8909a8..8780bf0b8 100644 --- a/pkg/metallb/addresspool.go +++ b/pkg/metallb/addresspool.go @@ -180,9 +180,8 @@ func (builder *IPAddressPoolBuilder) Create() (*IPAddressPoolBuilder, error) { builder.Definition.Name, builder.Definition.Namespace, ) - var err error if !builder.Exists() { - err = builder.apiClient.Create(context.TODO(), builder.Definition) + err := builder.apiClient.Create(context.TODO(), builder.Definition) if err != nil { glog.V(100).Infof("Failed to create IPAddressPool") @@ -192,7 +191,7 @@ func (builder *IPAddressPoolBuilder) Create() (*IPAddressPoolBuilder, error) { builder.Object = builder.Definition - return builder, err + return builder, nil } // Delete removes IPAddressPool object from a cluster. @@ -304,9 +303,7 @@ func (builder *IPAddressPoolBuilder) WithOptions(options ...IPAddressPoolAdditio glog.V(100).Infof("The IPAddressPool is undefined") builder.errorMsg = msg.UndefinedCrdObjectErrString("IPAddressPool") - } - if builder.errorMsg != "" { return builder } diff --git a/pkg/metallb/bfdprofile.go b/pkg/metallb/bfdprofile.go index 0b7ce299c..90bdbf975 100644 --- a/pkg/metallb/bfdprofile.go +++ b/pkg/metallb/bfdprofile.go @@ -354,6 +354,8 @@ func (builder *BFDBuilder) withBoolFlagFor(flagName string, flagValue bool) *BFD builder.Definition.Spec.PassiveMode = &flagValue default: builder.errorMsg = "invalid bool flag name parameter" + + return builder } return builder @@ -377,6 +379,8 @@ func (builder *BFDBuilder) withInterval(intervalName string, interval uint32) *B builder.Definition.Spec.EchoInterval = &interval default: builder.errorMsg = "invalid interval parameters" + + return builder } return builder diff --git a/pkg/metallb/bgpadvertisement.go b/pkg/metallb/bgpadvertisement.go index cf004d463..6b9e3f2a7 100644 --- a/pkg/metallb/bgpadvertisement.go +++ b/pkg/metallb/bgpadvertisement.go @@ -168,9 +168,8 @@ func (builder *BGPAdvertisementBuilder) Create() (*BGPAdvertisementBuilder, erro builder.Definition.Name, builder.Definition.Namespace, ) - var err error if !builder.Exists() { - err = builder.apiClient.Create(context.TODO(), builder.Definition) + err := builder.apiClient.Create(context.TODO(), builder.Definition) if err != nil { glog.V(100).Infof("Failed to create BGPAdvertisement") @@ -181,7 +180,7 @@ func (builder *BGPAdvertisementBuilder) Create() (*BGPAdvertisementBuilder, erro builder.Object = builder.Definition - return builder, err + return builder, nil } // Delete removes BGPAdvertisement object from a cluster. @@ -255,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 index 1115af198..00d9fa220 100644 --- 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() { @@ -184,16 +184,15 @@ func (builder *BGPPeerBuilder) Create() (*BGPPeerBuilder, error) { builder.Definition.Name, builder.Definition.Namespace, ) - var err error if !builder.Exists() { - err = builder.apiClient.Create(context.TODO(), builder.Definition) + err := builder.apiClient.Create(context.TODO(), builder.Definition) if err == nil { builder.Object = builder.Definition } } - return builder, err + return builder, nil } // Delete removes BGPPeer object from a cluster. diff --git a/pkg/metallb/l2advertisement.go b/pkg/metallb/l2advertisement.go index b362c74ff..f0c097f37 100644 --- 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/metallb/metallb.go b/pkg/metallb/metallb.go index 90bf8013e..1c69f6f6b 100644 --- a/pkg/metallb/metallb.go +++ b/pkg/metallb/metallb.go @@ -184,16 +184,15 @@ func (builder *Builder) Create() (*Builder, error) { builder.Definition.Name, builder.Definition.Namespace, ) - var err error if !builder.Exists() { - err = builder.apiClient.Create(context.TODO(), builder.Definition) + err := builder.apiClient.Create(context.TODO(), builder.Definition) if err == nil { builder.Object = builder.Definition } } - return builder, err + return builder, nil } // Delete removes MetalLb object from a cluster. diff --git a/pkg/nad/builder.go b/pkg/nad/builder.go index af93bfc77..e5aba7f74 100644 --- 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/namespace/namespace.go b/pkg/namespace/namespace.go index e5d9a7cab..e9bbd8d48 100644 --- a/pkg/namespace/namespace.go +++ b/pkg/namespace/namespace.go @@ -38,7 +38,7 @@ func NewBuilder(apiClient *clients.Settings, name string) *Builder { glog.V(100).Infof( "Initializing new namespace structure with the following param: %s", name) - builder := Builder{ + builder := &Builder{ apiClient: apiClient, Definition: &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ @@ -51,9 +51,11 @@ 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 + return builder } // WithLabel redefines namespace definition with the given label. @@ -194,7 +196,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. @@ -243,7 +245,7 @@ func (builder *Builder) Exists() bool { func Pull(apiClient *clients.Settings, nsname string) (*Builder, error) { glog.V(100).Infof("Pulling existing namespace: %s from cluster", nsname) - builder := Builder{ + builder := &Builder{ apiClient: apiClient, Definition: &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ @@ -253,7 +255,9 @@ func Pull(apiClient *clients.Settings, nsname string) (*Builder, error) { } if nsname == "" { - builder.errorMsg = "'namespace' cannot be empty" + glog.V(100).Infof("Namespace name is empty") + + return nil, fmt.Errorf("namespace name cannot be empty") } if !builder.Exists() { @@ -262,7 +266,7 @@ func Pull(apiClient *clients.Settings, nsname string) (*Builder, error) { builder.Definition = builder.Object - return &builder, nil + return builder, nil } // CleanObjects removes given objects from the namespace. @@ -370,13 +374,13 @@ func (builder *Builder) 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/network/operator.go b/pkg/network/operator.go index aef22db4b..4444dddec 100644 --- a/pkg/network/operator.go +++ b/pkg/network/operator.go @@ -238,13 +238,13 @@ func (builder *OperatorBuilder) 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/networkpolicy/multinetegressrule.go b/pkg/networkpolicy/multinetegressrule.go index 1b7d51fab..727378820 100644 --- a/pkg/networkpolicy/multinetegressrule.go +++ b/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 diff --git a/pkg/networkpolicy/multinetegressrule_test.go b/pkg/networkpolicy/multinetegressrule_test.go index ef3f90bd7..770600df5 100644 --- a/pkg/networkpolicy/multinetegressrule_test.go +++ b/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", diff --git a/pkg/networkpolicy/multinetingressrule.go b/pkg/networkpolicy/multinetingressrule.go index 630697364..e2ed66bca 100644 --- a/pkg/networkpolicy/multinetingressrule.go +++ b/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...) diff --git a/pkg/networkpolicy/multinetingressrule_test.go b/pkg/networkpolicy/multinetingressrule_test.go index d59f07af1..98419f05c 100644 --- 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 index 4bd673b26..0071cf0da 100644 --- a/pkg/networkpolicy/multinetworkpolicy.go +++ b/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. diff --git a/pkg/nfd/nodefeaturediscovery.go b/pkg/nfd/nodefeaturediscovery.go index 43504bd07..2cc923e6f 100644 --- a/pkg/nfd/nodefeaturediscovery.go +++ b/pkg/nfd/nodefeaturediscovery.go @@ -51,7 +51,7 @@ func NewBuilderFromObjectString(apiClient *clients.Settings, almExample string) glog.V(100).Infof( "Initializing Builder definition to NodeFeatureDiscovery object") - builder := Builder{ + builder := &Builder{ apiClient: apiClient, Definition: nodeFeatureDiscovery, } @@ -62,15 +62,20 @@ func NewBuilderFromObjectString(apiClient *clients.Settings, almExample string) builder.errorMsg = fmt.Sprintf("Error initializing NodeFeatureDiscovery from alm-examples: %s", err.Error()) + + return builder } 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 } - return &builder + return builder } // Pull loads an existing NodeFeatureDiscovery into Builder struct. @@ -143,7 +148,7 @@ func (builder *Builder) Get() (*nfdv1.NodeFeatureDiscovery, error) { return nil, err } - return nodeFeatureDiscovery, err + return nodeFeatureDiscovery, nil } // Exists checks whether the given NodeFeatureDiscovery exists. @@ -245,7 +250,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/nfd/nodefeaturediscovery_test.go b/pkg/nfd/nodefeaturediscovery_test.go index d27c69e21..3ba2908d2 100644 --- a/pkg/nfd/nodefeaturediscovery_test.go +++ b/pkg/nfd/nodefeaturediscovery_test.go @@ -48,12 +48,13 @@ func TestNFDNewBuilderFromObjectString(t *testing.T) { { almString: "", client: true, - expectedErrorText: "NodeFeatureDiscovery definition is nil", + expectedErrorText: "Error initializing NodeFeatureDiscovery from alm-examples: almExample is an empty string", }, { - almString: "{invalid}", - client: true, - expectedErrorText: "NodeFeatureDiscovery definition is nil", + almString: "{invalid}", + client: true, + //nolint:lll + expectedErrorText: "Error initializing NodeFeatureDiscovery from alm-examples: invalid character 'i' looking for beginning of object key string", }, { almString: almExample, diff --git a/pkg/nmstate/nmstate.go b/pkg/nmstate/nmstate.go index ee89277a6..305dd6f82 100644 --- a/pkg/nmstate/nmstate.go +++ b/pkg/nmstate/nmstate.go @@ -44,7 +44,7 @@ func NewBuilder(apiClient *clients.Settings, name string) *Builder { return nil } - builder := Builder{ + builder := &Builder{ apiClient: apiClient.Client, Definition: &nmstateV1.NMState{ ObjectMeta: metav1.ObjectMeta{ @@ -57,9 +57,11 @@ func NewBuilder(apiClient *clients.Settings, name string) *Builder { glog.V(100).Infof("The name of the NMState is empty") builder.errorMsg = "NMState 'name' cannot be empty" + + return builder } - return &builder + return builder } // Exists checks whether the given NMState exists. @@ -97,7 +99,7 @@ func (builder *Builder) Get() (*nmstateV1.NMState, error) { return nil, err } - return nmstate, err + return nmstate, nil } // Create makes a NMState in the cluster and stores the created object in struct. @@ -195,7 +197,7 @@ func PullNMstate(apiClient *clients.Settings, name string) (*Builder, error) { return nil, err } - builder := Builder{ + builder := &Builder{ apiClient: apiClient.Client, Definition: &nmstateV1.NMState{ ObjectMeta: metav1.ObjectMeta{ @@ -216,7 +218,7 @@ func PullNMstate(apiClient *clients.Settings, name string) (*Builder, error) { builder.Definition = builder.Object - return &builder, nil + return builder, nil } // validate will check that the builder and builder definition are properly initialized before @@ -233,13 +235,13 @@ func (builder *Builder) 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/nmstate/nodenetworkstate.go b/pkg/nmstate/nodenetworkstate.go index ac9137542..06f3124e5 100644 --- a/pkg/nmstate/nodenetworkstate.go +++ b/pkg/nmstate/nodenetworkstate.go @@ -231,13 +231,13 @@ func (builder *StateBuilder) validate() (bool, error) { if builder.Object == 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/nmstate/policy.go b/pkg/nmstate/policy.go index 9df3a4135..1cd2637de 100644 --- a/pkg/nmstate/policy.go +++ b/pkg/nmstate/policy.go @@ -63,7 +63,7 @@ func NewPolicyBuilder(apiClient *clients.Settings, name string, nodeSelector map return nil } - builder := PolicyBuilder{ + builder := &PolicyBuilder{ apiClient: apiClient.Client, Definition: &nmstateV1.NodeNetworkConfigurationPolicy{ ObjectMeta: metav1.ObjectMeta{ @@ -78,15 +78,19 @@ func NewPolicyBuilder(apiClient *clients.Settings, name string, nodeSelector map glog.V(100).Infof("The name of the NodeNetworkConfigurationPolicy is empty") builder.errorMsg = "nodeNetworkConfigurationPolicy 'name' cannot be empty" + + return builder } if len(nodeSelector) == 0 { glog.V(100).Infof("The nodeSelector of the NodeNetworkConfigurationPolicy is empty") builder.errorMsg = "nodeNetworkConfigurationPolicy 'nodeSelector' cannot be empty map" + + return builder } - return &builder + return builder } // Get returns NodeNetworkConfigurationPolicy object if found. @@ -110,7 +114,7 @@ func (builder *PolicyBuilder) Get() (*nmstateV1.NodeNetworkConfigurationPolicy, return nil, err } - return nmstatePolicy, err + return nmstatePolicy, nil } // Exists checks whether the given NodeNetworkConfigurationPolicy exists. @@ -260,15 +264,15 @@ func (builder *PolicyBuilder) WithBondInterface(slavePorts []string, bondName, m glog.V(100).Infof("error to add Bond mode %s, allowed modes are %v", mode, allowedBondModes) builder.errorMsg = "invalid Bond mode parameter" + + return builder } if bondName == "" { glog.V(100).Infof("The bondName can not be empty string") builder.errorMsg = "The bondName is empty sting" - } - if builder.errorMsg != "" { return builder } @@ -298,13 +302,13 @@ func (builder *PolicyBuilder) WithVlanInterface(baseInterface string, vlanID uin glog.V(100).Infof("The baseInterface can not be empty string") builder.errorMsg = "nodenetworkconfigurationpolicy 'baseInterface' cannot be empty" + + return builder } if vlanID > 4094 { builder.errorMsg = "invalid vlanID, allowed vlanID values are between 0-4094" - } - if builder.errorMsg != "" { return builder } @@ -405,9 +409,7 @@ func (builder *PolicyBuilder) WithAbsentInterface(interfaceName string) *PolicyB glog.V(100).Infof("The interfaceName can not be empty string") builder.errorMsg = "nodenetworkconfigurationpolicy 'interfaceName' cannot be empty" - } - if builder.errorMsg != "" { return builder } @@ -493,13 +495,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 != "" { diff --git a/pkg/nodes/node.go b/pkg/nodes/node.go index 0ca6cf620..b7a2a0363 100644 --- a/pkg/nodes/node.go +++ b/pkg/nodes/node.go @@ -225,9 +225,7 @@ func (builder *Builder) WithNewLabel(key, value string) *Builder { if key == "" { glog.V(100).Infof("Failed to apply label with an empty key to node %s", builder.Definition.Name) builder.errorMsg = "error to set empty key to node" - } - if builder.errorMsg != "" { return builder } @@ -239,6 +237,8 @@ func (builder *Builder) WithNewLabel(key, value string) *Builder { builder.Definition.Labels[key] = value } else { builder.errorMsg = fmt.Sprintf("cannot overwrite existing node label: %s", key) + + return builder } } @@ -281,9 +281,7 @@ func (builder *Builder) RemoveLabel(key, value string) *Builder { if key == "" { glog.V(100).Infof("Failed to remove empty label's key from node %s", builder.Definition.Name) builder.errorMsg = "error to remove empty key from node" - } - if builder.errorMsg != "" { return builder } @@ -413,13 +411,13 @@ func (builder *Builder) 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/nto/performanceprofile.go b/pkg/nto/performanceprofile.go index e267572c4..8d2fe69d8 100644 --- a/pkg/nto/performanceprofile.go +++ b/pkg/nto/performanceprofile.go @@ -118,7 +118,7 @@ func Pull(apiClient *clients.Settings, name string) (*Builder, error) { return nil, err } - builder := Builder{ + builder := &Builder{ apiClient: apiClient.Client, Definition: &v2.PerformanceProfile{ ObjectMeta: metav1.ObjectMeta{ @@ -139,7 +139,7 @@ func Pull(apiClient *clients.Settings, name string) (*Builder, error) { builder.Definition = builder.Object - return &builder, nil + return builder, nil } // WithHugePages defines the HugePages in the PerformanceProfile. hugePageSize allowed values are 2M, 1G. @@ -165,15 +165,15 @@ func (builder *Builder) WithHugePages(hugePageSize string, hugePages []v2.HugePa hugePageSize, allowedHugePageSize) builder.errorMsg = fmt.Sprintf("'hugePageSize' argument is not in allowed list: %v", allowedHugePageSize) + + return builder } if len(hugePages) == 0 { glog.V(100).Infof("'hugePages' argument cannot be empty") builder.errorMsg = "'hugePages' argument cannot be empty" - } - if builder.errorMsg != "" { return builder } @@ -207,9 +207,7 @@ func (builder *Builder) WithMachineConfigPoolSelector(machineConfigPoolSelector glog.V(100).Infof("'machineConfigPoolSelector' argument cannot be empty") builder.errorMsg = "'machineConfigPoolSelector' argument cannot be empty" - } - if builder.errorMsg != "" { return builder } @@ -264,9 +262,7 @@ func (builder *Builder) WithNumaTopology(topologyPolicy string) *Builder { builder.errorMsg = fmt.Sprintf("'allowedTopologyPolicies' argument is not in allowed list %v", allowedTopologyPolicies) - } - if builder.errorMsg != "" { return builder } @@ -537,13 +533,13 @@ func (builder *Builder) 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/nvidiagpu/clusterpolicy.go b/pkg/nvidiagpu/clusterpolicy.go index 2c25d793c..410abe309 100644 --- a/pkg/nvidiagpu/clusterpolicy.go +++ b/pkg/nvidiagpu/clusterpolicy.go @@ -62,7 +62,7 @@ func NewBuilderFromObjectString(apiClient *clients.Settings, almExample string) "Initializing new Builder structure from almExample string with clusterPolicy name: %s", clusterPolicy.Name) - builder := Builder{ + builder := &Builder{ apiClient: apiClient.Client, Definition: clusterPolicy, } @@ -71,9 +71,11 @@ func NewBuilderFromObjectString(apiClient *clients.Settings, almExample string) glog.V(100).Infof("The ClusterPolicy object definition is nil") builder.errorMsg = "ClusterPolicy 'Object.Definition' is nil" + + return builder } - return &builder + return builder } // Pull loads an existing clusterPolicy into Builder struct. @@ -247,13 +249,13 @@ func (builder *Builder) 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/ocm/placementbinding.go b/pkg/ocm/placementbinding.go index db98f3cf5..0323987bf 100644 --- a/pkg/ocm/placementbinding.go +++ b/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. @@ -97,7 +105,7 @@ func PullPlacementBinding(apiClient *clients.Settings, name, nsname string) (*Pl return nil, err } - builder := PlacementBindingBuilder{ + builder := &PlacementBindingBuilder{ apiClient: apiClient.Client, Definition: &policiesv1.PlacementBinding{ ObjectMeta: metav1.ObjectMeta{ @@ -125,7 +133,7 @@ func PullPlacementBinding(apiClient *clients.Settings, name, nsname string) (*Pl builder.Definition = builder.Object - return &builder, nil + return builder, nil } // Exists checks whether the given placementBinding exists. @@ -343,13 +351,13 @@ func (builder *PlacementBindingBuilder) 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/ocm/placementrule.go b/pkg/ocm/placementrule.go index 75fea155b..79e437e55 100644 --- a/pkg/ocm/placementrule.go +++ b/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. @@ -87,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{ @@ -115,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. @@ -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. @@ -263,13 +267,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 != "" { diff --git a/pkg/ocm/policy.go b/pkg/ocm/policy.go index 784d886f0..c64e8c056 100644 --- a/pkg/ocm/policy.go +++ b/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. @@ -426,13 +432,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 != "" { diff --git a/pkg/ocm/policyset.go b/pkg/ocm/policyset.go index b107de13d..97117a728 100644 --- a/pkg/ocm/policyset.go +++ b/pkg/ocm/policyset.go @@ -46,7 +46,7 @@ func NewPolicySetBuilder( return nil } - builder := PolicySetBuilder{ + builder := &PolicySetBuilder{ apiClient: apiClient.Client, Definition: &policiesv1beta1.PolicySet{ ObjectMeta: metav1.ObjectMeta{ @@ -63,21 +63,27 @@ func NewPolicySetBuilder( glog.V(100).Info("The name of the PolicySet is empty") builder.errorMsg = "policyset's 'name' cannot be empty" + + return builder } if nsname == "" { glog.V(100).Info("The namespace of the PolicySet is empty") builder.errorMsg = "policyset's 'nsname' cannot be empty" + + return builder } if policy == "" { glog.V(100).Info("The policy of the PolicySet is empty") builder.errorMsg = "policyset's 'policy' cannot be empty" + + return builder } - return &builder + return builder } // PullPolicySet pulls existing policySet into Builder struct. @@ -97,7 +103,7 @@ func PullPolicySet(apiClient *clients.Settings, name, nsname string) (*PolicySet return nil, err } - builder := PolicySetBuilder{ + builder := &PolicySetBuilder{ apiClient: apiClient.Client, Definition: &policiesv1beta1.PolicySet{ ObjectMeta: metav1.ObjectMeta{ @@ -125,7 +131,7 @@ func PullPolicySet(apiClient *clients.Settings, name, nsname string) (*PolicySet builder.Definition = builder.Object - return &builder, nil + return builder, nil } // Exists checks whether the given policySet exists. @@ -166,7 +172,7 @@ func (builder *PolicySetBuilder) Get() (*policiesv1beta1.PolicySet, error) { return nil, err } - return policySet, err + return policySet, nil } // Create makes a policySet in the cluster and stores the created object in struct. @@ -297,13 +303,13 @@ func (builder *PolicySetBuilder) 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/olm/catalogsource.go b/pkg/olm/catalogsource.go index c7209ff40..fa1a7b230 100644 --- a/pkg/olm/catalogsource.go +++ b/pkg/olm/catalogsource.go @@ -45,7 +45,7 @@ func NewCatalogSourceBuilder(apiClient *clients.Settings, name, nsname string) * return nil } - builder := CatalogSourceBuilder{ + builder := &CatalogSourceBuilder{ apiClient: apiClient.Client, Definition: &oplmV1alpha1.CatalogSource{ ObjectMeta: metav1.ObjectMeta{ @@ -59,15 +59,19 @@ func NewCatalogSourceBuilder(apiClient *clients.Settings, name, nsname string) * glog.V(100).Infof("The name of the catalogsource is empty") builder.errorMsg = "catalogsource 'name' cannot be empty" + + return builder } if nsname == "" { glog.V(100).Infof("The nsname of the catalogsource is empty") builder.errorMsg = "catalogsource 'nsname' cannot be empty" + + return builder } - return &builder + return builder } // PullCatalogSource loads an existing catalogsource into Builder struct. @@ -88,7 +92,7 @@ func PullCatalogSource(apiClient *clients.Settings, name, nsname string) (*Catal return nil, err } - builder := CatalogSourceBuilder{ + builder := &CatalogSourceBuilder{ apiClient: apiClient.Client, Definition: &oplmV1alpha1.CatalogSource{ ObjectMeta: metav1.ObjectMeta{ @@ -116,7 +120,7 @@ func PullCatalogSource(apiClient *clients.Settings, name, nsname string) (*Catal builder.Definition = builder.Object - return &builder, nil + return builder, nil } // Create makes an CatalogSourceBuilder in cluster and stores the created object in struct. @@ -265,13 +269,13 @@ func (builder *CatalogSourceBuilder) 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/olm/clusterserviceversion.go b/pkg/olm/clusterserviceversion.go index c6cc2b9ef..f33d64132 100644 --- 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. @@ -220,13 +220,13 @@ func (builder *ClusterServiceVersionBuilder) 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/olm/installplan.go b/pkg/olm/installplan.go index f4552664b..e7dac1fb5 100644 --- a/pkg/olm/installplan.go +++ b/pkg/olm/installplan.go @@ -45,7 +45,7 @@ func NewInstallPlanBuilder(apiClient *clients.Settings, name, nsname string) *In return nil } - builder := InstallPlanBuilder{ + builder := &InstallPlanBuilder{ apiClient: apiClient.Client, Definition: &operatorsV1alpha1.InstallPlan{ ObjectMeta: metav1.ObjectMeta{ @@ -59,15 +59,19 @@ func NewInstallPlanBuilder(apiClient *clients.Settings, name, nsname string) *In glog.V(100).Infof("The name of the installplan is empty") builder.errorMsg = "installplan 'name' cannot be empty" + + return builder } if nsname == "" { glog.V(100).Infof("The nsname of the installplan is empty") builder.errorMsg = "installplan 'nsname' cannot be empty" + + return builder } - return &builder + return builder } // PullInstallPlan loads existing InstallPlan from cluster into the InstallPlanBuilder struct. @@ -251,13 +255,13 @@ func (builder *InstallPlanBuilder) 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 builder %s 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/olm/operatorgroup.go b/pkg/olm/operatorgroup.go index ca528e652..4c6475842 100644 --- a/pkg/olm/operatorgroup.go +++ b/pkg/olm/operatorgroup.go @@ -64,12 +64,16 @@ func NewOperatorGroupBuilder(apiClient *clients.Settings, groupName, nsName stri glog.V(100).Infof("The Name of the OperatorGroup is empty") builder.errorMsg = "operatorGroup 'groupName' cannot be empty" + + return builder } if nsName == "" { glog.V(100).Infof("The Namespace of the OperatorGroup is empty") builder.errorMsg = "operatorGroup 'Namespace' cannot be empty" + + return builder } return builder @@ -255,13 +259,13 @@ func (builder *OperatorGroupBuilder) 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/olm/packagemanifest.go b/pkg/olm/packagemanifest.go index 2d48c492c..5c96f31b6 100644 --- a/pkg/olm/packagemanifest.go +++ b/pkg/olm/packagemanifest.go @@ -224,13 +224,13 @@ func (builder *PackageManifestBuilder) 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/olm/subscription.go b/pkg/olm/subscription.go index 440470092..c4023df0b 100644 --- a/pkg/olm/subscription.go +++ b/pkg/olm/subscription.go @@ -66,30 +66,40 @@ func NewSubscriptionBuilder(apiClient *clients.Settings, subName, subNamespace, glog.V(100).Infof("The Name of the Subscription is empty") builder.errorMsg = "subscription 'subName' cannot be empty" + + return builder } if subNamespace == "" { glog.V(100).Infof("The Namespace of the Subscription is empty") builder.errorMsg = "subscription 'subNamespace' cannot be empty" + + return builder } if catalogSource == "" { glog.V(100).Infof("The Catalogsource of the Subscription is empty") builder.errorMsg = "subscription 'catalogSource' cannot be empty" + + return builder } if catalogSourceNamespace == "" { glog.V(100).Infof("The Catalogsource namespace of the Subscription is empty") builder.errorMsg = "subscription 'catalogSourceNamespace' cannot be empty" + + return builder } if packageName == "" { glog.V(100).Infof("The Package name of the Subscription is empty") builder.errorMsg = "subscription 'packageName' cannot be empty" + + return builder } return builder @@ -105,9 +115,7 @@ func (builder *SubscriptionBuilder) WithChannel(channel string) *SubscriptionBui if channel == "" { builder.errorMsg = "can not redefine subscription with empty channel" - } - if builder.errorMsg != "" { return builder } @@ -127,9 +135,7 @@ func (builder *SubscriptionBuilder) WithStartingCSV(startingCSV string) *Subscri if startingCSV == "" { builder.errorMsg = "can not redefine subscription with empty startingCSV" - } - if builder.errorMsg != "" { return builder } @@ -153,9 +159,7 @@ func (builder *SubscriptionBuilder) WithInstallPlanApproval( "or \"Manual\"") builder.errorMsg = "Subscription 'installPlanApproval' must be either \"Automatic\" or \"Manual\"" - } - if builder.errorMsg != "" { return builder } @@ -347,13 +351,13 @@ func (builder *SubscriptionBuilder) 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/pod/container.go b/pkg/pod/container.go index b6baff12b..bf47e5dea 100644 --- a/pkg/pod/container.go +++ b/pkg/pod/container.go @@ -57,18 +57,24 @@ func NewContainerBuilder(name, image string, cmd []string) *ContainerBuilder { glog.V(100).Infof("The name of the container is empty") builder.errorMsg = "container's name is empty" + + return builder } if image == "" { glog.V(100).Infof("Container's image is empty") builder.errorMsg = "container's image is empty" + + return builder } if len(cmd) < 1 { glog.V(100).Infof("Container's cmd is empty") builder.errorMsg = "container's cmd is empty" + + return builder } return builder @@ -84,6 +90,8 @@ func (builder *ContainerBuilder) WithSecurityCapabilities(sCapabilities []string glog.V(100).Infof("Cannot modify pre-existing SecurityContext") builder.errorMsg = "can not modify pre-existing security context" + + return builder } builder.definition.SecurityContext = nil @@ -94,9 +102,7 @@ func (builder *ContainerBuilder) WithSecurityCapabilities(sCapabilities []string sCapabilities, AllowedSCList) builder.errorMsg = "one of the give securityCapabilities is invalid. Please extend allowed list or fix parameter" - } - if builder.errorMsg != "" { return builder } @@ -198,9 +204,7 @@ func (builder *ContainerBuilder) WithSecurityContext(securityContext *corev1.Sec glog.V(100).Infof("Cannot add empty securityContext to container structure") builder.errorMsg = "can not modify container config with empty securityContext" - } - if builder.errorMsg != "" { return builder } @@ -218,21 +222,23 @@ func (builder *ContainerBuilder) WithResourceLimit(hugePages, memory string, cpu glog.V(100).Infof("Container's resource limit hugePages is empty") builder.errorMsg = "container's resource limit 'hugePages' is empty" + + return builder } if memory == "" { glog.V(100).Infof("Container's resource limit memory is empty") builder.errorMsg = "container's resource limit 'memory' is empty" + + return builder } if cpu <= 0 { glog.V(100).Infof("Container's resource limit cpu can not be zero or negative number.") builder.errorMsg = "container's resource limit 'cpu' is invalid" - } - if builder.errorMsg != "" { return builder } @@ -254,21 +260,23 @@ func (builder *ContainerBuilder) WithResourceRequest(hugePages, memory string, c glog.V(100).Infof("Container's resource request hugePages is empty") builder.errorMsg = "container's resource request 'hugePages' is empty" + + return builder } if memory == "" { glog.V(100).Infof("Container's resource request memory is empty") builder.errorMsg = "container's resource request 'memory' is empty" + + return builder } if cpu <= 0 { glog.V(100).Infof("Container's resource request cpu can not be zero or negative number.") builder.errorMsg = "container's resource request 'cpu' is invalid" - } - if builder.errorMsg != "" { return builder } @@ -306,9 +314,7 @@ func (builder *ContainerBuilder) WithCustomResourcesLimits(resourceList corev1.R glog.V(100).Infof("Container's resource limit var 'resourceList' is empty") builder.errorMsg = "container's resource limit var 'resourceList' is empty" - } - if builder.errorMsg != "" { return builder } @@ -325,9 +331,7 @@ func (builder *ContainerBuilder) WithImagePullPolicy(pullPolicy corev1.PullPolic glog.V(100).Infof("Container's image pull policy 'pullPolicy' is empty") builder.errorMsg = "container's pull policy var 'pullPolicy' is empty" - } - if builder.errorMsg != "" { return builder } @@ -344,15 +348,15 @@ func (builder *ContainerBuilder) WithEnvVar(name, value string) *ContainerBuilde glog.V(100).Infof("Container's environment var 'name' is empty") builder.errorMsg = "container's environment var 'name' is empty" + + return builder } if value == "" { glog.V(100).Infof("Container's environment var 'value' is empty") builder.errorMsg = "container's environment var 'value' is empty" - } - if builder.errorMsg != name { return builder } @@ -375,15 +379,15 @@ func (builder *ContainerBuilder) WithVolumeMount(volMount corev1.VolumeMount) *C glog.V(100).Infof("Container's VolumeMount name cannot be empty") builder.errorMsg = "container's volume mount name is empty" + + return builder } if volMount.MountPath == "" { glog.V(100).Infof("Container's VolumeMount mount path cannot be empty") builder.errorMsg = "container's volume mount path is empty" - } - if builder.errorMsg != "" { return builder } diff --git a/pkg/pod/container_test.go b/pkg/pod/container_test.go index f3b4894cb..8cd5285ca 100644 --- a/pkg/pod/container_test.go +++ b/pkg/pod/container_test.go @@ -360,7 +360,7 @@ func TestPodContainerWithVolumeMount(t *testing.T) { }, { mount: corev1.VolumeMount{}, - expectedError: "container's volume mount path is empty", + expectedError: "container's volume mount name is empty", }, } @@ -511,7 +511,7 @@ func TestPodContainerGetContainerCfg(t *testing.T) { { builder: NewContainerBuilder("container", "test", []string{"/bin/bash", "-c", "sleep"}). WithEnvVar("", ""), - expectedError: fmt.Errorf("container's environment var 'value' is empty"), + expectedError: fmt.Errorf("container's environment var 'name' is empty"), }, } diff --git a/pkg/pod/pod.go b/pkg/pod/pod.go index 1f18a1aa8..bcaea0ac4 100644 --- a/pkg/pod/pod.go +++ b/pkg/pod/pod.go @@ -110,7 +110,7 @@ func Pull(apiClient *clients.Settings, name, nsname string) (*Builder, error) { return nil, fmt.Errorf("pod 'apiClient' cannot be empty") } - builder := Builder{ + builder := &Builder{ apiClient: apiClient, Definition: &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ @@ -141,7 +141,7 @@ func Pull(apiClient *clients.Settings, name, nsname string) (*Builder, error) { builder.Definition = builder.Object - return &builder, nil + return builder, nil } // DefineOnNode adds nodeName to the pod's definition. @@ -159,12 +159,12 @@ func (builder *Builder) DefineOnNode(nodeName string) *Builder { glog.V(100).Infof("The node name is empty") builder.errorMsg = "can not define pod on empty node" - } - if builder.errorMsg == "" { - builder.Definition.Spec.NodeName = nodeName + return builder } + builder.Definition.Spec.NodeName = nodeName + return builder } @@ -679,9 +679,7 @@ func (builder *Builder) WithRestartPolicy(restartPolicy corev1.RestartPolicy) *B builder.Definition.Name, builder.Definition.Namespace) builder.errorMsg = "can not define pod with empty restart policy" - } - if builder.errorMsg != "" { return builder } @@ -774,9 +772,7 @@ func (builder *Builder) WithNodeSelector(nodeSelector map[string]string) *Builde builder.Definition.Name, builder.Definition.Namespace) builder.errorMsg = "can not define pod with empty nodeSelector" - } - if builder.errorMsg != "" { return builder } @@ -818,9 +814,7 @@ func (builder *Builder) WithVolume(volume corev1.Volume) *Builder { glog.V(100).Infof("The volume's Name cannot be empty") builder.errorMsg = "the volume's name cannot be empty" - } - if builder.errorMsg != "" { return builder } @@ -847,12 +841,16 @@ func (builder *Builder) WithLocalVolume(volumeName, mountPath string) *Builder { glog.V(100).Infof("The 'volumeName' of the pod is empty") builder.errorMsg = "'volumeName' parameter is empty" + + return builder } if mountPath == "" { glog.V(100).Infof("The 'mountPath' of the pod is empty") builder.errorMsg = "'mountPath' parameter is empty" + + return builder } mountConfig := corev1.VolumeMount{Name: volumeName, MountPath: mountPath, ReadOnly: false} @@ -923,6 +921,8 @@ func (builder *Builder) WithAdditionalInitContainer(container *corev1.Container) glog.V(100).Infof("The 'container' parameter of the pod is empty") builder.errorMsg = "'container' parameter cannot be empty" + + return builder } if builder.errorMsg != "" { @@ -952,9 +952,7 @@ func (builder *Builder) WithSecondaryNetwork(network []*multus.NetworkSelectionE if err != nil { builder.errorMsg = fmt.Sprintf("error to unmarshal network annotation due to: %s", err.Error()) - } - if builder.errorMsg != "" { return builder } @@ -1070,14 +1068,16 @@ func (builder *Builder) WithSecurityContext(securityContext *corev1.PodSecurityC glog.V(100).Infof("The 'securityContext' of the pod is empty") builder.errorMsg = "'securityContext' parameter is empty" - } - if builder.errorMsg != "" { return builder } builder.isMutationAllowed("SecurityContext") + if builder.errorMsg != "" { + return builder + } + builder.Definition.Spec.SecurityContext = securityContext return builder @@ -1145,9 +1145,7 @@ func (builder *Builder) WithLabel(labelKey, labelValue string) *Builder { if labelKey == "" { builder.errorMsg = "can not apply empty labelKey" - } - if builder.errorMsg != "" { return builder } @@ -1348,13 +1346,13 @@ func (builder *Builder) 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/rbac/clusterrole.go b/pkg/rbac/clusterrole.go index f2fa4e905..58fd548c7 100644 --- a/pkg/rbac/clusterrole.go +++ b/pkg/rbac/clusterrole.go @@ -38,7 +38,7 @@ func NewClusterRoleBuilder(apiClient *clients.Settings, name string, rule rbacv1 "name: %s, policy rule: %v", name, rule) - builder := ClusterRoleBuilder{ + builder := &ClusterRoleBuilder{ apiClient: apiClient, Definition: &rbacv1.ClusterRole{ ObjectMeta: metav1.ObjectMeta{ @@ -51,11 +51,13 @@ func NewClusterRoleBuilder(apiClient *clients.Settings, name string, rule rbacv1 glog.V(100).Infof("The name of the clusterrole is empty") builder.errorMsg = "clusterrole 'name' cannot be empty" + + return builder } builder.WithRules([]rbacv1.PolicyRule{rule}) - return &builder + return builder } // WithRules appends additional rules to the clusterrole definition. @@ -71,9 +73,7 @@ func (builder *ClusterRoleBuilder) WithRules(rules []rbacv1.PolicyRule) *Cluster glog.V(100).Infof("The list of rules is empty") builder.errorMsg = "cannot accept nil or empty slice as rules" - } - if builder.errorMsg != "" { return builder } @@ -82,9 +82,7 @@ func (builder *ClusterRoleBuilder) WithRules(rules []rbacv1.PolicyRule) *Cluster glog.V(100).Infof("The clusterrole rule must contain at least one Verb entry") builder.errorMsg = "clusterrole rule must contain at least one Verb entry" - } - if builder.errorMsg != "" { return builder } } @@ -135,7 +133,7 @@ func PullClusterRole(apiClient *clients.Settings, name string) (*ClusterRoleBuil return nil, fmt.Errorf("the apiClient cannot be nil") } - builder := ClusterRoleBuilder{ + builder := &ClusterRoleBuilder{ apiClient: apiClient, Definition: &rbacv1.ClusterRole{ ObjectMeta: metav1.ObjectMeta{ @@ -156,7 +154,7 @@ func PullClusterRole(apiClient *clients.Settings, name string) (*ClusterRoleBuil builder.Definition = builder.Object - return &builder, nil + return builder, nil } // Create generates a clusterrole in the cluster and stores the created object in struct. @@ -256,13 +254,13 @@ func (builder *ClusterRoleBuilder) 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/rbac/clusterrole_test.go b/pkg/rbac/clusterrole_test.go index c16bca117..8cc09261e 100644 --- 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: "", diff --git a/pkg/rbac/clusterrolebinding.go b/pkg/rbac/clusterrolebinding.go index ac7007cfc..bb8beba92 100644 --- a/pkg/rbac/clusterrolebinding.go +++ b/pkg/rbac/clusterrolebinding.go @@ -37,7 +37,7 @@ func NewClusterRoleBindingBuilder( "name: %s, clusterrole: %s, subject %v", name, clusterRole, subject) - builder := ClusterRoleBindingBuilder{ + builder := &ClusterRoleBindingBuilder{ apiClient: apiClient, Definition: &rbacv1.ClusterRoleBinding{ ObjectMeta: metav1.ObjectMeta{ @@ -57,9 +57,11 @@ func NewClusterRoleBindingBuilder( glog.V(100).Infof("The name of the clusterrolebinding is empty") builder.errorMsg = "clusterrolebinding 'name' cannot be empty" + + return builder } - return &builder + return builder } // WithSubjects appends additional subjects to clusterrolebinding definition. @@ -75,9 +77,7 @@ func (builder *ClusterRoleBindingBuilder) WithSubjects(subjects []rbacv1.Subject glog.V(100).Infof("The list of subjects is empty") builder.errorMsg = "cannot accept nil or empty slice as subjects" - } - if builder.errorMsg != "" { return builder } @@ -86,15 +86,15 @@ func (builder *ClusterRoleBindingBuilder) WithSubjects(subjects []rbacv1.Subject glog.V(100).Infof("The clusterrolebinding subject kind must be one of 'ServiceAccount', 'User', or 'Group'") builder.errorMsg = "clusterrolebinding subject kind must be one of 'ServiceAccount', 'User', or 'Group'" + + return builder } if subject.Name == "" { glog.V(100).Infof("The clusterrolebinding subject name cannot be empty") builder.errorMsg = "clusterrolebinding subject name cannot be empty" - } - if builder.errorMsg != "" { return builder } } @@ -134,7 +134,7 @@ func (builder *ClusterRoleBindingBuilder) WithOptions( func PullClusterRoleBinding(apiClient *clients.Settings, name string) (*ClusterRoleBindingBuilder, error) { glog.V(100).Infof("Pulling existing clusterrolebinding name %s from cluster", name) - builder := ClusterRoleBindingBuilder{ + builder := &ClusterRoleBindingBuilder{ apiClient: apiClient, Definition: &rbacv1.ClusterRoleBinding{ ObjectMeta: metav1.ObjectMeta{ @@ -146,7 +146,7 @@ func PullClusterRoleBinding(apiClient *clients.Settings, name string) (*ClusterR if name == "" { glog.V(100).Infof("The name of the clusterrolebinding is empty") - builder.errorMsg = "clusterrolebinding 'name' cannot be empty" + return nil, fmt.Errorf("clusterrolebinding 'name' cannot be empty") } if !builder.Exists() { @@ -155,7 +155,7 @@ func PullClusterRoleBinding(apiClient *clients.Settings, name string) (*ClusterR builder.Definition = builder.Object - return &builder, nil + return builder, nil } // Create generates a clusterrolebinding in the cluster and stores the created object in struct. @@ -203,7 +203,7 @@ func (builder *ClusterRoleBindingBuilder) Delete() error { builder.Object = nil - return err + return nil } // Update modifies a clusterrolebinding object in the cluster. @@ -252,13 +252,13 @@ func (builder *ClusterRoleBindingBuilder) 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/rbac/role.go b/pkg/rbac/role.go index 9648051e8..6307cec7c 100644 --- a/pkg/rbac/role.go +++ b/pkg/rbac/role.go @@ -78,9 +78,7 @@ func (builder *RoleBuilder) WithRules(rules []rbacv1.PolicyRule) *RoleBuilder { glog.V(100).Infof("The list of rules is empty") builder.errorMsg = "cannot create role with empty rule" - } - if builder.errorMsg != "" { return builder } @@ -89,21 +87,23 @@ func (builder *RoleBuilder) WithRules(rules []rbacv1.PolicyRule) *RoleBuilder { glog.V(100).Infof("The role has no verbs") builder.errorMsg = "role must contain at least one Verb" + + return builder } if len(rule.Resources) == 0 { glog.V(100).Infof("The role has no resources") builder.errorMsg = "role must contain at least one Resource" + + return builder } if len(rule.APIGroups) == 0 { glog.V(100).Infof("The role has no apigroups") builder.errorMsg = "role must contain at least one APIGroup" - } - if builder.errorMsg != "" { return builder } } @@ -229,7 +229,7 @@ func (builder *RoleBuilder) Delete() error { builder.Object = nil - return err + return nil } // Update modifies the existing Role object with role definition in builder. @@ -282,13 +282,13 @@ func (builder *RoleBuilder) 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/rbac/role_test.go b/pkg/rbac/role_test.go index ce1f8d99e..f0fdf15b6 100644 --- a/pkg/rbac/role_test.go +++ b/pkg/rbac/role_test.go @@ -39,7 +39,7 @@ func TestNewRoleBuilder(t *testing.T) { rule: rbacv1.PolicyRule{ Resources: []string{"pods"}, APIGroups: []string{"v1"}, Verbs: []string{"get"}}, client: false, - expectedErrorText: "role builder cannot have nil apiClient", + expectedErrorText: "", }, { name: "", @@ -190,7 +190,7 @@ func TestRoleCreate(t *testing.T) { }, { testRole: buildInvalidRoleTestBuilder(buildTestClientWithDummyObject()), - expectedError: fmt.Errorf("role must contain at least one APIGroup"), + expectedError: fmt.Errorf("role must contain at least one Verb"), }, { testRole: buildValidRoleBuilder(clients.GetTestClients(clients.TestClientParams{})), @@ -244,7 +244,7 @@ func TestRoleDelete(t *testing.T) { }, { testRole: buildInvalidRoleTestBuilder(buildTestClientWithDummyObject()), - expectedError: fmt.Errorf("role must contain at least one APIGroup"), + expectedError: fmt.Errorf("role must contain at least one Verb"), }, { testRole: buildValidRoleBuilder(clients.GetTestClients(clients.TestClientParams{})), @@ -277,7 +277,7 @@ func TestRoleUpdate(t *testing.T) { }, { testRole: buildInvalidRoleTestBuilder(buildTestClientWithDummyObject()), - expectedError: fmt.Errorf("role must contain at least one APIGroup"), + expectedError: fmt.Errorf("role must contain at least one Verb"), }, { testRole: buildValidRoleBuilder(clients.GetTestClients(clients.TestClientParams{})), diff --git a/pkg/rbac/rolebinding.go b/pkg/rbac/rolebinding.go index e977ba23b..d2d38211a 100644 --- a/pkg/rbac/rolebinding.go +++ b/pkg/rbac/rolebinding.go @@ -38,7 +38,7 @@ func NewRoleBindingBuilder(apiClient *clients.Settings, "Initializing new rolebinding structure with the following params: "+ "name: %s, namespace: %s, role: %s, subject %v", name, nsname, role, subject) - builder := RoleBindingBuilder{ + builder := &RoleBindingBuilder{ apiClient: apiClient, Definition: &rbacv1.RoleBinding{ ObjectMeta: metav1.ObjectMeta{ @@ -57,17 +57,21 @@ func NewRoleBindingBuilder(apiClient *clients.Settings, glog.V(100).Infof("The name of the rolebinding is empty") builder.errorMsg = "RoleBinding 'name' cannot be empty" + + return builder } if nsname == "" { glog.V(100).Infof("The namespace of the rolebinding is empty") builder.errorMsg = "RoleBinding 'nsname' cannot be empty" + + return builder } builder.WithSubjects([]rbacv1.Subject{subject}) - return &builder + return builder } // WithSubjects adds specified Subject to the RoleBinding. @@ -83,9 +87,7 @@ func (builder *RoleBindingBuilder) WithSubjects(subjects []rbacv1.Subject) *Role glog.V(100).Infof("The list of subjects is empty") builder.errorMsg = "cannot create rolebinding with empty subject" - } - if builder.errorMsg != "" { return builder } @@ -94,15 +96,15 @@ func (builder *RoleBindingBuilder) WithSubjects(subjects []rbacv1.Subject) *Role glog.V(100).Infof("The rolebinding subject kind must be one of 'ServiceAccount', 'User', or 'Group'") builder.errorMsg = "rolebinding subject kind must be one of 'ServiceAccount', 'User', 'Group'" + + return builder } if subject.Name == "" { glog.V(100).Infof("The rolebinding subject name cannot be empty") builder.errorMsg = "rolebinding subject name cannot be empty" - } - if builder.errorMsg != "" { return builder } } @@ -140,7 +142,7 @@ func (builder *RoleBindingBuilder) WithOptions(options ...RoleBindingAdditionalO func PullRoleBinding(apiClient *clients.Settings, name, nsname string) (*RoleBindingBuilder, error) { glog.V(100).Infof("Pulling existing rolebinding name %s under namespace %s from cluster", name, nsname) - builder := RoleBindingBuilder{ + builder := &RoleBindingBuilder{ apiClient: apiClient, Definition: &rbacv1.RoleBinding{ ObjectMeta: metav1.ObjectMeta{ @@ -153,13 +155,13 @@ func PullRoleBinding(apiClient *clients.Settings, name, nsname string) (*RoleBin if name == "" { glog.V(100).Infof("The name of the rolebinding is empty") - builder.errorMsg = "rolebinding 'name' cannot be empty" + return nil, fmt.Errorf("rolebinding 'name' cannot be empty") } if nsname == "" { glog.V(100).Infof("The namespace of the rolebinding is empty") - builder.errorMsg = "rolebinding 'namespace' cannot be empty" + return nil, fmt.Errorf("rolebinding 'namespace' cannot be empty") } if !builder.Exists() { @@ -168,7 +170,7 @@ func PullRoleBinding(apiClient *clients.Settings, name, nsname string) (*RoleBin builder.Definition = builder.Object - return &builder, nil + return builder, nil } // Create generates a RoleBinding and stores the created object in struct. @@ -210,9 +212,13 @@ func (builder *RoleBindingBuilder) Delete() error { err := builder.apiClient.RoleBindings(builder.Definition.Namespace).Delete( context.TODO(), builder.Definition.Name, metav1.DeleteOptions{}) + if err != nil { + return err + } + builder.Object = nil - return err + return nil } // Update modifies an existing RoleBinding in the cluster. @@ -261,13 +267,13 @@ func (builder *RoleBindingBuilder) 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/route/route.go b/pkg/route/route.go index 25e507181..46f9fab90 100644 --- a/pkg/route/route.go +++ b/pkg/route/route.go @@ -40,7 +40,7 @@ func NewBuilder(apiClient *clients.Settings, name, nsname, serviceName string) * return nil } - builder := Builder{ + builder := &Builder{ apiClient: apiClient.Client, Definition: &routev1.Route{ ObjectMeta: metav1.ObjectMeta{ @@ -60,21 +60,27 @@ func NewBuilder(apiClient *clients.Settings, name, nsname, serviceName string) * glog.V(100).Infof("The name of the route is empty") builder.errorMsg = "route 'name' cannot be empty" + + return builder } if nsname == "" { glog.V(100).Infof("The namespace of the route is empty") builder.errorMsg = "route 'nsname' cannot be empty" + + return builder } if serviceName == "" { glog.V(100).Infof("The serviceName of the route is empty") builder.errorMsg = "route 'serviceName' cannot be empty" + + return builder } - return &builder + return builder } // Pull loads existing route from cluster. @@ -87,7 +93,7 @@ func Pull(apiClient *clients.Settings, name, nsname string) (*Builder, error) { return nil, fmt.Errorf("the apiClient cannot be nil") } - builder := Builder{ + builder := &Builder{ apiClient: apiClient.Client, Definition: &routev1.Route{ ObjectMeta: metav1.ObjectMeta{ @@ -115,7 +121,7 @@ func Pull(apiClient *clients.Settings, name, nsname string) (*Builder, error) { builder.Definition = builder.Object - return &builder, nil + return builder, nil } // WithTargetPortNumber adds a target port to the route by number. @@ -149,9 +155,7 @@ func (builder *Builder) WithTargetPortName(portName string) *Builder { glog.V(100).Infof("Received empty route portName") builder.errorMsg = "route target port name cannot be empty string" - } - if builder.errorMsg != "" { return builder } @@ -202,9 +206,7 @@ func (builder *Builder) WithWildCardPolicy(wildcardPolicy string) *Builder { builder.errorMsg = fmt.Sprintf("received unsupported route wildcardPolicy: supported policies %v", supportedWildCardPolicies()) - } - if builder.errorMsg != "" { return builder } @@ -250,7 +252,7 @@ func (builder *Builder) Get() (*routev1.Route, error) { return nil, err } - return route, err + return route, nil } // Create makes a route according to the route definition and stores the created object in the route builder. @@ -316,13 +318,13 @@ func (builder *Builder) 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/scc/scc.go b/pkg/scc/scc.go index 259d9cedf..93fa6c672 100644 --- a/pkg/scc/scc.go +++ b/pkg/scc/scc.go @@ -570,6 +570,10 @@ func (builder *Builder) Delete() error { return err } + if err != nil { + return err + } + builder.Object = nil return nil @@ -644,13 +648,13 @@ func (builder *Builder) 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/secret/secret.go b/pkg/secret/secret.go index 9e867f57e..3088bfc84 100644 --- a/pkg/secret/secret.go +++ b/pkg/secret/secret.go @@ -88,7 +88,7 @@ func Pull(apiClient *clients.Settings, name, nsname string) (*Builder, error) { return nil, fmt.Errorf("secret 'apiClient' cannot be empty") } - builder := Builder{ + builder := &Builder{ apiClient: apiClient, Definition: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ @@ -116,7 +116,7 @@ func Pull(apiClient *clients.Settings, name, nsname string) (*Builder, error) { builder.Definition = builder.Object - return &builder, nil + return builder, nil } // Create makes a secret in the cluster and stores the created object in struct. @@ -212,9 +212,7 @@ func (builder *Builder) WithData(data map[string][]byte) *Builder { glog.V(100).Infof("The data of the secret is empty") builder.errorMsg = "'data' cannot be empty" - } - if builder.errorMsg != "" { return builder } diff --git a/pkg/service/service.go b/pkg/service/service.go index 0021c8790..2f1c2964d 100644 --- a/pkg/service/service.go +++ b/pkg/service/service.go @@ -43,7 +43,7 @@ func NewBuilder( glog.V(100).Infof( "Initializing new service structure with the following params: %s, %s", name, nsname) - builder := Builder{ + builder := &Builder{ apiClient: apiClient.CoreV1Interface, Definition: &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ @@ -62,7 +62,7 @@ func NewBuilder( builder.errorMsg = "Service 'name' cannot be empty" - return &builder + return builder } if nsname == "" { @@ -70,10 +70,10 @@ func NewBuilder( builder.errorMsg = "Service 'nsname' cannot be empty" - return &builder + return builder } - return &builder + return builder } // WithNodePort redefines the service with NodePort service type. @@ -105,7 +105,7 @@ func Pull(apiClient *clients.Settings, name, nsname string) (*Builder, error) { return nil, fmt.Errorf("service 'apiClient' cannot be empty") } - builder := Builder{ + builder := &Builder{ apiClient: apiClient.CoreV1Interface, Definition: &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ @@ -129,7 +129,7 @@ func Pull(apiClient *clients.Settings, name, nsname string) (*Builder, error) { builder.Definition = builder.Object - return &builder, nil + return builder, nil } // Create the service in the cluster and store the created object in Object. diff --git a/pkg/servicemesh/controlplane.go b/pkg/servicemesh/controlplane.go index 20b86246f..8dd445079 100644 --- a/pkg/servicemesh/controlplane.go +++ b/pkg/servicemesh/controlplane.go @@ -59,12 +59,16 @@ func NewControlPlaneBuilder(apiClient *clients.Settings, name, nsname string) *C glog.V(100).Infof("The name of the serviceMeshControlPlane is empty") builder.errorMsg = "serviceMeshControlPlane 'name' cannot be empty" + + return builder } if nsname == "" { glog.V(100).Infof("The namespace of the serviceMeshControlPlane is empty") builder.errorMsg = "serviceMeshControlPlane 'nsname' cannot be empty" + + return builder } return builder @@ -121,17 +125,17 @@ func (builder *ControlPlaneBuilder) WithGrafanaAddon( glog.V(100).Infof("The grafanaInstallConfig of the Grafana addon is empty") builder.errorMsg = "the Grafana addon 'grafanaInstallConfig' cannot be empty when Grafana addon is enabled" + + return builder } if address == "" { glog.V(100).Infof("The address of the Grafana addon is empty") builder.errorMsg = "the Grafana addon 'address' cannot be empty when Grafana addon is enabled" - } - } - if builder.errorMsg != "" { - return builder + return builder + } } addonConfig := &istiov2.GrafanaAddonConfig{ @@ -167,15 +171,15 @@ func (builder *ControlPlaneBuilder) WithJaegerAddon( glog.V(100).Infof("The name of the Jaeger addon is empty") builder.errorMsg = "the Jaeger addon 'name' cannot be empty" + + return builder } if jaegerInstallConfig == nil { glog.V(100).Infof("The jaegerInstallConfig of the Jaeger addon is empty") builder.errorMsg = "the Jaeger addon 'jaegerInstallConfig' cannot be empty" - } - if builder.errorMsg != "" { return builder } @@ -211,17 +215,17 @@ func (builder *ControlPlaneBuilder) WithKialiAddon( glog.V(100).Infof("The kialiInstallConfig of the Kiali addon is empty") builder.errorMsg = "the Kiali addon 'kialiInstallConfig' cannot be empty when Kiali addon is enabled" + + return builder } if name == "" { glog.V(100).Infof("The name of the Kiali addon is empty") builder.errorMsg = "the Kiali addon 'name' cannot be empty when Kiali addon is enabled" - } - } - if builder.errorMsg != "" { - return builder + return builder + } } addonConfig := &istiov2.KialiAddonConfig{ @@ -262,6 +266,8 @@ func (builder *ControlPlaneBuilder) WithPrometheusAddon( builder.errorMsg = "the Prometheus addon 'prometheusInstallConfig' cannot " + "be empty when Prometheus addon is enabled" + + return builder } if metricsExpiryDuration == "" { @@ -269,17 +275,17 @@ func (builder *ControlPlaneBuilder) WithPrometheusAddon( builder.errorMsg = "the Prometheus addon 'metricsExpiryDuration' cannot " + "be empty when Prometheus addon is enabled" + + return builder } if address == "" { glog.V(100).Info("The address of the Prometheus addon is empty") builder.errorMsg = "the Prometheus addon 'address' cannot be empty when Prometheus addon is enabled" - } - } - if builder.errorMsg != "" { - return builder + return builder + } } addonConfig := &istiov2.PrometheusAddonConfig{ @@ -340,7 +346,7 @@ func PullControlPlane(apiClient *clients.Settings, name, nsname string) (*Contro return nil, err } - builder := ControlPlaneBuilder{ + builder := &ControlPlaneBuilder{ apiClient: apiClient.Client, Definition: &istiov2.ServiceMeshControlPlane{ ObjectMeta: metav1.ObjectMeta{ @@ -362,17 +368,13 @@ func PullControlPlane(apiClient *clients.Settings, name, nsname string) (*Contro return nil, fmt.Errorf("serviceMeshControlPlane 'nsname' cannot be empty") } - if builder.errorMsg != "" { - return &builder, nil - } - if !builder.Exists() { return nil, fmt.Errorf("serviceMeshControlPlane object %s does not exist in namespace %s", name, nsname) } builder.Definition = builder.Object - return &builder, nil + return builder, nil } // Get fetches existing serviceMeshControlPlane from cluster. @@ -514,13 +516,13 @@ func (builder *ControlPlaneBuilder) 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/servicemesh/memberroll.go b/pkg/servicemesh/memberroll.go index 29db04f25..1d65f6177 100644 --- a/pkg/servicemesh/memberroll.go +++ b/pkg/servicemesh/memberroll.go @@ -63,12 +63,16 @@ func NewMemberRollBuilder(apiClient *clients.Settings, name, nsname string) *Mem glog.V(100).Infof("The name of the serviceMeshMemberRoll is empty") builder.errorMsg = "serviceMeshMemberRoll 'name' cannot be empty" + + return builder } if nsname == "" { glog.V(100).Infof("The namespace of the serviceMeshMemberRoll is empty") builder.errorMsg = "serviceMeshMemberRoll 'nsname' cannot be empty" + + return builder } return builder @@ -92,7 +96,7 @@ func PullMemberRoll(apiClient *clients.Settings, name, nsname string) (*MemberRo return nil, err } - builder := MemberRollBuilder{ + builder := &MemberRollBuilder{ apiClient: apiClient.Client, Definition: &istiov1.ServiceMeshMemberRoll{ ObjectMeta: metav1.ObjectMeta{ @@ -120,7 +124,7 @@ func PullMemberRoll(apiClient *clients.Settings, name, nsname string) (*MemberRo builder.Definition = builder.Object - return &builder, nil + return builder, nil } // Get fetches existing serviceMeshMemberRoll from cluster. @@ -254,9 +258,7 @@ func (builder *MemberRollBuilder) WithMembersList(membersList []string) *MemberR glog.V(100).Infof("Cannot add empty membersList to the memberRoll structure") builder.errorMsg = "can not modify memberRoll config with empty membersList" - } - if builder.errorMsg != "" { return builder } @@ -331,13 +333,13 @@ func (builder *MemberRollBuilder) 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/sriov-fec/nodeconfig.go b/pkg/sriov-fec/nodeconfig.go index 0437c232d..dc2044676 100644 --- a/pkg/sriov-fec/nodeconfig.go +++ b/pkg/sriov-fec/nodeconfig.go @@ -321,13 +321,13 @@ func (builder *NodeConfigBuilder) 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/sriov/network.go b/pkg/sriov/network.go index 5940c4467..5a9fdebce 100644 --- a/pkg/sriov/network.go +++ b/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 } @@ -115,6 +121,8 @@ func (builder *NetworkBuilder) WithVlanProto(vlanProtocol string) *NetworkBuilde allowedVlanProto := []string{"802.1q", "802.1Q", "802.1ad", "802.1AD"} if !slices.Contains(allowedVlanProto, vlanProtocol) { builder.errorMsg = "invalid 'vlanProtocol' parameters" + + return builder } builder.Definition.Spec.VlanProto = vlanProtocol @@ -145,10 +153,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 +177,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 +234,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 +322,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 +350,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 +559,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 +582,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 != "" { diff --git a/pkg/sriov/networknodestate.go b/pkg/sriov/networknodestate.go index fbfd9e727..46f35b662 100644 --- a/pkg/sriov/networknodestate.go +++ b/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 @@ -225,17 +229,13 @@ func (builder *NetworkNodeStateBuilder) findInterfaceByName(sriovInterfaceName s if err := builder.Discover(); err != nil { 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, fmt.Errorf("failed to discover sriov network node state") } 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 +261,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 != "" { diff --git a/pkg/sriov/operatorconfig.go b/pkg/sriov/operatorconfig.go index 870870f23..54206674a 100644 --- a/pkg/sriov/operatorconfig.go +++ b/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. diff --git a/pkg/sriov/policy.go b/pkg/sriov/policy.go index 64d0e4b14..e93f87feb 100644 --- a/pkg/sriov/policy.go +++ b/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{ @@ -75,30 +75,54 @@ func NewPolicyBuilder( } if name == "" { + glog.V(100).Infof("The name of the sriovnetworknodepolicy is empty") + builder.errorMsg = "SriovNetworkNodePolicy 'name' cannot be empty" + + return builder } if nsname == "" { + glog.V(100).Infof("The namespace of the sriovnetworknodepolicy is empty") + builder.errorMsg = "SriovNetworkNodePolicy 'nsname' cannot be empty" + + return builder } if resName == "" { + glog.V(100).Infof("The resName of the sriovnetworknodepolicy is empty") + builder.errorMsg = "SriovNetworkNodePolicy 'resName' cannot be empty" + + return builder } if len(nicNames) == 0 { + glog.V(100).Infof("The nicNames of the sriovnetworknodepolicy is empty") + builder.errorMsg = "SriovNetworkNodePolicy 'nicNames' cannot be empty list" + + return builder } if len(nodeSelector) == 0 { + glog.V(100).Infof("The nodeSelector of the sriovnetworknodepolicy is empty") + builder.errorMsg = "SriovNetworkNodePolicy 'nodeSelector' cannot be empty map" + + return builder } if vfsNumber <= 0 { + glog.V(100).Infof("The vfsNumber of the sriovnetworknodepolicy is zero or negative") + 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 +152,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 +186,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 +276,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 +304,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 +406,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 != "" { diff --git a/pkg/sriov/poolconfig.go b/pkg/sriov/poolconfig.go index f6b2b67bd..ce4ff46e6 100644 --- a/pkg/sriov/poolconfig.go +++ b/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 != "" { diff --git a/pkg/statefulset/statefulset.go b/pkg/statefulset/statefulset.go index 9293b81ba..013981101 100644 --- a/pkg/statefulset/statefulset.go +++ b/pkg/statefulset/statefulset.go @@ -43,7 +43,7 @@ func NewBuilder( "name: %s, namespace: %s, labels: %s, containerSpec %v", name, nsname, labels, containerSpec) - builder := Builder{ + builder := &Builder{ apiClient: apiClient, Definition: &appsv1.StatefulSet{ Spec: appsv1.StatefulSetSpec{ @@ -69,21 +69,27 @@ func NewBuilder( glog.V(100).Infof("The name of the statefulset is empty") builder.errorMsg = "statefulset 'name' cannot be empty" + + return builder } if nsname == "" { glog.V(100).Infof("The namespace of the statefulset is empty") builder.errorMsg = "statefulset 'namespace' cannot be empty" + + return builder } if labels == nil { glog.V(100).Infof("There are no labels for the statefulset") builder.errorMsg = "statefulset 'labels' cannot be empty" + + return builder } - return &builder + return builder } // WithAdditionalContainerSpecs appends a list of container specs to the statefulset definition. @@ -99,9 +105,7 @@ func (builder *Builder) WithAdditionalContainerSpecs(specs []corev1.Container) * glog.V(100).Infof("The container specs are empty") builder.errorMsg = "cannot accept nil or empty list as container specs" - } - if builder.errorMsg != "" { return builder } @@ -145,7 +149,7 @@ func (builder *Builder) WithOptions(options ...AdditionalOptions) *Builder { func Pull(apiClient *clients.Settings, name, nsname string) (*Builder, error) { glog.V(100).Infof("Pulling existing statefulset name: %s under namespace: %s", name, nsname) - builder := Builder{ + builder := &Builder{ apiClient: apiClient, Definition: &appsv1.StatefulSet{ ObjectMeta: metav1.ObjectMeta{ @@ -156,11 +160,19 @@ func Pull(apiClient *clients.Settings, name, nsname string) (*Builder, error) { } if name == "" { + glog.V(100).Infof("The name of the statefulset is empty") + builder.errorMsg = "statefulset 'name' cannot be empty" + + return nil, fmt.Errorf("statefulset 'name' cannot be empty") } if nsname == "" { + glog.V(100).Infof("The namespace of the statefulset is empty") + builder.errorMsg = "statefulset 'namespace' cannot be empty" + + return nil, fmt.Errorf("statefulset 'namespace' cannot be empty") } if !builder.Exists() { @@ -169,7 +181,7 @@ func Pull(apiClient *clients.Settings, name, nsname string) (*Builder, error) { builder.Definition = builder.Object - return &builder, nil + return builder, nil } // Create generates a statefulset in cluster and stores the created object in struct. @@ -285,13 +297,13 @@ func (builder *Builder) 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/storage/pvc.go b/pkg/storage/pvc.go index e5850d8d7..b590e6cf1 100644 --- a/pkg/storage/pvc.go +++ b/pkg/storage/pvc.go @@ -48,7 +48,7 @@ func NewPVCBuilder(apiClient *clients.Settings, name, nsname string) *PVCBuilder return nil } - builder := PVCBuilder{ + builder := &PVCBuilder{ Definition: &corev1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -64,15 +64,19 @@ func NewPVCBuilder(apiClient *clients.Settings, name, nsname string) *PVCBuilder glog.V(100).Infof("PVC name is empty") builder.errorMsg = "PVC name is empty" + + return builder } if nsname == "" { glog.V(100).Infof("PVC namespace is empty") builder.errorMsg = "PVC namespace is empty" + + return builder } - return &builder + return builder } // WithPVCAccessMode configure access mode for the PV. @@ -81,16 +85,14 @@ func (builder *PVCBuilder) WithPVCAccessMode(accessMode string) (*PVCBuilder, er if accessMode == "" { glog.V(100).Infof("Empty accessMode for PVC %s", builder.Definition.Name) - builder.errorMsg = "Empty accessMode for PVC requested" - return builder, fmt.Errorf(builder.errorMsg) + return nil, fmt.Errorf("empty accessMode for PVC requested") } if !validatePVCAccessMode(accessMode) { glog.V(100).Infof("Invalid accessMode for PVC %s", accessMode) - builder.errorMsg = fmt.Sprintf("Invalid accessMode for PVC %s", accessMode) - return builder, fmt.Errorf(builder.errorMsg) + return builder, fmt.Errorf("invalid accessMode for PVC %s", accessMode) } if builder.Definition.Spec.AccessModes != nil { @@ -116,19 +118,16 @@ func validatePVCAccessMode(accessMode string) bool { // WithPVCCapacity configures the minimum resources the volume should have. func (builder *PVCBuilder) WithPVCCapacity(capacity string) (*PVCBuilder, error) { if capacity == "" { - glog.V(100).Infof("Capacity of the PersistentVolumeClaim is empty") + glog.V(100).Infof("capacity of the PersistentVolumeClaim is empty") - builder.errorMsg = "Capacity of the PersistentVolumeClaim is empty" - - return builder, fmt.Errorf(builder.errorMsg) + return nil, fmt.Errorf("capacity of the PersistentVolumeClaim is empty") } defer func() (*PVCBuilder, error) { if r := recover(); r != nil { glog.V(100).Infof("Failed to parse %v", capacity) - builder.errorMsg = fmt.Sprintf("Failed to parse: %v", capacity) - return builder, fmt.Errorf("failed to parse: %v", capacity) + return nil, fmt.Errorf("failed to parse: %v", capacity) } return builder, nil @@ -147,12 +146,10 @@ func (builder *PVCBuilder) WithStorageClass(storageClass string) (*PVCBuilder, e glog.V(100).Infof("Set storage class %s for the PersistentVolumeClaim", storageClass) if storageClass == "" { - glog.V(100).Infof("Empty storageClass requested for the PersistentVolumeClaim", storageClass) + glog.V(100).Infof("empty storageClass requested for the PersistentVolumeClaim", storageClass) - builder.errorMsg = fmt.Sprintf("Empty storageClass requested for the PersistentVolumeClaim %s", + return nil, fmt.Errorf("empty storageClass requested for the PersistentVolumeClaim %s", builder.Definition.Name) - - return builder, fmt.Errorf(builder.errorMsg) } builder.Definition.Spec.StorageClassName = &storageClass @@ -165,22 +162,18 @@ func (builder *PVCBuilder) WithVolumeMode(volumeMode string) (*PVCBuilder, error glog.V(100).Infof("Set VolumeMode %s for the PersistentVolumeClaim", volumeMode) if volumeMode == "" { - glog.V(100).Infof(fmt.Sprintf("Empty volumeMode requested for the PersistentVolumeClaim %s in %s namespace", + glog.V(100).Infof(fmt.Sprintf("empty volumeMode requested for the PersistentVolumeClaim %s in %s namespace", builder.Definition.Name, builder.Definition.Namespace)) - builder.errorMsg = fmt.Sprintf("Empty volumeMode requested for the PersistentVolumeClaim %s in %s namespace", + return nil, fmt.Errorf("empty volumeMode requested for the PersistentVolumeClaim %s in %s namespace", builder.Definition.Name, builder.Definition.Namespace) - - return builder, fmt.Errorf(builder.errorMsg) } if !validateVolumeMode(volumeMode) { - glog.V(100).Infof(fmt.Sprintf("Unsupported VolumeMode: %s", volumeMode)) + glog.V(100).Infof(fmt.Sprintf("unsupported VolumeMode: %s", volumeMode)) - builder.errorMsg = fmt.Sprintf("Unsupported VolumeMode %q requested for %s PersistentVolumeClaim in %s namespace", + return nil, fmt.Errorf("unsupported VolumeMode %q requested for %s PersistentVolumeClaim in %s namespace", volumeMode, builder.Definition.Name, builder.Definition.Name) - - return builder, fmt.Errorf(builder.errorMsg) } // volumeMode is string while Spec.VolumeMode requires pointer to corev1.PersistentVolumeMode, @@ -295,7 +288,7 @@ func PullPersistentVolumeClaim( return nil, fmt.Errorf("persistentVolumeClaim 'apiClient' cannot be empty") } - builder := PVCBuilder{ + builder := &PVCBuilder{ apiClient: apiClient, Definition: &corev1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{ @@ -324,7 +317,7 @@ func PullPersistentVolumeClaim( builder.Definition = builder.Object - return &builder, nil + return builder, nil } // Exists checks whether the given PersistentVolumeClaim exists. diff --git a/pkg/storage/pvc_test.go b/pkg/storage/pvc_test.go index b2730a0e6..0b32c63d5 100644 --- a/pkg/storage/pvc_test.go +++ b/pkg/storage/pvc_test.go @@ -290,12 +290,12 @@ func TestPersistentVolumeClaimWithVolumeMode(t *testing.T) { }, { testPVC: "Disk", - expectedError: "Unsupported VolumeMode \"Disk\" requested for persistentvolumeclaim-test " + + expectedError: "unsupported VolumeMode \"Disk\" requested for persistentvolumeclaim-test " + "PersistentVolumeClaim in persistentvolumeclaim-test namespace", }, { testPVC: "", - expectedError: "Empty volumeMode requested for the PersistentVolumeClaim persistentvolumeclaim-test " + + expectedError: "empty volumeMode requested for the PersistentVolumeClaim persistentvolumeclaim-test " + "in persistentvolumeclaim-namespace namespace", }, } @@ -326,7 +326,7 @@ func TestPersistentVolumeClaimWithStorageClass(t *testing.T) { }, { testStorageClass: "", - expectedError: "Empty storageClass requested for the PersistentVolumeClaim persistentvolumeclaim-test", + expectedError: "empty storageClass requested for the PersistentVolumeClaim persistentvolumeclaim-test", }, } @@ -356,7 +356,7 @@ func TestPersistentVolumeClaimWithPVCCapacity(t *testing.T) { }, { testCapacity: "", - expectedError: "Capacity of the PersistentVolumeClaim is empty", + expectedError: "capacity of the PersistentVolumeClaim is empty", }, } @@ -390,7 +390,7 @@ func TestPersistentVolumeClaimWithPVCAccessMode(t *testing.T) { }, { testAccessMode: "", - expectedError: "Empty accessMode for PVC requested", + expectedError: "empty accessMode for PVC requested", }, } diff --git a/pkg/velero/backup.go b/pkg/velero/backup.go index db23dc8a0..064b29a20 100644 --- a/pkg/velero/backup.go +++ b/pkg/velero/backup.go @@ -393,13 +393,13 @@ func (builder *BackupBuilder) 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/velero/restore.go b/pkg/velero/restore.go index 2faef10fe..58c7cd43d 100644 --- a/pkg/velero/restore.go +++ b/pkg/velero/restore.go @@ -61,18 +61,24 @@ func NewRestoreBuilder(apiClient *clients.Settings, name, nsname, backupName str glog.V(100).Infof("The name of the restore is empty") builder.errorMsg = "restore name cannot be an empty string" + + return builder } if nsname == "" { glog.V(100).Infof("The namespace of the restore is empty") builder.errorMsg = "restore namespace cannot be an empty string" + + return builder } if backupName == "" { glog.V(100).Infof("The backupName of the restore is empty") builder.errorMsg = "restore backupName cannot be an empty string" + + return builder } return builder @@ -95,7 +101,7 @@ func PullRestore(apiClient *clients.Settings, name, nsname string) (*RestoreBuil return nil, err } - builder := RestoreBuilder{ + builder := &RestoreBuilder{ apiClient: apiClient.Client, Definition: &velerov1.Restore{ ObjectMeta: metav1.ObjectMeta{ @@ -106,10 +112,14 @@ func PullRestore(apiClient *clients.Settings, name, nsname string) (*RestoreBuil } if name == "" { + glog.V(100).Infof("The name of the restore is empty") + return nil, fmt.Errorf("restore name cannot be empty") } if nsname == "" { + glog.V(100).Infof("The namespace of the restore is empty") + return nil, fmt.Errorf("restore namespace cannot be empty") } @@ -119,7 +129,7 @@ func PullRestore(apiClient *clients.Settings, name, nsname string) (*RestoreBuil builder.Definition = builder.Object - return &builder, nil + return builder, nil } // WithStorageLocation adds a storage location to the restore. @@ -136,9 +146,7 @@ func (builder *RestoreBuilder) WithStorageLocation(location string) *RestoreBuil glog.V(100).Infof("Backup storage location is empty") builder.errorMsg = "restore storage location cannot be an empty string" - } - if builder.errorMsg != "" { return builder } @@ -269,13 +277,13 @@ func (builder *RestoreBuilder) 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/webhook/mutatingwebhook.go b/pkg/webhook/mutatingwebhook.go index 9894d3197..bb03494af 100644 --- a/pkg/webhook/mutatingwebhook.go +++ b/pkg/webhook/mutatingwebhook.go @@ -31,7 +31,7 @@ type MutatingConfigurationBuilder struct { func PullMutatingConfiguration(apiClient *clients.Settings, name string) (*MutatingConfigurationBuilder, error) { glog.V(100).Infof("Pulling existing MutatingWebhookConfiguration name %s from cluster", name) - builder := MutatingConfigurationBuilder{ + builder := &MutatingConfigurationBuilder{ apiClient: apiClient, Definition: &admregv1.MutatingWebhookConfiguration{ ObjectMeta: metav1.ObjectMeta{ @@ -43,7 +43,7 @@ func PullMutatingConfiguration(apiClient *clients.Settings, name string) (*Mutat if name == "" { glog.V(100).Infof("The name of the MutatingWebhookConfiguration is empty") - builder.errorMsg = "MutatingWebhookConfiguration 'name' cannot be empty" + return nil, fmt.Errorf("MutatingWebhookConfiguration 'name' cannot be empty") } if !builder.Exists() { @@ -52,7 +52,7 @@ func PullMutatingConfiguration(apiClient *clients.Settings, name string) (*Mutat builder.Definition = builder.Object - return &builder, nil + return builder, nil } // Exists checks whether the given MutatingWebhookConfiguration object exists in the cluster. @@ -146,7 +146,7 @@ func (builder *MutatingConfigurationBuilder) 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 != "" { diff --git a/pkg/webhook/validatingwebhook.go b/pkg/webhook/validatingwebhook.go index f0524738d..da8909734 100644 --- a/pkg/webhook/validatingwebhook.go +++ b/pkg/webhook/validatingwebhook.go @@ -32,7 +32,7 @@ func PullValidatingConfiguration(apiClient *clients.Settings, name string) ( *ValidatingConfigurationBuilder, error) { glog.V(100).Infof("Pulling existing ValidatingWebhookConfiguration name %s from cluster", name) - builder := ValidatingConfigurationBuilder{ + builder := &ValidatingConfigurationBuilder{ apiClient: apiClient, Definition: &admregv1.ValidatingWebhookConfiguration{ ObjectMeta: metav1.ObjectMeta{ @@ -45,6 +45,8 @@ func PullValidatingConfiguration(apiClient *clients.Settings, name string) ( glog.V(100).Infof("The name of the ValidatingWebhookConfiguration is empty") builder.errorMsg = "ValidatingWebhookConfiguration 'name' cannot be empty" + + return builder, nil } if !builder.Exists() { @@ -53,7 +55,7 @@ func PullValidatingConfiguration(apiClient *clients.Settings, name string) ( builder.Definition = builder.Object - return &builder, nil + return builder, nil } // Exists checks whether the given ValidatingWebhookConfiguration object exists in the cluster. @@ -148,7 +150,7 @@ func (builder *ValidatingConfigurationBuilder) 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 != "" {