Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Rework intel amt provider to not panic: #304

Merged
merged 1 commit into from
Jan 23, 2023
Merged

Rework intel amt provider to not panic: #304

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 24 additions & 30 deletions providers/intelamt/intelamt.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ var (
}
)



type amtProvider interface {
IsPoweredOn(context.Context) (bool, error)
PowerOn(context.Context) error
Expand All @@ -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
Expand All @@ -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
}
Expand All @@ -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
}

Expand Down
35 changes: 0 additions & 35 deletions providers/intelamt/intelamt_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down