Supermicro X13 inventory/firmware support FS-1671

.github/workflows/ci.yaml

@@ -18,8 +18,8 @@ jobs:
     - name: Run golangci-lint
       uses: golangci/golangci-lint-action@v3
       with:
-        args: -v --config .golangci.yml --timeout=5m
-        version: latest
+        args: -v --config .golangci.yml --timeout=5m --out-format=colored-line-number
+        version: v1.55.2
     - name: make all-checks
       run: make all-checks
   test:

internal/redfishwrapper/firmware.go

@@ -19,6 +19,7 @@ import (
 
 	"github.com/bmc-toolbox/bmclib/v2/constants"
 	bmclibErrs "github.com/bmc-toolbox/bmclib/v2/errors"
+	redfish "github.com/stmcginnis/gofish/redfish"
 )
 
 type installMethod string
@@ -105,6 +106,14 @@ func (c *Client) FirmwareUpload(ctx context.Context, updateFile *os.File, params
 		)
 	}
 
+	// For X13 the full Task structure is returned in the body.   If we can Unmarshall then we can safely assume
+	// that redfishTask.ID contains the ID.
+	redfishTask := &redfish.Task{}
+	err = json.Unmarshal(response, redfishTask)
+	if err == nil {
+		return redfishTask.ID, nil
+	}
+
 	// The response contains a location header pointing to the task URI
 	// Location: /redfish/v1/TaskService/Tasks/JID_467696020275
 	var location = resp.Header.Get("Location")

providers/supermicro/firmware.go

@@ -27,6 +27,7 @@ var (
 		"X11SSE-F",
 		"X12STH-SYS",
 		"X12SPO-NTF",
+		"X13DEM",
 	}
 
 	errUploadTaskIDExpected = errors.New("expected an firmware upload taskID")
@@ -46,7 +47,7 @@ func (c *Client) FirmwareUpload(ctx context.Context, component string, file *os.
 		return "", err
 	}
 
