From fa233a6a56505f7cb5b589771b7a7a70bb839f1c Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Wed, 11 Nov 2020 15:07:20 -0800 Subject: [PATCH 1/3] remove unused code leftover from original progress bar implementation --- common/terminal.go | 6 --- common/terminal_posix.go | 39 ----------------- common/terminal_test.go | 9 ---- common/terminal_windows.go | 86 -------------------------------------- 4 files changed, 140 deletions(-) delete mode 100644 common/terminal.go delete mode 100644 common/terminal_posix.go delete mode 100644 common/terminal_test.go delete mode 100644 common/terminal_windows.go diff --git a/common/terminal.go b/common/terminal.go deleted file mode 100644 index 4e862880a..000000000 --- a/common/terminal.go +++ /dev/null @@ -1,6 +0,0 @@ -package common - -// call into one of the platform-specific implementations to get the current terminal dimensions -func GetTerminalDimensions() (width, height int, err error) { - return platformGetTerminalDimensions() -} diff --git a/common/terminal_posix.go b/common/terminal_posix.go deleted file mode 100644 index 2a98a4fbd..000000000 --- a/common/terminal_posix.go +++ /dev/null @@ -1,39 +0,0 @@ -// +build !windows - -package common - -// Imports for determining terminal information across platforms -import ( - "os" - - "golang.org/x/sys/unix" -) - -// posix api -func platformGetTerminalDimensions() (width, height int, err error) { - - // grab the handle to stdin - // XXX: in some cases, packer closes stdin, so the following can't be guaranteed - /* - tty := os.Stdin - */ - - // open up a handle to the current tty - tty, err := os.Open("/dev/tty") - if err != nil { - return 0, 0, err - } - defer tty.Close() - - // convert the handle into a file descriptor - fd := int(tty.Fd()) - - // use it to make an Ioctl - ws, err := unix.IoctlGetWinsize(fd, unix.TIOCGWINSZ) - if err != nil { - return 0, 0, err - } - - // return the width and height - return int(ws.Col), int(ws.Row), nil -} diff --git a/common/terminal_test.go b/common/terminal_test.go deleted file mode 100644 index f1c66b092..000000000 --- a/common/terminal_test.go +++ /dev/null @@ -1,9 +0,0 @@ -package common - -import "testing" - -func TestGetTerminalDimensions(t *testing.T) { - if _, _, err := GetTerminalDimensions(); err != nil { - t.Fatalf("Unable to get terminal dimensions: %s", err) - } -} diff --git a/common/terminal_windows.go b/common/terminal_windows.go deleted file mode 100644 index 556873eb0..000000000 --- a/common/terminal_windows.go +++ /dev/null @@ -1,86 +0,0 @@ -// +build windows - -package common - -import ( - "syscall" - "unsafe" -) - -// windows constants and structures pulled from msdn -const ( - _STD_INPUT_HANDLE = -10 - _STD_OUTPUT_HANDLE = -11 - _STD_ERROR_HANDLE = -12 -) - -type ( - _SHORT int16 - _WORD uint16 - - _SMALL_RECT struct { - Left, Top, Right, Bottom _SHORT - } - _COORD struct { - X, Y _SHORT - } - _CONSOLE_SCREEN_BUFFER_INFO struct { - dwSize, dwCursorPosition _COORD - wAttributes _WORD - srWindow _SMALL_RECT - dwMaximumWindowSize _COORD - } -) - -// Low-level functions that call into Windows API for getting console info -var kernel32 = syscall.NewLazyDLL("kernel32.dll") -var kernel32_GetStdHandleProc = kernel32.NewProc("GetStdHandle") -var kernel32_GetConsoleScreenBufferInfoProc = kernel32.NewProc("GetConsoleScreenBufferInfo") - -func kernel32_GetStdHandle(nStdHandle int32) (syscall.Handle, error) { - res, _, err := kernel32_GetStdHandleProc.Call(uintptr(nStdHandle)) - if res == uintptr(syscall.InvalidHandle) { - return syscall.InvalidHandle, error(err) - } - return syscall.Handle(res), nil -} - -func kernel32_GetConsoleScreenBufferInfo(hConsoleOutput syscall.Handle, info *_CONSOLE_SCREEN_BUFFER_INFO) error { - ok, _, err := kernel32_GetConsoleScreenBufferInfoProc.Call(uintptr(hConsoleOutput), uintptr(unsafe.Pointer(info))) - if int(ok) == 0 { - return error(err) - } - return nil -} - -// windows api to get the console screen buffer info -func getConsoleScreenBufferInfo(csbi *_CONSOLE_SCREEN_BUFFER_INFO) (err error) { - var ( - bi _CONSOLE_SCREEN_BUFFER_INFO - fd syscall.Handle - ) - - // Re-open CONOUT$ as in some instances, stdout may be closed and guaranteed an stdout - if fd, err = syscall.Open("CONOUT$", syscall.O_RDWR, 0); err != nil { - return err - } - defer syscall.Close(fd) - - // grab the dimensions for the console - if err = kernel32_GetConsoleScreenBufferInfo(fd, &bi); err != nil { - return err - } - - *csbi = bi - return nil -} - -func platformGetTerminalDimensions() (width, height int, err error) { - var csbi _CONSOLE_SCREEN_BUFFER_INFO - - if err = getConsoleScreenBufferInfo(&csbi); err != nil { - return 0, 0, err - } - - return int(csbi.dwSize.X), int(csbi.dwSize.Y), nil -} From f52a2ad0fa1e1062e3200f019153a0cb6e8348a0 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Wed, 11 Nov 2020 15:52:06 -0800 Subject: [PATCH 2/3] move retry function that is only used by osc builder into that builder's common dir instead of the global common dir. Fix bug in quemu where the wrong retry value is checked against. --- {common => builder/osc/common/retry}/retry.go | 2 +- {common => builder/osc/common/retry}/retry_test.go | 2 +- builder/osc/common/state.go | 4 ++-- builder/osc/common/step_create_tags.go | 2 +- builder/osc/common/step_run_source_vm.go | 2 +- builder/osc/common/step_stop_bsu_backed_vm.go | 4 ++-- builder/qemu/step_convert_disk.go | 6 +++--- 7 files changed, 11 insertions(+), 11 deletions(-) rename {common => builder/osc/common/retry}/retry.go (99%) rename {common => builder/osc/common/retry}/retry_test.go (99%) diff --git a/common/retry.go b/builder/osc/common/retry/retry.go similarity index 99% rename from common/retry.go rename to builder/osc/common/retry/retry.go index b820cf817..628a86a69 100644 --- a/common/retry.go +++ b/builder/osc/common/retry/retry.go @@ -1,4 +1,4 @@ -package common +package retry import ( "fmt" diff --git a/common/retry_test.go b/builder/osc/common/retry/retry_test.go similarity index 99% rename from common/retry_test.go rename to builder/osc/common/retry/retry_test.go index 364f31a5c..bd59cb459 100644 --- a/common/retry_test.go +++ b/builder/osc/common/retry/retry_test.go @@ -1,4 +1,4 @@ -package common +package retry import ( "fmt" diff --git a/builder/osc/common/state.go b/builder/osc/common/state.go index a2e0e53a7..50afe4cce 100644 --- a/builder/osc/common/state.go +++ b/builder/osc/common/state.go @@ -6,7 +6,7 @@ import ( "log" "github.com/antihax/optional" - "github.com/hashicorp/packer/common" + "github.com/hashicorp/packer/builder/osc/common/retry" "github.com/outscale/osc-sdk-go/osc" ) @@ -68,7 +68,7 @@ func WaitUntilOscSnapshotDone(conn *osc.APIClient, snapshotID string) error { } func waitForState(errCh chan<- error, target string, refresh stateRefreshFunc) { - err := common.Retry(2, 2, 0, func(_ uint) (bool, error) { + err := retry.Retry(2, 2, 0, func(_ uint) (bool, error) { state, err := refresh() if err != nil { return false, err diff --git a/builder/osc/common/step_create_tags.go b/builder/osc/common/step_create_tags.go index f0382f560..2a2c60961 100644 --- a/builder/osc/common/step_create_tags.go +++ b/builder/osc/common/step_create_tags.go @@ -6,7 +6,7 @@ import ( "github.com/antihax/optional" "github.com/aws/aws-sdk-go/aws/awserr" - retry "github.com/hashicorp/packer/common" + "github.com/hashicorp/packer/builder/osc/common/retry" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" "github.com/hashicorp/packer/packer-plugin-sdk/template/interpolate" diff --git a/builder/osc/common/step_run_source_vm.go b/builder/osc/common/step_run_source_vm.go index 2eb251c47..0d0cb6054 100644 --- a/builder/osc/common/step_run_source_vm.go +++ b/builder/osc/common/step_run_source_vm.go @@ -12,7 +12,7 @@ import ( "github.com/aws/aws-sdk-go/aws/awserr" "github.com/outscale/osc-sdk-go/osc" - retry "github.com/hashicorp/packer/common" + "github.com/hashicorp/packer/builder/osc/common/retry" "github.com/hashicorp/packer/helper/communicator" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" diff --git a/builder/osc/common/step_stop_bsu_backed_vm.go b/builder/osc/common/step_stop_bsu_backed_vm.go index 9c31a3c8b..62cd830ac 100644 --- a/builder/osc/common/step_stop_bsu_backed_vm.go +++ b/builder/osc/common/step_stop_bsu_backed_vm.go @@ -6,7 +6,7 @@ import ( "github.com/antihax/optional" "github.com/aws/aws-sdk-go/aws/awserr" - "github.com/hashicorp/packer/common" + "github.com/hashicorp/packer/builder/osc/common/retry" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" "github.com/outscale/osc-sdk-go/osc" @@ -41,7 +41,7 @@ func (s *StepStopBSUBackedVm) Run(ctx context.Context, state multistep.StateBag) // does not exist. // Work around this by retrying a few times, up to about 5 minutes. - err := common.Retry(10, 60, 6, func(i uint) (bool, error) { + err := retry.Retry(10, 60, 6, func(i uint) (bool, error) { ui.Message(fmt.Sprintf("Stopping vm, attempt %d", i+1)) _, _, err = oscconn.VmApi.StopVms(context.Background(), &osc.StopVmsOpts{ diff --git a/builder/qemu/step_convert_disk.go b/builder/qemu/step_convert_disk.go index 8e06cc821..a97343264 100644 --- a/builder/qemu/step_convert_disk.go +++ b/builder/qemu/step_convert_disk.go @@ -7,7 +7,6 @@ import ( "strings" "time" - "github.com/hashicorp/packer/common" "github.com/hashicorp/packer/common/retry" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" @@ -62,12 +61,13 @@ func (s *stepConvertDisk) Run(ctx context.Context, state multistep.StateBag) mul }) if err != nil { - if err == common.RetryExhaustedError { + switch err.(type) { + case *retry.RetryExhaustedError: err = fmt.Errorf("Exhausted retries for getting file lock: %s", err) state.Put("error", err) ui.Error(err.Error()) return multistep.ActionHalt - } else { + default: err := fmt.Errorf("Error converting hard drive: %s", err) state.Put("error", err) ui.Error(err.Error()) From 94a660147e5e2e9a1350cbdda7397d85cd30af17 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Mon, 16 Nov 2020 11:49:33 -0800 Subject: [PATCH 3/3] rename retry so it doesn't stutter --- builder/osc/common/retry/retry.go | 4 ++-- builder/osc/common/retry/retry_test.go | 8 ++++---- builder/osc/common/state.go | 2 +- builder/osc/common/step_create_tags.go | 2 +- builder/osc/common/step_run_source_vm.go | 2 +- builder/osc/common/step_stop_bsu_backed_vm.go | 2 +- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/builder/osc/common/retry/retry.go b/builder/osc/common/retry/retry.go index 628a86a69..531000335 100644 --- a/builder/osc/common/retry/retry.go +++ b/builder/osc/common/retry/retry.go @@ -15,7 +15,7 @@ var RetryExhaustedError error = fmt.Errorf("Function never succeeded in Retry") type RetryableFunc func(uint) (bool, error) /* -Retry retries a function up to numTries times with exponential backoff. +Run retries a function up to numTries times with exponential backoff. If numTries == 0, retry indefinitely. If interval == 0, Retry will not delay retrying and there will be no exponential backoff. @@ -24,7 +24,7 @@ Intervals are in seconds. Returns an error if initial > max intervals, if retries are exhausted, or if the passed function returns an error. */ -func Retry(initialInterval float64, maxInterval float64, numTries uint, function RetryableFunc) error { +func Run(initialInterval float64, maxInterval float64, numTries uint, function RetryableFunc) error { if maxInterval == 0 { maxInterval = math.Inf(1) } else if initialInterval < 0 || initialInterval > maxInterval { diff --git a/builder/osc/common/retry/retry_test.go b/builder/osc/common/retry/retry_test.go index bd59cb459..9936b2652 100644 --- a/builder/osc/common/retry/retry_test.go +++ b/builder/osc/common/retry/retry_test.go @@ -8,7 +8,7 @@ import ( func TestRetry(t *testing.T) { numTries := uint(0) // Test that a passing function only gets called once. - err := Retry(0, 0, 0, func(i uint) (bool, error) { + err := Run(0, 0, 0, func(i uint) (bool, error) { numTries++ return true, nil }) @@ -22,7 +22,7 @@ func TestRetry(t *testing.T) { // Test that a failing function gets retried (once in this example). numTries = 0 results := []bool{false, true} - err = Retry(0, 0, 0, func(i uint) (bool, error) { + err = Run(0, 0, 0, func(i uint) (bool, error) { result := results[numTries] numTries++ return result, nil @@ -37,7 +37,7 @@ func TestRetry(t *testing.T) { // Test that a function error gets returned, and the function does not get called again. numTries = 0 funcErr := fmt.Errorf("This function had an error!") - err = Retry(0, 0, 0, func(i uint) (bool, error) { + err = Run(0, 0, 0, func(i uint) (bool, error) { numTries++ return false, funcErr }) @@ -51,7 +51,7 @@ func TestRetry(t *testing.T) { // Test when a function exhausts its retries. numTries = 0 expectedTries := uint(3) - err = Retry(0, 0, expectedTries, func(i uint) (bool, error) { + err = Run(0, 0, expectedTries, func(i uint) (bool, error) { numTries++ return false, nil }) diff --git a/builder/osc/common/state.go b/builder/osc/common/state.go index 50afe4cce..302df6d24 100644 --- a/builder/osc/common/state.go +++ b/builder/osc/common/state.go @@ -68,7 +68,7 @@ func WaitUntilOscSnapshotDone(conn *osc.APIClient, snapshotID string) error { } func waitForState(errCh chan<- error, target string, refresh stateRefreshFunc) { - err := retry.Retry(2, 2, 0, func(_ uint) (bool, error) { + err := retry.Run(2, 2, 0, func(_ uint) (bool, error) { state, err := refresh() if err != nil { return false, err diff --git a/builder/osc/common/step_create_tags.go b/builder/osc/common/step_create_tags.go index 2a2c60961..49f784de8 100644 --- a/builder/osc/common/step_create_tags.go +++ b/builder/osc/common/step_create_tags.go @@ -90,7 +90,7 @@ func (s *StepCreateTags) Run(_ context.Context, state multistep.StateBag) multis snapshotTags.Report(ui) // Retry creating tags for about 2.5 minutes - err = retry.Retry(0.2, 30, 11, func(_ uint) (bool, error) { + err = retry.Run(0.2, 30, 11, func(_ uint) (bool, error) { // Tag images and snapshots _, _, err := regionconn.TagApi.CreateTags(context.Background(), &osc.CreateTagsOpts{ CreateTagsRequest: optional.NewInterface(osc.CreateTagsRequest{ diff --git a/builder/osc/common/step_run_source_vm.go b/builder/osc/common/step_run_source_vm.go index 0d0cb6054..d5ba558c6 100644 --- a/builder/osc/common/step_run_source_vm.go +++ b/builder/osc/common/step_run_source_vm.go @@ -234,7 +234,7 @@ func (s *StepRunSourceVm) Run(ctx context.Context, state multistep.StateBag) mul if s.IsRestricted { oscTags.Report(ui) // Retry creating tags for about 2.5 minutes - err = retry.Retry(0.2, 30, 11, func(_ uint) (bool, error) { + err = retry.Run(0.2, 30, 11, func(_ uint) (bool, error) { _, _, err := oscconn.TagApi.CreateTags(context.Background(), &osc.CreateTagsOpts{ CreateTagsRequest: optional.NewInterface(osc.CreateTagsRequest{ Tags: oscTags, diff --git a/builder/osc/common/step_stop_bsu_backed_vm.go b/builder/osc/common/step_stop_bsu_backed_vm.go index 62cd830ac..11dd2654c 100644 --- a/builder/osc/common/step_stop_bsu_backed_vm.go +++ b/builder/osc/common/step_stop_bsu_backed_vm.go @@ -41,7 +41,7 @@ func (s *StepStopBSUBackedVm) Run(ctx context.Context, state multistep.StateBag) // does not exist. // Work around this by retrying a few times, up to about 5 minutes. - err := retry.Retry(10, 60, 6, func(i uint) (bool, error) { + err := retry.Run(10, 60, 6, func(i uint) (bool, error) { ui.Message(fmt.Sprintf("Stopping vm, attempt %d", i+1)) _, _, err = oscconn.VmApi.StopVms(context.Background(), &osc.StopVmsOpts{