Merge "Move remote client interface to separate package"

pkg/remote/ifc/ifc.go

@@ -0,0 +1,44 @@
+/*
+ Licensed under the Apache License, Version 2.0 (the "License");
+ you may not use this file except in compliance with the License.
+ You may obtain a copy of the License at
+
+     https://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+*/
+
+package ifc
+
+import (
+	"context"
+
+	"opendev.org/airship/airshipctl/pkg/remote/power"
+)
+
+// Client is a set of functions that clients created for out-of-band power management and control should implement. The
+// functions within client are used by power management commands and remote direct functionality.
+type Client interface {
+	EjectVirtualMedia(context.Context) error
+	NodeID() string
+	RebootSystem(context.Context) error
+	SetBootSourceByType(context.Context) error
+	SystemPowerOff(context.Context) error
+	SystemPowerOn(context.Context) error
+	SystemPowerStatus(context.Context) (power.Status, error)
+
+	// TODO(drewwalters96): This function is tightly coupled to Redfish. It should be combined with the
+	// SetBootSource operation and removed from the client interface.
+	SetVirtualMedia(context.Context, string) error
+}
+
+// ClientFactory is a function to be used
+type ClientFactory func(
+	redfishURL string,
+	insecure bool, useProxy bool,
+	username string, password string,
+	systemActionRetries int, systemRebootDelay int) (Client, error)

pkg/remote/management.go

@@ -16,34 +16,16 @@
 package remote
 
 import (
-	"context"
-
 	"opendev.org/airship/airshipctl/pkg/config"
 	"opendev.org/airship/airshipctl/pkg/document"
 	"opendev.org/airship/airshipctl/pkg/log"
 	"opendev.org/airship/airshipctl/pkg/phase"
 	"opendev.org/airship/airshipctl/pkg/phase/ifc"
-	"opendev.org/airship/airshipctl/pkg/remote/power"
+	remoteifc "opendev.org/airship/airshipctl/pkg/remote/ifc"
 	"opendev.org/airship/airshipctl/pkg/remote/redfish"
 	redfishdell "opendev.org/airship/airshipctl/pkg/remote/redfish/vendors/dell"
 )
 
-// Client is a set of functions that clients created for out-of-band power management and control should implement. The
-// functions within client are used by power management commands and remote direct functionality.
-type Client interface {
-	EjectVirtualMedia(context.Context) error
-	NodeID() string
-	RebootSystem(context.Context) error
-	SetBootSourceByType(context.Context) error
-	SystemPowerOff(context.Context) error
-	SystemPowerOn(context.Context) error
-	SystemPowerStatus(context.Context) (power.Status, error)
-
-	// TODO(drewwalters96): This function is tightly coupled to Redfish. It should be combined with the
-	// SetBootSource operation and removed from the client interface.
-	SetVirtualMedia(context.Context, string) error
-}
-
 // Manager orchestrates a grouping of baremetal hosts. When a manager is created using its convenience function, the
 // manager contains a list of hosts ready for out-of-band management. Iterate over the Hosts property to invoke actions
 // on each host.
