From 94d8539e3de8113c8bf6d2c1ba8a170339fb1ca6 Mon Sep 17 00:00:00 2001 From: Rebecca Mahany-Horton Date: Tue, 1 Aug 2023 10:05:57 -0400 Subject: [PATCH 1/4] TraceExporter Execute must exit before launcher can exit --- pkg/traces/exporter/exporter.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pkg/traces/exporter/exporter.go b/pkg/traces/exporter/exporter.go index 3e976f537..d55b5c66c 100644 --- a/pkg/traces/exporter/exporter.go +++ b/pkg/traces/exporter/exporter.go @@ -56,6 +56,7 @@ type TraceExporter struct { disableIngestTLS bool enabled bool traceSamplingRate float64 + interrupt chan struct{} } // NewTraceExporter sets up our traces to be exported via OTLP over HTTP. @@ -86,6 +87,7 @@ func NewTraceExporter(ctx context.Context, k types.Knapsack, client osquery.Quer disableIngestTLS: k.DisableTraceIngestTLS(), enabled: k.ExportTraces(), traceSamplingRate: k.TraceSamplingRate(), + interrupt: make(chan struct{}), } // Observe ExportTraces and IngestServerURL changes to know when to start/stop exporting, and where @@ -248,8 +250,7 @@ func (t *TraceExporter) setNewGlobalProvider() { // Execute is a no-op -- the exporter is already running in the background. The TraceExporter // otherwise only responds to control server events. func (t *TraceExporter) Execute() error { - // Does nothing, just waiting for launcher to exit - <-context.Background().Done() + <-t.interrupt return nil } @@ -257,6 +258,8 @@ func (t *TraceExporter) Interrupt(_ error) { if t.provider != nil { t.provider.Shutdown(context.Background()) } + + t.interrupt <- struct{}{} } // Update satisfies control.subscriber interface -- looks at changes to the `observability_ingest` subsystem, From 7542d62bb047693a284cea4838eabb1017e8151f Mon Sep 17 00:00:00 2001 From: Rebecca Mahany-Horton Date: Tue, 1 Aug 2023 10:06:11 -0400 Subject: [PATCH 2/4] Sendbuffer Run must exit --- pkg/sendbuffer/sendbuffer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/sendbuffer/sendbuffer.go b/pkg/sendbuffer/sendbuffer.go index 5b2fcc47f..09cec52ca 100644 --- a/pkg/sendbuffer/sendbuffer.go +++ b/pkg/sendbuffer/sendbuffer.go @@ -141,7 +141,7 @@ func (sb *SendBuffer) Run(ctx context.Context) error { case <-ticker.C: continue case <-ctx.Done(): - break + return nil } } } From 7841f8cfe998b83c0f89f9d39765ba0870a0a444 Mon Sep 17 00:00:00 2001 From: Rebecca Mahany-Horton Date: Tue, 1 Aug 2023 12:10:52 -0400 Subject: [PATCH 3/4] Ensure Updater ctx is cancelled so that actor Execute will exit --- cmd/launcher/internal/updater/updater.go | 6 ++++++ cmd/launcher/internal/updater/updater_test.go | 5 +++++ 2 files changed, 11 insertions(+) diff --git a/cmd/launcher/internal/updater/updater.go b/cmd/launcher/internal/updater/updater.go index 6e293660d..660816bac 100644 --- a/cmd/launcher/internal/updater/updater.go +++ b/cmd/launcher/internal/updater/updater.go @@ -62,9 +62,12 @@ func NewUpdater( return nil, err } + ctx, cancel := context.WithCancel(ctx) + updateCmd := &updaterCmd{ updater: updater, ctx: ctx, + cancel: cancel, stopChan: make(chan bool), config: config, runUpdaterRetryInterval: 30 * time.Minute, @@ -84,6 +87,7 @@ type updater interface { type updaterCmd struct { updater updater ctx context.Context + cancel context.CancelFunc stopChan chan bool stopExecution func() config *UpdaterConfig @@ -155,4 +159,6 @@ func (u *updaterCmd) interrupt(err error) { if u.stopExecution != nil { u.stopExecution() } + + u.cancel() } diff --git a/cmd/launcher/internal/updater/updater_test.go b/cmd/launcher/internal/updater/updater_test.go index d9235f20d..8ba35f1f0 100644 --- a/cmd/launcher/internal/updater/updater_test.go +++ b/cmd/launcher/internal/updater/updater_test.go @@ -114,6 +114,7 @@ func Test_updaterCmd_execute(t *testing.T) { u := &updaterCmd{ updater: tt.fields.updater, ctx: ctx, + cancel: cancelCtx, stopChan: tt.fields.stopChan, config: tt.fields.config, runUpdaterRetryInterval: tt.fields.runUpdaterRetryInterval, @@ -194,9 +195,13 @@ func Test_updaterCmd_interrupt(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() + ctx, cancel := context.WithCancel(context.Background()) + u := &updaterCmd{ stopChan: tt.fields.stopChan, config: tt.fields.config, + ctx: ctx, + cancel: cancel, } // using this wait group to ensure that something gets received on u.StopChan From 7916e3f4dddbcc6e26791d43abdda4890e0a29ef Mon Sep 17 00:00:00 2001 From: Rebecca Mahany-Horton Date: Tue, 1 Aug 2023 12:32:51 -0400 Subject: [PATCH 4/4] Ensure TUF autoupdater finishes updates (when not performing download) quicker --- pkg/autoupdate/tuf/autoupdate.go | 12 ++++++------ pkg/autoupdate/tuf/library_manager.go | 6 ++++++ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/pkg/autoupdate/tuf/autoupdate.go b/pkg/autoupdate/tuf/autoupdate.go index 0aa30169d..6c1077dff 100644 --- a/pkg/autoupdate/tuf/autoupdate.go +++ b/pkg/autoupdate/tuf/autoupdate.go @@ -87,7 +87,7 @@ func NewTufAutoupdater(k types.Knapsack, metadataHttpClient *http.Client, mirror checkInterval: k.AutoupdateInterval(), store: k.AutoupdateErrorsStore(), osquerier: osquerier, - osquerierRetryInterval: 1 * time.Minute, + osquerierRetryInterval: 30 * time.Second, logger: log.NewNopLogger(), } @@ -184,10 +184,10 @@ func (ta *TufAutoupdater) Execute() (err error) { } func (ta *TufAutoupdater) Interrupt(_ error) { - ta.interrupt <- struct{}{} if err := ta.libraryManager.Close(); err != nil { level.Debug(ta.logger).Log("msg", "could not close library on interrupt", "err", err) } + ta.interrupt <- struct{}{} } // tidyLibrary gets the current running version for each binary (so that the current version is not removed) @@ -306,6 +306,10 @@ func (ta *TufAutoupdater) downloadUpdate(binary autoupdatableBinary, targets dat return "", fmt.Errorf("could not find release: %w", err) } + if ta.libraryManager.Available(binary, release) { + return "", nil + } + // Get the current running version if available -- don't error out if we can't // get it, since the worst case is that we download an update whose version matches // our install version. @@ -315,10 +319,6 @@ func (ta *TufAutoupdater) downloadUpdate(binary autoupdatableBinary, targets dat return "", nil } - if ta.libraryManager.Available(binary, release) { - return "", nil - } - if err := ta.libraryManager.AddToLibrary(binary, currentVersion, release, releaseMetadata); err != nil { return "", fmt.Errorf("could not add release %s for binary %s to library: %w", release, binary, err) } diff --git a/pkg/autoupdate/tuf/library_manager.go b/pkg/autoupdate/tuf/library_manager.go index 2ded06a4e..457cdd286 100644 --- a/pkg/autoupdate/tuf/library_manager.go +++ b/pkg/autoupdate/tuf/library_manager.go @@ -69,6 +69,12 @@ func newUpdateLibraryManager(mirrorUrl string, mirrorClient *http.Client, baseDi // Close cleans up the temporary staging directory func (ulm *updateLibraryManager) Close() error { + // Acquire lock to ensure we aren't interrupting an ongoing operation + for _, binary := range binaries { + ulm.lock.Lock(binary) + defer ulm.lock.Unlock(binary) + } + if err := os.RemoveAll(ulm.stagingDir); err != nil { return fmt.Errorf("could not remove staging dir %s: %w", ulm.stagingDir, err) }