diff --git a/providers/intelamt/intelamt.go b/providers/intelamt/intelamt.go index ac9614b1..4b2711be 100644 --- a/providers/intelamt/intelamt.go +++ b/providers/intelamt/intelamt.go @@ -28,6 +28,8 @@ var ( } ) + + type amtProvider interface { IsPoweredOn(context.Context) (bool, error) PowerOn(context.Context) error @@ -53,21 +55,13 @@ func New(log logr.Logger, host string, port string, user string, pass string) *C if err != nil { p = 16992 } - conn := amt.Connection{ - Host: host, - Port: uint32(p), - User: user, - Pass: pass, - Logger: log, - } - client, _ := amt.NewClient(conn) + c := &Conn{ - Host: host, - Port: uint32(p), - User: user, - Pass: pass, - Log: log, - client: client, + Host: host, + Port: uint32(p), + User: user, + Pass: pass, + Log: log, } return c @@ -78,24 +72,25 @@ func (c *Conn) Name() string { return ProviderName } + + // Open a connection to the BMC via AMT. // The AMT library does not do/use sessions so opening just instantiates the Conn.client. // It will communicate with the BMC. func (c *Conn) Open(ctx context.Context) (err error) { - if c.client == nil { - conn := amt.Connection{ - Host: c.Host, - Port: c.Port, - User: c.User, - Pass: c.Pass, - Logger: c.Log, - } - client, err := amt.NewClient(conn) - if err != nil { - return err - } - c.client = client + conn := amt.Connection{ + Host: c.Host, + Port: c.Port, + User: c.User, + Pass: c.Pass, + Logger: c.Log, + } + // amt.NewClient is used here in Open instead of in New because amt.NewClient makes a connection to the BMC. + client, err := amt.NewClient(conn) + if err != nil { + return err } + c.client = client return nil } @@ -107,12 +102,11 @@ func (c *Conn) Close() (err error) { // Compatible tests whether a BMC is compatible with the ipmitool provider func (c *Conn) Compatible(ctx context.Context) bool { - amtclient, ok := c.client.(*amt.Client) - if !ok || amtclient == nil { + if err := c.Open(ctx); err != nil { return false } - if _, err := amtclient.IsPoweredOn(ctx); err != nil { + if _, err := c.client.IsPoweredOn(ctx); err != nil { return false } diff --git a/providers/intelamt/intelamt_test.go b/providers/intelamt/intelamt_test.go index 7b1f7bb8..b7aa9cb7 100644 --- a/providers/intelamt/intelamt_test.go +++ b/providers/intelamt/intelamt_test.go @@ -47,17 +47,6 @@ func (m *mock) SetPXE(ctx context.Context) error { return m.errSetPXE } -func TestOpen(t *testing.T) { - conn := &Conn{client: &mock{}} - if err := conn.Open(context.Background()); err != nil { - t.Fatal(err) - } - conn = &Conn{} - if err := conn.Open(context.Background()); err == nil { - t.Fatal("expected error, got nil") - } -} - func TestClose(t *testing.T) { conn := &Conn{client: &mock{}} if err := conn.Close(); err != nil { @@ -183,30 +172,6 @@ func TestPowerSet(t *testing.T) { } } -func TestCompatible(t *testing.T) { - tests := map[string]struct { - want bool - }{ - "compatible": {want: true}, - "not compatible": {want: false}, - } - - for name, tt := range tests { - t.Run(name, func(t *testing.T) { - var err error - if !tt.want { - err = errors.New("not compatible") - } - m := &mock{errIsPoweredOn: err} - conn := &Conn{client: m} - got := conn.Compatible(context.Background()) - if diff := cmp.Diff(got, tt.want); diff != "" { - t.Fatal(diff) - } - }) - } -} - func TestNew(t *testing.T) { conn := amt.Connection{Logger: logr.Discard()} wantClient, _ := amt.NewClient(conn)