From 1fe00036bd266a8a3dd0da0d7ba0abffe61e003e Mon Sep 17 00:00:00 2001 From: Jacob Weinstock Date: Wed, 8 Mar 2023 14:31:10 -0700 Subject: [PATCH 1/4] Update Intel AMT library: The previous Intel AMT libraries were forks and the hope was that I would be able to upstream the changes. The upstreams themselves were actually forks and the PRs I opened have been sitting for some time. Because of all of this, I have created a new repo that I'll be maintaining. The new repo handles context timeouts across the board. Signed-off-by: Jacob Weinstock --- go.mod | 6 +-- go.sum | 23 +--------- providers/intelamt/intelamt.go | 65 ++++++++++----------------- providers/intelamt/intelamt_test.go | 68 +++++++++++++++++++++++++---- 4 files changed, 84 insertions(+), 78 deletions(-) diff --git a/go.mod b/go.mod index 398ffb89..a11037eb 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,7 @@ require ( github.com/go-logr/logr v1.2.3 github.com/google/go-cmp v0.5.8 github.com/hashicorp/go-multierror v1.1.1 - github.com/jacobweinstock/go-amt v0.0.0-20221125040441-53475f4ae023 + github.com/jacobweinstock/iamt v0.0.0-20230304043040-a6b4a1001123 github.com/jacobweinstock/registrar v0.4.6 github.com/pkg/errors v0.9.1 github.com/sirupsen/logrus v1.8.1 @@ -25,12 +25,8 @@ require ( github.com/VictorLowther/soap v0.0.0-20150314151524-8e36fca84b22 // indirect github.com/davecgh/go-spew v1.1.1 // indirect github.com/hashicorp/errwrap v1.1.0 // indirect - github.com/jacobweinstock/wsman v0.0.0-20221125035617-2eae65734c77 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/satori/go.uuid v1.2.0 // indirect - golang.org/x/lint v0.0.0-20210508222113-6edffad5e616 // indirect - golang.org/x/mod v0.6.0 // indirect golang.org/x/sys v0.1.0 // indirect - golang.org/x/tools v0.2.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index 323baded..872df6d1 100644 --- a/go.sum +++ b/go.sum @@ -20,12 +20,10 @@ github.com/hashicorp/errwrap v1.1.0 h1:OxrOeh75EUXMY8TBjag2fzXGZ40LB6IKw45YeGUDY github.com/hashicorp/errwrap v1.1.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= github.com/hashicorp/go-multierror v1.1.1 h1:H5DkEtf6CXdFp0N0Em5UCwQpXMWke8IA0+lD48awMYo= github.com/hashicorp/go-multierror v1.1.1/go.mod h1:iw975J/qwKPdAO1clOe2L8331t/9/fmwbPZ6JB6eMoM= -github.com/jacobweinstock/go-amt v0.0.0-20221125040441-53475f4ae023 h1:ethqol+SkT5WtPI23GXlgQhRu/AYSHFTelrve/Acsr4= -github.com/jacobweinstock/go-amt v0.0.0-20221125040441-53475f4ae023/go.mod h1:5SFONG5z/Tqp3iZuxd/k5p/YX2Gu7vVhxpLEmCLrJRQ= +github.com/jacobweinstock/iamt v0.0.0-20230304043040-a6b4a1001123 h1:Mh2eOcadGcu/6E0bq5FfaGlFYFe2oyNOeRjpgC1vOq0= +github.com/jacobweinstock/iamt v0.0.0-20230304043040-a6b4a1001123/go.mod h1:FgmiLTU6cJewV4Xgrq6m5o8CUlTQOJtqzaFLGA0mG+E= github.com/jacobweinstock/registrar v0.4.6 h1:0O3g2jT2Lx+Bf+yl4QsMUN48fVZxUpM3kS+NtIJ+ucw= github.com/jacobweinstock/registrar v0.4.6/go.mod h1:IDx65tQ7DLJ2UqiVjE1zo74jMZZfel9YZW8VrC26m6o= -github.com/jacobweinstock/wsman v0.0.0-20221125035617-2eae65734c77 h1:ks0g5tEGDIW4y+2vGQsfF1Um47u6Cw2AzH6sQC4uGD4= -github.com/jacobweinstock/wsman v0.0.0-20221125035617-2eae65734c77/go.mod h1:XHdWy9hT+4XUjR4lnmA/129m9XGA2gh4EB+GbG/cC+c= github.com/konsorten/go-windows-terminal-sequences v1.0.3/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ= github.com/kr/pretty v0.2.1 h1:Fmg33tUaq4/8ym9TJN1x7sLJnHVwhP33CNkpYV/7rwI= github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI= @@ -49,34 +47,17 @@ github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXf github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.7.2 h1:4jaiDzPyXQvSd7D0EjG45355tLlV3VOECpq10pLC+8s= github.com/stretchr/testify v1.7.2/go.mod h1:R6va5+xMeoiuVRoj+gSkQ7d3FALtqAAGI1FQKckRals= -golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= -golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.1.0 h1:MDRAIl0xIo9Io2xV565hzXHw3zVseKrJKodhohM5CjU= golang.org/x/crypto v0.1.0/go.mod h1:RecgLatLF4+eUMCP1PoPZQb+cVrJcOPbHkTkbkB9sbw= golang.org/x/exp v0.0.0-20230127130021-4ca2cb1a16b7 h1:o7Ps2IYdzLRolS9/nadqeMSHpa9k8pu8u+VKBFUG7cQ= golang.org/x/exp v0.0.0-20230127130021-4ca2cb1a16b7/go.mod h1:CxIveKay+FTh1D0yPZemJVgC/95VzuuOLq5Qi4xnoYc= -golang.org/x/lint v0.0.0-20210508222113-6edffad5e616 h1:VLliZ0d+/avPrXXH+OakdXhpJuEoBZuwh1m2j7U6Iug= -golang.org/x/lint v0.0.0-20210508222113-6edffad5e616/go.mod h1:3xt1FjdF8hUf6vQPIChWIBhFzV8gjjsPE/fR3IyQdNY= -golang.org/x/mod v0.1.1-0.20191105210325-c90efee705ee/go.mod h1:QqPTAvyqsEbceGzBzNggFXnrqF1CaUcvgkdR5Ot7KZg= -golang.org/x/mod v0.6.0 h1:b9gGHsz9/HhJ3HF5DHQytPpuwocVTChQJK3AvoLRD5I= -golang.org/x/mod v0.6.0/go.mod h1:4mET923SAdbXp2ki8ey+zGs1SLqsuM2Y0uvdZR/fUNI= -golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= -golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.1.0 h1:hZ/3BUoy5aId7sCpA/Tc5lt8DkFgdVS2onTpJsZ/fl0= golang.org/x/net v0.1.0/go.mod h1:Cx3nUiGt4eDBEyega/BKRp+/AlGL8hYe7U9odMt2Cco= -golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= -golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20191026070338-33540a1f6037/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210608053332-aa57babbf139/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.1.0 h1:kunALQeHf1/185U1i0GOB/fy1IPRDDpuoOOqRReG57U= golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.1.0 h1:g6Z6vPFA9dYBAF7DWcH6sCcOntplXsDKcliusYijMlw= -golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= -golang.org/x/tools v0.0.0-20200130002326-2f3ba24bd6e7/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28= -golang.org/x/tools v0.2.0 h1:G6AHpWxTMGY1KyEYoAQ5WTtIekUUvDNjan3ugu60JvE= -golang.org/x/tools v0.2.0/go.mod h1:y4OqIKeOV/fWJetJ8bXPU1sEVniLMIyDAZWeHdV+NTA= -golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= diff --git a/providers/intelamt/intelamt.go b/providers/intelamt/intelamt.go index b81769d1..928ad4ea 100644 --- a/providers/intelamt/intelamt.go +++ b/providers/intelamt/intelamt.go @@ -3,12 +3,13 @@ package intelamt import ( "context" "errors" + "fmt" "strconv" "strings" "github.com/bmc-toolbox/bmclib/v2/providers" "github.com/go-logr/logr" - "github.com/jacobweinstock/go-amt" + "github.com/jacobweinstock/iamt" "github.com/jacobweinstock/registrar" ) @@ -28,38 +29,33 @@ var ( } ) -type amtProvider interface { +// iamtClient is +type iamtClient interface { + Close(context.Context) error IsPoweredOn(context.Context) (bool, error) - PowerOn(context.Context) error - PowerOff(context.Context) error + Open(context.Context) error PowerCycle(context.Context) error + PowerOff(context.Context) error + PowerOn(context.Context) error SetPXE(context.Context) error - Close() error } -// Conn is a connection to a BMC via AMT +// Conn is a connection to a BMC via Intel AMT type Conn struct { - Host string - Port uint32 - User string - Pass string - Log logr.Logger - client amtProvider + client iamtClient } // New creates a new AMT connection func New(log logr.Logger, host string, port string, user string, pass string) *Conn { p, err := strconv.Atoi(port) - if err != nil { + if err != nil || p == 623 { p = 16992 } + cli := iamt.NewClient(log, host, "", user, pass) + cli.Port = uint32(p) c := &Conn{ - Host: host, - Port: uint32(p), - User: user, - Pass: pass, - Log: log, + client: cli, } return c @@ -70,37 +66,19 @@ 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. +// Open a connection to the BMC via Intel AMT. func (c *Conn) Open(ctx context.Context) (err error) { - 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 + return c.client.Open(ctx) } // Close a connection to a BMC func (c *Conn) Close() (err error) { - return c.client.Close() + return c.client.Close(context.Background()) } // Compatible tests whether a BMC is compatible with the ipmitool provider func (c *Conn) Compatible(ctx context.Context) bool { - if err := c.Open(ctx); err != nil { + if err := c.client.Open(ctx); err != nil { return false } @@ -125,15 +103,16 @@ func (c *Conn) BootDeviceSet(ctx context.Context, bootDevice string, setPersiste // PowerStateGet gets the power state of a BMC machine func (c *Conn) PowerStateGet(ctx context.Context) (state string, err error) { + fmt.Println("PowerStateGet") on, err := c.client.IsPoweredOn(ctx) if err != nil { return "", err } - if !on { - return "off", nil + if on { + return "on", nil } - return "on", nil + return "off", nil } // PowerSet sets the power state of a BMC machine diff --git a/providers/intelamt/intelamt_test.go b/providers/intelamt/intelamt_test.go index b7aa9cb7..1eb42c08 100644 --- a/providers/intelamt/intelamt_test.go +++ b/providers/intelamt/intelamt_test.go @@ -8,7 +8,7 @@ import ( "github.com/go-logr/logr" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" - "github.com/jacobweinstock/go-amt" + "github.com/jacobweinstock/iamt" ) type mock struct { @@ -18,9 +18,14 @@ type mock struct { errPowerOn error errPowerOff error errPowerCycle error + errOpen error } -func (m *mock) Close() error { +func (m *mock) Open(ctx context.Context) error { + return m.errOpen +} + +func (m *mock) Close(ctx context.Context) error { return nil } @@ -80,7 +85,12 @@ func TestBootDeviceSet(t *testing.T) { m = &mock{errSetPXE: tt.err} } conn := &Conn{client: m} - got, err := conn.BootDeviceSet(context.Background(), tt.device, false, false) + ctx := context.Background() + if err := conn.Open(ctx); err != nil { + t.Fatal(err) + } + defer conn.Close() + got, err := conn.BootDeviceSet(ctx, tt.device, false, false) if err != nil && tt.err == nil { t.Fatalf("expected nil error, got: %v", err) } @@ -114,7 +124,12 @@ func TestPowerStateGet(t *testing.T) { } m := &mock{poweredON: state, errIsPoweredOn: tt.err} conn := &Conn{client: m} - got, err := conn.PowerStateGet(context.Background()) + ctx := context.Background() + if err := conn.Open(ctx); err != nil { + t.Fatal(err) + } + defer conn.Close() + got, err := conn.PowerStateGet(ctx) if err != nil && tt.err == nil { t.Fatalf("expected nil error, got: %v", err) } @@ -161,7 +176,12 @@ func TestPowerSet(t *testing.T) { } m.poweredON = tt.poweredOn conn := &Conn{client: m} - got, err := conn.PowerSet(context.Background(), tt.wantState) + ctx := context.Background() + if err := conn.Open(ctx); err != nil { + t.Fatal(err) + } + defer conn.Close() + got, err := conn.PowerSet(ctx, tt.wantState) if err != nil && tt.err == nil { t.Fatalf("expected nil error, got: %v", err) } @@ -172,14 +192,44 @@ func TestPowerSet(t *testing.T) { } } +func TestCompatible(t *testing.T) { + tests := map[string]struct { + want bool + failOnOpen bool + }{ + "success": {want: true}, + "failed on open": {want: false, failOnOpen: true}, + "failed on power": {want: false}, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + m := &mock{} + if !tt.want { + if tt.failOnOpen { + m.errOpen = errors.New("failed to open") + } else { + m.errIsPoweredOn = errors.New("failed to power on") + } + } + conn := &Conn{client: m} + ctx := context.Background() + defer conn.Close() + got := conn.Compatible(ctx) + 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) - want := &Conn{client: wantClient, Host: "localhost", Port: 16992, User: "admin", Pass: "pass", Log: logr.Discard()} + wantClient := iamt.NewClient(logr.Discard(), "localhost", "", "admin", "pass") + want := &Conn{client: wantClient} got := New(logr.Discard(), "localhost", "", "admin", "pass") t.Log(got == nil) c := Conn{} - a := amt.Client{} + a := iamt.Client{} l := logr.Logger{} if diff := cmp.Diff(got, want, cmpopts.IgnoreUnexported(c, a, l)); diff != "" { t.Fatal(diff) From 0e3db9097d77e0ace7280c33e1b53b82665b492a Mon Sep 17 00:00:00 2001 From: Jacob Weinstock Date: Wed, 8 Mar 2023 14:40:18 -0700 Subject: [PATCH 2/4] Update linter and fix lint issue. Signed-off-by: Jacob Weinstock --- lint.mk | 2 +- providers/intelamt/intelamt_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lint.mk b/lint.mk index 015bb4ca..1bac7a79 100644 --- a/lint.mk +++ b/lint.mk @@ -20,7 +20,7 @@ LINTERS := FIXERS := GOLANGCI_LINT_CONFIG := $(LINT_ROOT)/.golangci.yml -GOLANGCI_LINT_VERSION ?= v1.50.0 +GOLANGCI_LINT_VERSION ?= v1.51.2 GOLANGCI_LINT_BIN := $(LINT_ROOT)/out/linters/golangci-lint-$(GOLANGCI_LINT_VERSION)-$(LINT_ARCH) $(GOLANGCI_LINT_BIN): mkdir -p $(LINT_ROOT)/out/linters diff --git a/providers/intelamt/intelamt_test.go b/providers/intelamt/intelamt_test.go index 1eb42c08..fb7251ff 100644 --- a/providers/intelamt/intelamt_test.go +++ b/providers/intelamt/intelamt_test.go @@ -231,7 +231,7 @@ func TestNew(t *testing.T) { c := Conn{} a := iamt.Client{} l := logr.Logger{} - if diff := cmp.Diff(got, want, cmpopts.IgnoreUnexported(c, a, l)); diff != "" { + if diff := cmp.Diff(got, want, cmpopts.IgnoreUnexported(c, a, l)); diff != "" { //nolint:govet t.Fatal(diff) } } From 310cac229921f2a7f79f063288c0fa6f9605ed3c Mon Sep 17 00:00:00 2001 From: Jacob Weinstock Date: Wed, 8 Mar 2023 14:48:51 -0700 Subject: [PATCH 3/4] Fix linting issue Signed-off-by: Jacob Weinstock --- providers/intelamt/intelamt_test.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/providers/intelamt/intelamt_test.go b/providers/intelamt/intelamt_test.go index fb7251ff..f317c175 100644 --- a/providers/intelamt/intelamt_test.go +++ b/providers/intelamt/intelamt_test.go @@ -8,7 +8,6 @@ import ( "github.com/go-logr/logr" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" - "github.com/jacobweinstock/iamt" ) type mock struct { @@ -224,14 +223,13 @@ func TestCompatible(t *testing.T) { } func TestNew(t *testing.T) { - wantClient := iamt.NewClient(logr.Discard(), "localhost", "", "admin", "pass") + wantClient := &mock{} want := &Conn{client: wantClient} got := New(logr.Discard(), "localhost", "", "admin", "pass") t.Log(got == nil) c := Conn{} - a := iamt.Client{} l := logr.Logger{} - if diff := cmp.Diff(got, want, cmpopts.IgnoreUnexported(c, a, l)); diff != "" { //nolint:govet + if diff := cmp.Diff(got, want, cmpopts.IgnoreUnexported(c, l)); diff != "" { t.Fatal(diff) } } From b46b875dee1cf25d410cff84c02db54344cdb1ab Mon Sep 17 00:00:00 2001 From: Jacob Weinstock Date: Thu, 9 Mar 2023 07:13:19 -0700 Subject: [PATCH 4/4] Remove left over debug print statement. Signed-off-by: Jacob Weinstock --- providers/intelamt/intelamt.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/providers/intelamt/intelamt.go b/providers/intelamt/intelamt.go index 928ad4ea..a202cf75 100644 --- a/providers/intelamt/intelamt.go +++ b/providers/intelamt/intelamt.go @@ -3,7 +3,6 @@ package intelamt import ( "context" "errors" - "fmt" "strconv" "strings" @@ -103,7 +102,6 @@ func (c *Conn) BootDeviceSet(ctx context.Context, bootDevice string, setPersiste // PowerStateGet gets the power state of a BMC machine func (c *Conn) PowerStateGet(ctx context.Context) (state string, err error) { - fmt.Println("PowerStateGet") on, err := c.client.IsPoweredOn(ctx) if err != nil { return "", err