From 725e651bac57ba84a7ba09ed98188e178528c9fa Mon Sep 17 00:00:00 2001
From: Joel Rebello <jrebello@packet.com>
Date: Tue, 11 Jul 2023 15:16:25 +0200
Subject: [PATCH] redfish/FirmwareInstall: add test case and lower timeout
 value

---
 providers/redfish/firmware.go      | 16 ++++++++----
 providers/redfish/firmware_test.go | 41 +++++++++++++++++++++++-------
 2 files changed, 43 insertions(+), 14 deletions(-)

diff --git a/providers/redfish/firmware.go b/providers/redfish/firmware.go
index 3d5f432a..94112521 100644
--- a/providers/redfish/firmware.go
+++ b/providers/redfish/firmware.go
@@ -23,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{
@@ -44,10 +48,12 @@ func (c *Conn) FirmwareInstall(ctx context.Context, component, applyAt string, f
 		return "", errors.Wrap(bmclibErrs.ErrFirmwareInstall, "invalid applyAt parameter: "+applyAt)
 	}
 
-	// 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
+	//
+	// 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) < 30*time.Minute {
-		return "", errors.New("remaining context deadline insufficient to perform update: " + time.Until(ctxDeadline).String())
+	if time.Until(ctxDeadline) < 10*time.Minute {
+		return "", errors.Wrap(errInsufficientCtxTimeout, " "+time.Until(ctxDeadline).String())
 	}
 
 	// list redfish firmware install task if theres one present
@@ -84,8 +90,8 @@ 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 that is generally set to a much lower value,
-	// and we cannot pass a context deadline for the multipart upload.
+	// 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()
diff --git a/providers/redfish/firmware_test.go b/providers/redfish/firmware_test.go
index 9611d6d1..8e098e4d 100644
--- a/providers/redfish/firmware_test.go
+++ b/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()
 		})
 	}