Handle scope creation properly (#522)

kubernetes-sigs · Jan 25, 2022 · d42e191 · d42e191
1 parent 5be03f2
commit d42e191
Show file tree

Hide file tree

Showing 2 changed files with 77 additions and 49 deletions.
diff --git a/cloud/scope/powervs_cluster.go b/cloud/scope/powervs_cluster.go
@@ -64,46 +64,71 @@ type PowerVSClusterScope struct {
 }
 
 // NewPowerVSClusterScope creates a new PowerVSClusterScope from the supplied parameters.
-func NewPowerVSClusterScope(params PowerVSClusterScopeParams) (*PowerVSClusterScope, error) {
+func NewPowerVSClusterScope(params PowerVSClusterScopeParams) (scope *PowerVSClusterScope, err error) {
+	scope = &PowerVSClusterScope{}
+
+	if params.Client == nil {
+		err = errors.New("failed to generate new scope from nil Client")
+		return
+	}
+	scope.client = params.Client
+
 	if params.Cluster == nil {
-		return nil, errors.New("failed to generate new scope from nil Cluster")
+		err = errors.New("failed to generate new scope from nil Cluster")
+		return
 	}
+	scope.Cluster = params.Cluster
+
 	if params.IBMPowerVSCluster == nil {
-		return nil, errors.New("failed to generate new scope from nil IBMVPCCluster")
+		err = errors.New("failed to generate new scope from nil IBMPowerVSCluster")
+		return
 	}
+	scope.IBMPowerVSCluster = params.IBMPowerVSCluster
 
 	if params.Logger == (logr.Logger{}) {
 		params.Logger = klogr.New()
 	}
+	scope.Logger = params.Logger
+
+	helper, err := patch.NewHelper(params.IBMPowerVSCluster, params.Client)
+	if err != nil {
+		err = errors.Wrap(err, "failed to init patch helper")
+		return
+	}
+	scope.patchHelper = helper
 
 	spec := params.IBMPowerVSCluster.Spec
 
 	auth, err := authenticator.GetAuthenticator()
 	if err != nil {
-		return nil, errors.Wrap(err, "failed to get authenticator")
+		err = errors.Wrap(err, "failed to get authenticator")
+		return
 	}
 
 	account, err := servicesutils.GetAccount(auth)
 	if err != nil {
-		return nil, errors.Wrap(err, "failed to get account")
+		err = errors.Wrap(err, "failed to get account")
+		return
 	}
 
 	rc, err := resourcecontroller.NewService(resourcecontroller.ServiceOptions{})
 	if err != nil {
-		return nil, err
+		return
 	}
 
 	res, _, err := rc.GetResourceInstance(
 		&resourcecontrollerv2.GetResourceInstanceOptions{
 			ID: core.StringPtr(spec.ServiceInstanceID),
 		})
 	if err != nil {
-		return nil, errors.Wrap(err, "failed to get resource instance")
+		err = errors.Wrap(err, "failed to get resource instance")
+		return
 	}
 
 	region, err := utils.GetRegion(*res.RegionID)
 	if err != nil {
-		return nil, errors.Wrap(err, "failed to get region")
+		err = errors.Wrap(err, "failed to get region")
+		return
 	}
 
 	options := powervs.ServiceOptions{
@@ -116,24 +141,13 @@ func NewPowerVSClusterScope(params PowerVSClusterScopeParams) (*PowerVSClusterSc
 		CloudInstanceID: spec.ServiceInstanceID,
 	}
 	c, err := powervs.NewService(options)
-
-	if err != nil {
-		return nil, fmt.Errorf("failed to create NewIBMPowerVSClient")
-	}
-
-	helper, err := patch.NewHelper(params.IBMPowerVSCluster, params.Client)
 	if err != nil {
-		return nil, errors.Wrap(err, "failed to init patch helper")
+		err = fmt.Errorf("failed to create NewIBMPowerVSClient")
+		return
 	}
+	scope.IBMPowerVSClient = c
 
-	return &PowerVSClusterScope{
-		Logger:            params.Logger,
-		client:            params.Client,
-		IBMPowerVSClient:  c,
-		Cluster:           params.Cluster,
-		IBMPowerVSCluster: params.IBMPowerVSCluster,
-		patchHelper:       helper,
-	}, nil
+	return
 }
 
 // PatchObject persists the cluster configuration and status.

diff --git a/cloud/scope/powervs_machine.go b/cloud/scope/powervs_machine.go
@@ -72,52 +72,78 @@ type PowerVSMachineScope struct {
 }
 
 // NewPowerVSMachineScope creates a new PowerVSMachineScope from the supplied parameters.
-func NewPowerVSMachineScope(params PowerVSMachineScopeParams) (*PowerVSMachineScope, error) {
+func NewPowerVSMachineScope(params PowerVSMachineScopeParams) (scope *PowerVSMachineScope, err error) {
+	scope = &PowerVSMachineScope{}
+
 	if params.Client == nil {
-		return nil, errors.New("client is required when creating a MachineScope")
+		err = errors.New("client is required when creating a MachineScope")
+		return
 	}
+	scope.client = params.Client
+
 	if params.Machine == nil {
-		return nil, errors.New("machine is required when creating a MachineScope")
+		err = errors.New("machine is required when creating a MachineScope")
+		return
 	}
+	scope.Machine = params.Machine
+
 	if params.Cluster == nil {
-		return nil, errors.New("cluster is required when creating a MachineScope")
+		err = errors.New("cluster is required when creating a MachineScope")
+		return
 	}
+	scope.Cluster = params.Cluster
+
 	if params.IBMPowerVSMachine == nil {
-		return nil, errors.New("aws machine is required when creating a MachineScope")
+		err = errors.New("aws machine is required when creating a MachineScope")
+		return
 	}
+	scope.IBMPowerVSMachine = params.IBMPowerVSMachine
+	scope.IBMPowerVSCluster = params.IBMPowerVSCluster
 
 	if params.Logger == (logr.Logger{}) {
 		params.Logger = klogr.New()
 	}
+	scope.Logger = params.Logger
+
+	helper, err := patch.NewHelper(params.IBMPowerVSMachine, params.Client)
+	if err != nil {
+		err = errors.Wrap(err, "failed to init patch helper")
+		return
+	}
+	scope.patchHelper = helper
 
 	m := params.IBMPowerVSMachine
 
 	auth, err := authenticator.GetAuthenticator()
 	if err != nil {
-		return nil, errors.Wrap(err, "failed to get authenticator")
+		err = errors.Wrap(err, "failed to get authenticator")
+		return
 	}
 
 	account, err := servicesutils.GetAccount(auth)
 	if err != nil {
-		return nil, errors.Wrap(err, "failed to get account")
+		err = errors.Wrap(err, "failed to get account")
+		return
 	}
 
 	rc, err := resourcecontroller.NewService(resourcecontroller.ServiceOptions{})
 	if err != nil {
-		return nil, err
+		return
 	}
 
 	res, _, err := rc.GetResourceInstance(
 		&resourcecontrollerv2.GetResourceInstanceOptions{
 			ID: core.StringPtr(m.Spec.ServiceInstanceID),
 		})
 	if err != nil {
-		return nil, errors.Wrap(err, "failed to get resource instance")
+		err = errors.Wrap(err, "failed to get resource instance")
+		return
 	}
 
 	region, err := utils.GetRegion(*res.RegionID)
 	if err != nil {
-		return nil, errors.Wrap(err, "failed to get region")
+		err = errors.Wrap(err, "failed to get region")
+		return
 	}
 
 	options := powervs.ServiceOptions{
@@ -131,24 +157,12 @@ func NewPowerVSMachineScope(params PowerVSMachineScopeParams) (*PowerVSMachineSc
 	}
 	c, err := powervs.NewService(options)
 	if err != nil {
-		return nil, fmt.Errorf("failed to create PowerVS service")
+		err = fmt.Errorf("failed to create PowerVS service")
+		return
 	}
+	scope.IBMPowerVSClient = c
 
-	helper, err := patch.NewHelper(params.IBMPowerVSMachine, params.Client)
-	if err != nil {
-		return nil, errors.Wrap(err, "failed to init patch helper")
-	}
-	return &PowerVSMachineScope{
-		Logger:      params.Logger,
-		client:      params.Client,
-		patchHelper: helper,
-
-		IBMPowerVSClient:  c,
-		Cluster:           params.Cluster,
-		Machine:           params.Machine,
-		IBMPowerVSMachine: params.IBMPowerVSMachine,
-		IBMPowerVSCluster: params.IBMPowerVSCluster,
-	}, nil
+	return
 }
 
 func (m *PowerVSMachineScope) ensureInstanceUnique(instanceName string) (*models.PVMInstanceReference, error) {