airship/airshipctl
Code Issues Proposed changes

Move remote client interface to separate package

Browse Source 
This will allow to add constructors that return interface
instead of struct.

Relates-To: #397
Relates-To: #362
Relates-To: #359

Change-Id: Ia750d5c9472041b34ab74a2935ef77ae955a016e
This commit is contained in:
Kostiantyn Kalynovskyi 2020-11-09 12:10:21 -06:00 committed by Kostyantyn Kalynovskyi
parent 7a404c6fa6
commit d442e20dcb
6 changed files with 105 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

44
pkg/remote/ifc/ifc.go Normal file
View File

@ -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)

67
pkg/remote/management.go
View File

@ -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(
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}
clientFactory = redfish.ClientFactory
case redfishdell.ClientType:
log.Debug("Remote type: Redfish for Integrated Dell Remote Access Controller (iDrac) systems")
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}
clientFactory = redfish.ClientFactory
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
}

13
pkg/remote/redfish/client.go
View File

@ -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"
)
@ -349,3 +350,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)
}

6
pkg/remote/redfish/client_test.go
View File

@ -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

17
pkg/remote/redfish/vendors/dell/client.go vendored
View File

@ -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"
)
@ -123,8 +124,8 @@ func (c *Client) SetBootSourceByType(ctx context.Context) error {
return nil
}
// NewClient returns a client with the capability to make Redfish requests.
func NewClient(redfishURL string,
// newClient returns a client with the capability to make Redfish requests.
func newClient(redfishURL string,
insecure bool,
useProxy bool,
username string,
@ -141,3 +142,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)
}

12
pkg/remote/redfish/vendors/dell/client_test.go vendored
View File

@ -32,14 +32,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)
@ -48,7 +54,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 := context.Background()