@@ -55,7 +37,7 @@ type Manager struct {
 // baremetalHost is an airshipctl representation of a baremetal host, defined by a baremetal host document, that embeds
 // actions an out-of-band client can perform. Once instantiated, actions can be performed on a baremetal host.
 type baremetalHost struct {
-	Client
+	remoteifc.Client
 	BMCAddress string
 	HostName   string
 	username   string
@@ -213,44 +195,31 @@ func newBaremetalHost(mgmtCfg config.ManagementConfiguration,
 		return host, err
 	}
 
+	var clientFactory remoteifc.ClientFactory
 	// Select the client that corresponds to the management type specified in the airshipctl config.
 	switch mgmtCfg.Type {
 	case redfish.ClientType:
 		log.Debug("Remote type: Redfish")
-		client, err := redfish.NewClient(
+		clientFactory = redfish.ClientFactory
-			address,
-			mgmtCfg.Insecure,
-			mgmtCfg.UseProxy,
-			username,
-			password,
-			mgmtCfg.SystemActionRetries,
-			mgmtCfg.SystemRebootDelay)
-
-		if err != nil {
-			return host, err
-		}
-
-		host = baremetalHost{client, address, hostDoc.GetName(), username, password}
 	case redfishdell.ClientType:
-		log.Debug("Remote type: Redfish for Integrated Dell Remote Access Controller (iDrac) systems")
+		clientFactory = redfish.ClientFactory
-		client, err := redfishdell.NewClient(
-			address,
-			mgmtCfg.Insecure,
-			mgmtCfg.UseProxy,
-			username,
-			password,
-			mgmtCfg.SystemActionRetries,
-			mgmtCfg.SystemRebootDelay)
-
-		if err != nil {
-			return host, err
-		}
-
-		host = baremetalHost{client, address, hostDoc.GetName(), username, password}
 	default:
 		return host, ErrUnknownManagementType{Type: mgmtCfg.Type}
 	}
 
+	client, err := clientFactory(
+		address,
+		mgmtCfg.Insecure,
+		mgmtCfg.UseProxy,
+		username,
+		password, mgmtCfg.SystemActionRetries,
+		mgmtCfg.SystemRebootDelay)
+
+	if err != nil {
+		return host, err
+	}
+	host = baremetalHost{client, address, hostDoc.GetName(), username, password}
+
 	return host, nil
 }
 

pkg/remote/redfish/client.go

@@ -24,6 +24,7 @@ import (
 	redfishClient "opendev.org/airship/go-redfish/client"
 
 	"opendev.org/airship/airshipctl/pkg/log"
+	"opendev.org/airship/airshipctl/pkg/remote/ifc"
 	"opendev.org/airship/airshipctl/pkg/remote/power"
 )
 
@@ -337,3 +338,15 @@ func NewClient(redfishURL string,
 
 	return c, nil
 }
+
+// ClientFactory is a constructor for redfish ifc.Client implementation
+var ClientFactory ifc.ClientFactory = func(redfishURL string,
+	insecure bool,
+	useProxy bool,
+	username string,
+	password string,
+	systemActionRetries int,
+	systemRebootDelay int) (ifc.Client, error) {
+	return NewClient(redfishURL, insecure, useProxy,
+		username, password, systemActionRetries, systemRebootDelay)
+}

pkg/remote/redfish/client_test.go

@@ -45,6 +45,12 @@ func TestNewClient(t *testing.T) {
 	assert.NotNil(t, c)
 }
 
+func TestNewClientInterface(t *testing.T) {
+	c, err := ClientFactory(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay)
+	assert.NoError(t, err)
+	assert.NotNil(t, c)
+}
+
 func TestNewClientDefaultValues(t *testing.T) {
 	sysActRetr := 111
 	sysRebDel := 999

pkg/remote/redfish/vendors/dell/client.go

@@ -26,6 +26,7 @@ import (
 	redfishClient "opendev.org/airship/go-redfish/client"
 
 	"opendev.org/airship/airshipctl/pkg/log"
+	"opendev.org/airship/airshipctl/pkg/remote/ifc"
 	"opendev.org/airship/airshipctl/pkg/remote/redfish"
 )
 
@@ -126,8 +127,8 @@ func (c *Client) SetBootSourceByType(ctx context.Context) error {
 	return nil
 }
 
-// NewClient returns a client with the capability to make Redfish requests.
+// newClient returns a client with the capability to make Redfish requests.
-func NewClient(redfishURL string,
+func newClient(redfishURL string,
 	insecure bool,
 	useProxy bool,
 	username string,
@@ -144,3 +145,15 @@ func NewClient(redfishURL string,
 
 	return c, nil
 }
+
+// ClientFactory is a constructor for redfish ifc.Client implementation
+var ClientFactory ifc.ClientFactory = func(redfishURL string,
+	insecure bool,
+	useProxy bool,
+	username string,
+	password string,
+	systemActionRetries int,
+	systemRebootDelay int) (ifc.Client, error) {
+	return newClient(redfishURL, insecure, useProxy,
+		username, password, systemActionRetries, systemRebootDelay)
+}

pkg/remote/redfish/vendors/dell/client_test.go

@@ -34,14 +34,20 @@ const (
 )
 
 func TestNewClient(t *testing.T) {
-	_, err := NewClient(redfishURL, false, false, "username", "password", systemActionRetries, systemRebootDelay)
+	_, err := newClient(redfishURL, false, false, "username", "password", systemActionRetries, systemRebootDelay)
 	assert.NoError(t, err)
 }
 
+func TestNewClientInterface(t *testing.T) {
+	c, err := ClientFactory(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay)
+	assert.NoError(t, err)
+	assert.NotNil(t, c)
+}
+
 func TestNewClientDefaultValues(t *testing.T) {
 	sysActRetr := 222
 	sysRebDel := 555
-	c, err := NewClient(redfishURL, false, false, "", "", sysActRetr, sysRebDel)
+	c, err := newClient(redfishURL, false, false, "", "", sysActRetr, sysRebDel)
 	assert.Equal(t, c.SystemActionRetries(), sysActRetr)
 	assert.Equal(t, c.SystemRebootDelay(), sysRebDel)
 	assert.NoError(t, err)
@@ -50,7 +56,7 @@ func TestSetBootSourceByTypeGetSystemError(t *testing.T) {
 	m := &redfishMocks.RedfishAPI{}
 	defer m.AssertExpectations(t)
 
-	client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay)
+	client, err := newClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay)
 	assert.NoError(t, err)
 
 	ctx := redfish.SetAuth(context.Background(), "", "")