Skip to content

Commit e68a336

Browse files
authored
Bump goutils, update pexec OUE handlers (#5235)
1 parent 2cb20cd commit e68a336

File tree

6 files changed

+93
-15
lines changed

6 files changed

+93
-15
lines changed

go.mod

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ require (
4242
github.com/google/go-cmp v0.7.0
4343
github.com/google/uuid v1.6.0
4444
github.com/grpc-ecosystem/go-grpc-middleware v1.4.0
45-
github.com/grpc-ecosystem/grpc-gateway/v2 v2.20.0
45+
github.com/grpc-ecosystem/grpc-gateway/v2 v2.26.3
4646
github.com/invopop/jsonschema v0.6.0
4747
github.com/jedib0t/go-pretty/v6 v6.4.6
4848
github.com/jhump/protoreflect v1.15.6
@@ -83,7 +83,7 @@ require (
8383
go.uber.org/zap v1.27.0
8484
go.viam.com/api v0.1.473
8585
go.viam.com/test v1.2.4
86-
go.viam.com/utils v0.1.163
86+
go.viam.com/utils v0.1.164
8787
goji.io v2.0.2+incompatible
8888
golang.org/x/image v0.25.0
8989
golang.org/x/mobile v0.0.0-20240112133503-c713f31d574b
@@ -422,9 +422,11 @@ require (
422422
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.54.0 // indirect
423423
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.54.0 // indirect
424424
go.opentelemetry.io/otel v1.37.0 // indirect
425+
go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.37.0 // indirect
425426
go.opentelemetry.io/otel/metric v1.37.0 // indirect
426427
go.opentelemetry.io/otel/sdk v1.37.0 // indirect
427428
go.opentelemetry.io/otel/trace v1.37.0 // indirect
429+
go.opentelemetry.io/proto/otlp v1.7.0 // indirect
428430
go.uber.org/automaxprocs v1.5.3 // indirect
429431
go.uber.org/goleak v1.3.0 // indirect
430432
go4.org/unsafe/assume-no-moving-gc v0.0.0-20230525183740-e7c30c78aeb2 // indirect

go.sum

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -709,8 +709,8 @@ github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0/go.mod h1:8NvIoxWQoOIhqOTXgf
709709
github.com/grpc-ecosystem/grpc-gateway v1.9.0/go.mod h1:vNeuVxBJEsws4ogUvrchl83t/GYV9WGTSLVdBhOQFDY=
710710
github.com/grpc-ecosystem/grpc-gateway v1.9.5/go.mod h1:vNeuVxBJEsws4ogUvrchl83t/GYV9WGTSLVdBhOQFDY=
711711
github.com/grpc-ecosystem/grpc-gateway v1.16.0/go.mod h1:BDjrQk3hbvj6Nolgz8mAMFbcEtjT1g+wF4CSlocrBnw=
712-
github.com/grpc-ecosystem/grpc-gateway/v2 v2.20.0 h1:bkypFPDjIYGfCYD5mRBvpqxfYX1YCS1PXdKYWi8FsN0=
713-
github.com/grpc-ecosystem/grpc-gateway/v2 v2.20.0/go.mod h1:P+Lt/0by1T8bfcF3z737NnSbmxQAppXMRziHUxPOC8k=
712+
github.com/grpc-ecosystem/grpc-gateway/v2 v2.26.3 h1:5ZPtiqj0JL5oKWmcsq4VMaAW5ukBEgSGXEN89zeH1Jo=
713+
github.com/grpc-ecosystem/grpc-gateway/v2 v2.26.3/go.mod h1:ndYquD05frm2vACXE1nsccT4oJzjhw2arTS2cpUD1PI=
714714
github.com/hashicorp/consul/api v1.1.0/go.mod h1:VmuI/Lkw1nC05EYQWNKwWGbkg+FbDBtguAZLlVdkD9Q=
715715
github.com/hashicorp/consul/api v1.3.0/go.mod h1:MmDNSzIMUjNpY/mQ398R4bk2FnqQLoPndWW5VkKPlCE=
716716
github.com/hashicorp/consul/sdk v0.1.1/go.mod h1:VKf9jXwCTEY1QZP2MOLRhb5i/I/ssyNV1vwHyQBF0x8=
@@ -1490,8 +1490,8 @@ go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.54.0 h1:TT4fX+n
14901490
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.54.0/go.mod h1:L7UH0GbB0p47T4Rri3uHjbpCFYrVrwc1I25QhNPiGK8=
14911491
go.opentelemetry.io/otel v1.37.0 h1:9zhNfelUvx0KBfu/gb+ZgeAfAgtWrfHJZcAqFC228wQ=
14921492
go.opentelemetry.io/otel v1.37.0/go.mod h1:ehE/umFRLnuLa/vSccNq9oS1ErUlkkK71gMcN34UG8I=
1493-
go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.32.0 h1:IJFEoHiytixx8cMiVAO+GmHR6Frwu+u5Ur8njpFO6Ac=
1494-
go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.32.0/go.mod h1:3rHrKNtLIoS0oZwkY2vxi+oJcwFRWdtUyRII+so45p8=
1493+
go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.37.0 h1:Ahq7pZmv87yiyn3jeFz/LekZmPLLdKejuO3NcK9MssM=
1494+
go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.37.0/go.mod h1:MJTqhM0im3mRLw1i8uGHnCvUEeS7VwRyxlLC78PA18M=
14951495
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.24.0 h1:Xw8U6u2f8DK2XAkGRFV7BBLENgnTGX9i4rQRxJf+/vs=
14961496
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.24.0/go.mod h1:6KW1Fm6R/s6Z3PGXwSJN2K4eT6wQB3vXX6CVnYX9NmM=
14971497
go.opentelemetry.io/otel/metric v1.37.0 h1:mvwbQS5m0tbmqML4NqK+e3aDiO02vsf/WgbsdpcPoZE=
@@ -1503,8 +1503,8 @@ go.opentelemetry.io/otel/sdk/metric v1.37.0/go.mod h1:cNen4ZWfiD37l5NhS+Keb5RXVW
15031503
go.opentelemetry.io/otel/trace v1.37.0 h1:HLdcFNbRQBE2imdSEgm/kwqmQj1Or1l/7bW6mxVK7z4=
15041504
go.opentelemetry.io/otel/trace v1.37.0/go.mod h1:TlgrlQ+PtQO5XFerSPUYG0JSgGyryXewPGyayAWSBS0=
15051505
go.opentelemetry.io/proto/otlp v0.7.0/go.mod h1:PqfVotwruBrMGOCsRd/89rSnXhoiJIqeYNgFYFoEGnI=
1506-
go.opentelemetry.io/proto/otlp v1.3.1 h1:TrMUixzpM0yuc/znrFTP9MMRh8trP93mkCiDVeXrui0=
1507-
go.opentelemetry.io/proto/otlp v1.3.1/go.mod h1:0X1WI4de4ZsLrrJNLAQbFeLCm3T7yBkR0XqQ7niQU+8=
1506+
go.opentelemetry.io/proto/otlp v1.7.0 h1:jX1VolD6nHuFzOYso2E73H85i92Mv8JQYk0K9vz09os=
1507+
go.opentelemetry.io/proto/otlp v1.7.0/go.mod h1:fSKjH6YJ7HDlwzltzyMj036AJ3ejJLCgCSHGj4efDDo=
15081508
go.uber.org/atomic v1.3.2/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE=
15091509
go.uber.org/atomic v1.4.0/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE=
15101510
go.uber.org/atomic v1.5.0/go.mod h1:sABNBOSYdrvTF6hTgEIbc7YasKWGhgEQZyfxyTvoXHQ=
@@ -1534,8 +1534,8 @@ go.viam.com/api v0.1.473 h1:Hy1JybY6b9lqO2WIfniQN6Mj5+1bmibTzsXkuqos41c=
15341534
go.viam.com/api v0.1.473/go.mod h1:p/am76zx8SZ74V/F4rEAYQIpHaaLUwJgY2q3Uw3FIWk=
15351535
go.viam.com/test v1.2.4 h1:JYgZhsuGAQ8sL9jWkziAXN9VJJiKbjoi9BsO33TW3ug=
15361536
go.viam.com/test v1.2.4/go.mod h1:zI2xzosHdqXAJ/kFqcN+OIF78kQuTV2nIhGZ8EzvaJI=
1537-
go.viam.com/utils v0.1.163 h1:5TcsGcnQ9UXgjIsLXL30fDv3fMg9Q6yhlUJ+54xIxG4=
1538-
go.viam.com/utils v0.1.163/go.mod h1:rX7xWH6QHAlROw4NTmErG1TlzEYYb6tu8gRHBOItTqw=
1537+
go.viam.com/utils v0.1.164 h1:EVKu5AuulD2m7V0OQyXb1YBEgvbHd4p/OwHkw8uT25Y=
1538+
go.viam.com/utils v0.1.164/go.mod h1:qZsilNWaD7Fd5WnTAI+s5SkWWxN9YLzImf7JwsB8JLg=
15391539
go4.org/unsafe/assume-no-moving-gc v0.0.0-20230525183740-e7c30c78aeb2 h1:WJhcL4p+YeDxmZWg141nRm7XC8IDmhz7lk5GpadO1Sg=
15401540
go4.org/unsafe/assume-no-moving-gc v0.0.0-20230525183740-e7c30c78aeb2/go.mod h1:FftLjUGFEDu5k8lt0ddY+HcrH/qU/0qk+H8j9/nTl3E=
15411541
gocv.io/x/gocv v0.25.0/go.mod h1:Rar2PS6DV+T4FL+PM535EImD/h13hGVaHhnCu1xarBs=

module/modmanager/manager.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -817,7 +817,7 @@ var oueRestartInterval = 5 * time.Second
817817
// newOnUnexpectedExitHandler returns the appropriate OnUnexpectedExit function
818818
// for the passed-in module to include in the pexec.ProcessConfig.
819819
func (mgr *Manager) newOnUnexpectedExitHandler(ctx context.Context, mod *module) pexec.UnexpectedExitHandler {
820-
return func(exitCode int) (continueAttemptingRestart bool) {
820+
return func(oueCtx context.Context, exitCode int) (continueAttemptingRestart bool) {
821821
// Log error immediately, as this is unexpected behavior.
822822
mod.logger.Errorw(
823823
"Module has unexpectedly exited.", "module", mod.cfg.Name, "exit_code", exitCode,
@@ -863,6 +863,10 @@ func (mgr *Manager) newOnUnexpectedExitHandler(ctx context.Context, mod *module)
863863
mod.logger.Infow("Restart context canceled, abandoning restart attempt", "err", err)
864864
return
865865
}
866+
if err := oueCtx.Err(); err != nil {
867+
mod.logger.Infow("pexec context canceled, abandoning restart attempt", "err", err)
868+
return
869+
}
866870

867871
if !cleanupPerformed {
868872
mod.cleanupAfterCrash(mgr)
@@ -971,12 +975,12 @@ func (mgr *Manager) attemptRestart(ctx context.Context, mod *module) error {
971975
// continues with the normal OUE execution on success.
972976
blockRestart := make(chan struct{})
973977
defer close(blockRestart)
974-
oue := func(exitCode int) bool {
978+
oue := func(oueCtx context.Context, exitCode int) bool {
975979
<-blockRestart
976980
if !success {
977981
return false
978982
}
979-
return mgr.newOnUnexpectedExitHandler(ctx, mod)(exitCode)
983+
return mgr.newOnUnexpectedExitHandler(ctx, mod)(oueCtx, exitCode)
980984
}
981985

982986
if err := mgr.startModuleProcess(mod, oue); err != nil {

module/modmanager/manager_test.go

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -741,6 +741,78 @@ func TestModuleReloading(t *testing.T) {
741741
err = mod.process.Stop()
742742
test.That(t, err, test.ShouldBeNil)
743743
})
744+
t.Run("do not restart module if pexec context is cancelled", func(t *testing.T) {
745+
// Lower restart interval so the test runs faster
746+
originalInterval := oueRestartInterval
747+
t.Cleanup(func() {
748+
oueRestartInterval = originalInterval
749+
})
750+
oueRestartInterval = 50 * time.Millisecond
751+
752+
logger, logs := logging.NewObservedTestLogger(t)
753+
754+
// Precompile module to avoid timeout issues when building takes too long.
755+
modCfg.ExePath = rtestutils.BuildTempModule(t, "module/testmodule")
756+
757+
// This test neither uses a resource manager nor asserts anything about
758+
// the existence of resources in the graph. Use a dummy
759+
// HandleOrphanedResources function so orphaned resource logic does not
760+
// panic.
761+
var dummyHandleOrphanedResourcesCallCount atomic.Uint64
762+
dummyHandleOrphanedResources := func(context.Context, []resource.Name) {
763+
dummyHandleOrphanedResourcesCallCount.Add(1)
764+
}
765+
mgr := setupModManager(t, ctx, parentAddr, logger, modmanageroptions.Options{
766+
UntrustedEnv: false,
767+
HandleOrphanedResources: dummyHandleOrphanedResources,
768+
})
769+
err = mgr.Add(ctx, modCfg)
770+
test.That(t, err, test.ShouldBeNil)
771+
772+
// Add helper resource and ensure "echo" works correctly.
773+
h, err := mgr.AddResource(ctx, cfgMyHelper, nil)
774+
test.That(t, err, test.ShouldBeNil)
775+
ok := mgr.IsModularResource(rNameMyHelper)
776+
test.That(t, ok, test.ShouldBeTrue)
777+
778+
// Remove testmodule binary, so process cannot be successfully restarted
779+
// after crash.
780+
err = os.Remove(modCfg.ExePath)
781+
test.That(t, err, test.ShouldBeNil)
782+
783+
// Grab a copy of the original managed process before the modmanager
784+
// overwrites it during restart attempts.
785+
mod, ok := mgr.modules.Load(modCfg.Name)
786+
test.That(t, ok, test.ShouldBeTrue)
787+
modProc := mod.process
788+
789+
// Run 'kill_module' command through helper resource to cause module to
790+
// exit with error. Assert that we do not restart the module if context is cancelled.
791+
_, err = h.DoCommand(ctx, map[string]interface{}{"command": "kill_module"})
792+
test.That(t, err, test.ShouldNotBeNil)
793+
test.That(t, err.Error(), test.ShouldContainSubstring, "rpc error")
794+
795+
testutils.WaitForAssertion(t, func(tb testing.TB) {
796+
tb.Helper()
797+
test.That(tb, logs.FilterMessageSnippet("Module has unexpectedly exited.").Len(),
798+
test.ShouldEqual, 1)
799+
})
800+
801+
ok = mgr.IsModularResource(rNameMyHelper)
802+
test.That(t, ok, test.ShouldBeTrue)
803+
_, err = h.DoCommand(ctx, map[string]interface{}{"command": "echo"})
804+
test.That(t, err, test.ShouldNotBeNil)
805+
806+
// Stop the module managed process manually and make sure the OUE handler
807+
// stops trying to restart it.
808+
err = modProc.Stop()
809+
test.That(t, err, test.ShouldBeNil)
810+
testutils.WaitForAssertion(t, func(tb testing.TB) {
811+
tb.Helper()
812+
matching := logs.FilterMessageSnippet("pexec context canceled, abandoning restart attempt").Len()
813+
test.That(tb, matching, test.ShouldEqual, 1)
814+
})
815+
})
744816
t.Run("timed out module process is stopped", func(t *testing.T) {
745817
logger, logs := logging.NewObservedTestLogger(t)
746818

module/modmanager/module.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ func (m *module) tcpMode() bool {
158158
func (m *module) startProcess(
159159
ctx context.Context,
160160
parentAddr string,
161-
oue func(int) bool,
161+
oue pexec.UnexpectedExitHandler,
162162
viamHomeDir string,
163163
packagesDir string,
164164
) error {

testutils/robottestutils/robot_utils.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ type ServerProcessOption func(*pexec.ProcessConfig)
101101
// WithoutRestart disables the automatic restart of the pexec library for the server process.
102102
func WithoutRestart() ServerProcessOption {
103103
return func(cfg *pexec.ProcessConfig) {
104-
cfg.OnUnexpectedExit = func(int) bool { return false }
104+
cfg.OnUnexpectedExit = func(context.Context, int) bool { return false }
105105
}
106106
}
107107

0 commit comments

Comments
 (0)