-	//	// expect atleast 5 minutes left in the deadline to proceed with the upload
+	// expect atleast 5 minutes left in the deadline to proceed with the upload
 	d, _ := ctx.Deadline()
 	if time.Until(d) < 5*time.Minute {
 		return "", errors.New("remaining context deadline insufficient to perform update: " + time.Until(d).String())

providers/supermicro/supermicro.go

@@ -175,10 +175,14 @@ func (c *Client) Open(ctx context.Context) (err error) {
 		return err
 	}
 
-	if !bytes.Contains(body, []byte(`url_redirect.cgi?url_name=mainmenu`)) &&
-		!bytes.Contains(body, []byte(`url_redirect.cgi?url_name=topmenu`)) {
+	// X13 appears to have dropped the initial 'mainmenu' redirect
+	if !bytes.Contains(body, []byte(`url_redirect.cgi?url_name=topmenu`)) {
 		return closeWithError(ctx, errors.Wrap(bmclibErrs.ErrLoginFailed, "unexpected response contents"))
 	}
+	// if !bytes.Contains(body, []byte(`url_redirect.cgi?url_name=mainmenu`)) &&
+	// 	!bytes.Contains(body, []byte(`url_redirect.cgi?url_name=topmenu`)) {
+	// 	return closeWithError(ctx, errors.Wrap(bmclibErrs.ErrLoginFailed, "unexpected response contents"))
+	// }
 
 	contentsTopMenu, status, err := c.serviceClient.query(ctx, "cgi/url_redirect.cgi?url_name=topmenu", http.MethodGet, nil, nil, 0)
 	if err != nil {
@@ -197,6 +201,7 @@ func (c *Client) Open(ctx context.Context) (err error) {
 	c.serviceClient.setCsrfToken(csrfToken)
 
 	c.bmc, err = c.bmcQueryor(ctx)
+
 	if err != nil {
 		return closeWithError(ctx, errors.Wrap(bmclibErrs.ErrLoginFailed, err.Error()))
 	}
@@ -281,17 +286,36 @@ func (c *Client) ResetBiosConfiguration(ctx context.Context) (err error) {
 }
 
 func (c *Client) bmcQueryor(ctx context.Context) (bmcQueryor, error) {
-	x11 := newX11Client(c.serviceClient, c.log)
-	x12 := newX12Client(c.serviceClient, c.log)
+	x11bmc := newX11Client(c.serviceClient, c.log)
+	x12bmc := newX12Client(c.serviceClient, c.log)
+	x13bmc := newX13Client(c.serviceClient, c.log)
 
 	var queryor bmcQueryor
 
-	for _, bmc := range []bmcQueryor{x11, x12} {
+	expected := func(deviceModel string, bmc bmcQueryor) bool {
+		deviceModel = strings.ToLower(deviceModel)
+		switch bmc.(type) {
+		case *x11:
+			if strings.HasPrefix(deviceModel, "x11") {
+				return true
+			}
+		case *x12:
+			if strings.HasPrefix(deviceModel, "x12") {
+				return true
+			}
+		case *x13:
+			if strings.HasPrefix(deviceModel, "x13") {
+				return true
+			}
+		}
+
+		return false
+	}
+
+	for _, bmc := range []bmcQueryor{x11bmc, x12bmc, x13bmc} {
 		var err error
 
-		// Note to maintainers: x12 lacks support for the ipmi.cgi endpoint,
-		// which will lead to our graceful handling of ErrXMLAPIUnsupported below.
-		_, err = bmc.queryDeviceModel(ctx)
+		deviceModel, err := bmc.queryDeviceModel(ctx)
 		if err != nil {
 			if errors.Is(err, ErrXMLAPIUnsupported) {
 				continue
@@ -300,6 +324,11 @@ func (c *Client) bmcQueryor(ctx context.Context) (bmcQueryor, error) {
 			return nil, errors.Wrap(ErrModelUnknown, err.Error())
 		}
 
+		// ensure the device model matches the expected queryor
+		if !expected(deviceModel, bmc) {
+			continue
+		}
+
 		queryor = bmc
 		break
 	}
@@ -309,8 +338,8 @@ func (c *Client) bmcQueryor(ctx context.Context) (bmcQueryor, error) {
 	}
 
 	model := strings.ToLower(queryor.deviceModel())
-	if !strings.HasPrefix(model, "x12") && !strings.HasPrefix(model, "x11") {
-		return nil, errors.Wrap(ErrModelUnsupported, "expected one of X11* or X12*, got:"+model)
+	if !strings.HasPrefix(model, "x13") && !strings.HasPrefix(model, "x12") && !strings.HasPrefix(model, "x11") {
+		return nil, errors.Wrap(ErrModelUnsupported, "expected one of X11*, X12* or X13*, got:"+model)
 	}
 
 	return queryor, nil

providers/supermicro/supermicro_test.go

@@ -143,7 +143,7 @@ func TestOpen(t *testing.T) {
 					<title></title>
 					<script language="JavaScript" type="text/javascript">
 				<!--
-					self.location = "../cgi/url_redirect.cgi?url_name=mainmenu";
+					self.location = "../cgi/url_redirect.cgi?url_name=topmenu";
 				-->
 					</script>
 				</head>

providers/supermicro/x11.go

@@ -39,7 +39,7 @@ func (c *x11) queryDeviceModel(ctx context.Context) (string, error) {
 	errBoardPartNumUnknown := errors.New("baseboard part number unknown")
 	data, err := c.fruInfo(ctx)
 	if err != nil {
-		if strings.Contains(err.Error(), "404") {
+		if strings.Contains(err.Error(), "404") || strings.Contains(err.Error(), "<html>") {
 			return "", ErrXMLAPIUnsupported
 		}
 

providers/supermicro/x12.go

@@ -142,6 +142,10 @@ func (c *x12) firmwareTaskActive(ctx context.Context, component string) error {
 
 // noTasksRunning returns an error if a firmware related task was found active
 func noTasksRunning(component string, t *redfish.Task) error {
+	if t.TaskState == "Killed" {
+		return nil
+	}
+
 	errTaskActive := errors.New("A firmware task was found active for component: " + component)
 
 	const (
@@ -222,7 +226,7 @@ func (c *x12) biosFwInstallParams() (map[string]bool, error) {
 		}, nil
 	default:
 		// ideally we never get in this position, since theres model number validation in parent callers.
-		return nil, errors.New("unsupported model for BIOS fw install: " + c.model)
+		return nil, errors.New("unsupported model for x12 BIOS fw install: " + c.model)
 	}
 }