Merge pull request #337 from bmc-toolbox/redfish-override-timeout

Redfish override timeout for FirmwareInstall

Author: joelrebel

internal/redfishwrapper/client.go

@@ -161,6 +161,16 @@ func (c *Client) SessionActive() error {
 	return nil
 }
 
+// Overrides the HTTP client timeout
+func (c *Client) SetHttpClientTimeout(t time.Duration) {
+	c.client.HTTPClient.Timeout = t
+}
+
+// retrieve the current HTTP client timeout
+func (c *Client) HttpClientTimeout() time.Duration {
+	return c.client.HTTPClient.Timeout
+}
+
 // RunRawRequestWithHeaders wraps the gofish client method RunRawRequestWithHeaders
 func (c *Client) RunRawRequestWithHeaders(method, url string, payloadBuffer io.ReadSeeker, contentType string, customHeaders map[string]string) (*http.Response, error) {
 	if err := c.SessionActive(); err != nil {

providers/redfish/firmware.go

@@ -13,6 +13,7 @@ import (
 	"path/filepath"
 	"strconv"
 	"strings"
+	"time"
 
 	"github.com/pkg/errors"
 	gofishrf "github.com/stmcginnis/gofish/redfish"
@@ -22,6 +23,10 @@ import (
 	"github.com/bmc-toolbox/bmclib/v2/internal"
 )
 
+var (
+	errInsufficientCtxTimeout = errors.New("remaining context timeout insufficient to install firmware")
+)
+
 // SupportedFirmwareApplyAtValues returns the supported redfish firmware applyAt values
 func SupportedFirmwareApplyAtValues() []string {
 	return []string{
@@ -43,6 +48,14 @@ func (c *Conn) FirmwareInstall(ctx context.Context, component, applyAt string, f
 		return "", errors.Wrap(bmclibErrs.ErrFirmwareInstall, "invalid applyAt parameter: "+applyAt)
 	}
 
+	// expect atleast 10 minutes left in the deadline to proceed with the update
+	//
+	// this gives the BMC enough time to have the firmware uploaded and return a response to the client.
+	ctxDeadline, _ := ctx.Deadline()
+	if time.Until(ctxDeadline) < 10*time.Minute {
+		return "", errors.Wrap(errInsufficientCtxTimeout, " "+time.Until(ctxDeadline).String())
+	}
+
 	// list redfish firmware install task if theres one present
 	task, err := c.GetFirmwareInstallTaskQueued(ctx, component)
 	if err != nil {
@@ -77,6 +90,17 @@ func (c *Conn) FirmwareInstall(ctx context.Context, component, applyAt string, f
 		return "", errors.Wrap(bmclibErrs.ErrFirmwareInstall, err.Error())
 	}
 
+	// override the gofish HTTP client timeout,
+	// since the context timeout is set at Open() and is at a lower value than required for this operation.
+	//
+	// record the http client time to be restored
+	httpClientTimeout := c.redfishwrapper.HttpClientTimeout()
+	defer func() {
+		c.redfishwrapper.SetHttpClientTimeout(httpClientTimeout)
+	}()
+
+	c.redfishwrapper.SetHttpClientTimeout(time.Until(ctxDeadline))
+
 	payload := map[string]io.Reader{
 		"UpdateParameters": bytes.NewReader(updateParameters),
 		"UpdateFile":       reader,

providers/redfish/firmware_test.go

@@ -10,6 +10,7 @@ import (
 	"path/filepath"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 
@@ -70,19 +71,32 @@ func Test_FirmwareInstall(t *testing.T) {
 	defer os.Remove(binPath)
 
 	tests := []struct {
-		component       string
-		applyAt         string
-		forceInstall    bool
-		reader          io.Reader
-		expectTaskID    string
-		expectErr       error
-		expectErrSubStr string
-		testName        string
+		component          string
+		applyAt            string
+		forceInstall       bool
+		setRequiredTimeout bool
+		reader             io.Reader
+		expectTaskID       string
+		expectErr          error
+		expectErrSubStr    string
+		testName           string
 	}{
+		{
+			common.SlugBIOS,
+			constants.FirmwareApplyOnReset,
+			false,
+			false,
+			nil,
+			"",
+			errInsufficientCtxTimeout,
+			"",
+			"remaining context deadline",
+		},
 		{
 			common.SlugBIOS,
 			"invalidApplyAt",
 			false,
+			true,
 			nil,
 			"",
 			bmclibErrs.ErrFirmwareInstall,
@@ -93,6 +107,7 @@ func Test_FirmwareInstall(t *testing.T) {
 			common.SlugBIOS,
 			constants.FirmwareApplyOnReset,
 			false,
+			true,
 			fh,
 			"467696020275",
 			bmclibErrs.ErrFirmwareInstall,
@@ -103,6 +118,7 @@ func Test_FirmwareInstall(t *testing.T) {
 			common.SlugBIOS,
 			constants.FirmwareApplyOnReset,
 			true,
+			true,
 			fh,
 			"467696020275",
 			nil,
@@ -113,7 +129,12 @@ func Test_FirmwareInstall(t *testing.T) {
 
 	for _, tc := range tests {
 		t.Run(tc.testName, func(t *testing.T) {
-			taskID, err := mockClient.FirmwareInstall(context.TODO(), tc.component, tc.applyAt, tc.forceInstall, tc.reader)
+			ctx, cancel := context.WithTimeout(context.TODO(), 1*time.Second)
+			if tc.setRequiredTimeout {
+				ctx, cancel = context.WithTimeout(context.TODO(), 20*time.Minute)
+			}
+
+			taskID, err := mockClient.FirmwareInstall(ctx, tc.component, tc.applyAt, tc.forceInstall, tc.reader)
 			if tc.expectErr != nil {
 				assert.ErrorIs(t, err, tc.expectErr)
 				if tc.expectErrSubStr != "" {
@@ -123,6 +144,8 @@ func Test_FirmwareInstall(t *testing.T) {
 				assert.Nil(t, err)
 				assert.Equal(t, tc.expectTaskID, taskID)
 			}
+
+			defer cancel()
 		})
 	}
 

providers/supermicro/firmware.go

@@ -28,9 +28,9 @@ func (c *Client) FirmwareInstall(ctx context.Context, component, applyAt string,
 		size = finfo.Size()
 	}
 
-	// expect atleast 30 minutes left in the deadline to proceed with the update
+	// expect atleast 10 minutes left in the deadline to proceed with the update
 	d, _ := ctx.Deadline()
-	if time.Until(d) < 30*time.Minute {
+	if time.Until(d) < 10*time.Minute {
 		return "", errors.New("remaining context deadline insufficient to perform update: " + time.Until(d).String())
 	}