Skip to content

Commit

Permalink
redfish/FirmwareInstall: add test case and lower timeout value
Browse files Browse the repository at this point in the history
  • Loading branch information
joelrebel committed Jul 14, 2023
1 parent 7ebacd2 commit 725e651
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 14 deletions.
16 changes: 11 additions & 5 deletions providers/redfish/firmware.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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
Expand Down Expand Up @@ -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()
Expand Down
41 changes: 32 additions & 9 deletions providers/redfish/firmware_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"path/filepath"
"strings"
"testing"
"time"

"github.com/stretchr/testify/assert"

Expand Down Expand Up @@ -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,
Expand All @@ -93,6 +107,7 @@ func Test_FirmwareInstall(t *testing.T) {
common.SlugBIOS,
constants.FirmwareApplyOnReset,
false,
true,
fh,
"467696020275",
bmclibErrs.ErrFirmwareInstall,
Expand All @@ -103,6 +118,7 @@ func Test_FirmwareInstall(t *testing.T) {
common.SlugBIOS,
constants.FirmwareApplyOnReset,
true,
true,
fh,
"467696020275",
nil,
Expand All @@ -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 != "" {
Expand All @@ -123,6 +144,8 @@ func Test_FirmwareInstall(t *testing.T) {
assert.Nil(t, err)
assert.Equal(t, tc.expectTaskID, taskID)
}

defer cancel()
})
}

Expand Down

0 comments on commit 725e651

Please sign in to comment.