From 8e92dddb5519a4cf72b84ac0cf460915bd36fb6b Mon Sep 17 00:00:00 2001 From: sagnik3788 Date: Sun, 14 Jul 2024 23:49:52 +0530 Subject: [PATCH 1/4] Handle SIGINT to abort hanging queries in ValidateBuilderExists Signed-off-by: sagnik3788 --- internal/commands/config_default_builder.go | 52 ++++++++++++++----- .../commands/config_default_builder_test.go | 31 +++++++++++ 2 files changed, 69 insertions(+), 14 deletions(-) diff --git a/internal/commands/config_default_builder.go b/internal/commands/config_default_builder.go index 111eebea70..c35622ec17 100644 --- a/internal/commands/config_default_builder.go +++ b/internal/commands/config_default_builder.go @@ -2,6 +2,10 @@ package commands import ( "fmt" + "os" + "os/signal" + "syscall" + "time" "github.com/pkg/errors" "github.com/spf13/cobra" @@ -48,7 +52,7 @@ func ConfigDefaultBuilder(logger logging.Logger, cfg config.Config, cfgPath stri return nil default: imageName := args[0] - if err := validateBuilderExists(logger, imageName, client); err != nil { + if err := ValidateBuilderExists(logger, imageName, client); err != nil { return errors.Wrapf(err, "validating that builder %s exists", style.Symbol(imageName)) } @@ -68,24 +72,44 @@ func ConfigDefaultBuilder(logger logging.Logger, cfg config.Config, cfgPath stri return cmd } -func validateBuilderExists(logger logging.Logger, imageName string, client PackClient) error { - logger.Debug("Verifying local image...") - info, err := client.InspectBuilder(imageName, true) - if err != nil { - return err - } +func ValidateBuilderExists(logger logging.Logger, imageName string, client PackClient) error { + + sigChan := make(chan os.Signal, 1) + signal.Notify(sigChan, syscall.SIGINT, syscall.SIGTERM) - if info == nil { - logger.Debug("Verifying remote image...") - info, err := client.InspectBuilder(imageName, false) + resultChan := make(chan error, 1) + + go func() { + logger.Debug("Verifying local image...") + info, err := client.InspectBuilder(imageName, true) if err != nil { - return errors.Wrapf(err, "failed to inspect remote image %s", style.Symbol(imageName)) + resultChan <- err + return } if info == nil { - return fmt.Errorf("builder %s not found", style.Symbol(imageName)) + logger.Debug("Verifying remote image...") + info, err = client.InspectBuilder(imageName, false) + if err != nil { + resultChan <- errors.Wrapf(err, "failed to inspect remote image %s", style.Symbol(imageName)) + return + } + + if info == nil { + resultChan <- fmt.Errorf("builder %s not found", style.Symbol(imageName)) + return + } } - } - return nil + resultChan <- nil + }() + + select { + case err := <-resultChan: + return err + case <-sigChan: + return fmt.Errorf("operation aborted") + case <-time.After(30 * time.Second): + return fmt.Errorf("operation timed out") + } } diff --git a/internal/commands/config_default_builder_test.go b/internal/commands/config_default_builder_test.go index e06cde74b7..7132b22a63 100644 --- a/internal/commands/config_default_builder_test.go +++ b/internal/commands/config_default_builder_test.go @@ -5,7 +5,9 @@ import ( "fmt" "os" "path/filepath" + "syscall" "testing" + "time" "github.com/golang/mock/gomock" "github.com/heroku/color" @@ -197,6 +199,35 @@ func testConfigDefaultBuilder(t *testing.T, when spec.G, it spec.S) { h.AssertContains(t, outBuf.String(), "builder 'nonexisting/image' not found") }) }) + + when("SIGINT is received during query", func() { + it("aborts the operation", func() { + imageName := "myremotehub.com/buildpacks/builder:latest" + + mockClient.EXPECT().InspectBuilder(imageName, true, gomock.Any()).DoAndReturn(func(string, bool, ...client.BuilderInspectionModifier) (*client.BuilderInfo, error) { + time.Sleep(5 * time.Second) + return nil, nil + }) + + done := make(chan error, 1) + go func() { + cmd.SetArgs([]string{imageName}) + done <- cmd.Execute() + }() + + time.Sleep(2 * time.Second) + + p, err := os.FindProcess(os.Getpid()) + h.AssertNil(t, err) + p.Signal(syscall.SIGINT) + + err = <-done + + h.AssertError(t, err, "operation aborted") + h.AssertContains(t, outBuf.String(), "operation aborted") + }) + }) + }) }) } From 909776ce1ec1e82dd203135469b67efdd14c1398 Mon Sep 17 00:00:00 2001 From: sagnik3788 Date: Mon, 15 Jul 2024 00:21:54 +0530 Subject: [PATCH 2/4] update Signed-off-by: sagnik3788 --- internal/commands/config_default_builder.go | 1 - internal/commands/config_default_builder_test.go | 1 - 2 files changed, 2 deletions(-) diff --git a/internal/commands/config_default_builder.go b/internal/commands/config_default_builder.go index c35622ec17..c9b3c6cca0 100644 --- a/internal/commands/config_default_builder.go +++ b/internal/commands/config_default_builder.go @@ -73,7 +73,6 @@ func ConfigDefaultBuilder(logger logging.Logger, cfg config.Config, cfgPath stri } func ValidateBuilderExists(logger logging.Logger, imageName string, client PackClient) error { - sigChan := make(chan os.Signal, 1) signal.Notify(sigChan, syscall.SIGINT, syscall.SIGTERM) diff --git a/internal/commands/config_default_builder_test.go b/internal/commands/config_default_builder_test.go index 7132b22a63..b455de081c 100644 --- a/internal/commands/config_default_builder_test.go +++ b/internal/commands/config_default_builder_test.go @@ -227,7 +227,6 @@ func testConfigDefaultBuilder(t *testing.T, when spec.G, it spec.S) { h.AssertContains(t, outBuf.String(), "operation aborted") }) }) - }) }) } From 101a48f49fcbaad5d8b08fef966fc3d07577b3f3 Mon Sep 17 00:00:00 2001 From: sagnik3788 Date: Mon, 15 Jul 2024 12:20:14 +0530 Subject: [PATCH 3/4] fix errors Signed-off-by: sagnik3788 --- internal/commands/config_default_builder_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/commands/config_default_builder_test.go b/internal/commands/config_default_builder_test.go index b455de081c..671cd47c33 100644 --- a/internal/commands/config_default_builder_test.go +++ b/internal/commands/config_default_builder_test.go @@ -218,8 +218,9 @@ func testConfigDefaultBuilder(t *testing.T, when spec.G, it spec.S) { time.Sleep(2 * time.Second) p, err := os.FindProcess(os.Getpid()) + h.AssertNil(t, err) + err = p.Signal(syscall.SIGINT) h.AssertNil(t, err) - p.Signal(syscall.SIGINT) err = <-done From c0ee9875ccdf9b47db32adafea81f81c20492b11 Mon Sep 17 00:00:00 2001 From: sagnik3788 Date: Mon, 15 Jul 2024 12:26:59 +0530 Subject: [PATCH 4/4] update Signed-off-by: sagnik3788 --- internal/commands/config_default_builder_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/commands/config_default_builder_test.go b/internal/commands/config_default_builder_test.go index 671cd47c33..e21772af64 100644 --- a/internal/commands/config_default_builder_test.go +++ b/internal/commands/config_default_builder_test.go @@ -218,7 +218,7 @@ func testConfigDefaultBuilder(t *testing.T, when spec.G, it spec.S) { time.Sleep(2 * time.Second) p, err := os.FindProcess(os.Getpid()) - h.AssertNil(t, err) + h.AssertNil(t, err) err = p.Signal(syscall.SIGINT) h.AssertNil(t, err)