Remove mixer/clock due to bug (#1926)

kolide · Oct 30, 2024 · 9ae6142 · 9ae6142
1 parent 833363a
commit 9ae6142
Show file tree

Hide file tree

Showing 4 changed files with 19 additions and 38 deletions.
diff --git a/go.mod b/go.mod
@@ -18,7 +18,6 @@ require (
 	github.com/kolide/krypto v0.1.1-0.20231229162826-db516b7e0121
 	github.com/mat/besticon v3.9.0+incompatible
 	github.com/mattn/go-sqlite3 v1.14.19
-	github.com/mixer/clock v0.0.0-20170901150240-b08e6b4da7ea
 	github.com/nfnt/resize v0.0.0-20180221191011-83c6a9932646
 	github.com/osquery/osquery-go v0.0.0-20231130195733-61ac79279aaa
 	github.com/peterbourgon/ff/v3 v3.1.2

diff --git a/go.sum b/go.sum
@@ -205,8 +205,6 @@ github.com/mattn/go-sqlite3 v1.14.19/go.mod h1:2eHXhiwb8IkHr+BDWZGa96P6+rkvnG63S
 github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0=
 github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0=
 github.com/mitchellh/mapstructure v1.1.2/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y=
-github.com/mixer/clock v0.0.0-20170901150240-b08e6b4da7ea h1:eWwD4TJbXXLj8Ay2KQ1F3lh1YtgpcWZL3ZZN2sKvSDg=
-github.com/mixer/clock v0.0.0-20170901150240-b08e6b4da7ea/go.mod h1:U8TDygO2XZh1RtBCgX7oRbJ7gmSH4C6FROsBdQ6QyCc=
 github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U=
 github.com/nfnt/resize v0.0.0-20180221191011-83c6a9932646 h1:zYyBkD/k9seD2A7fsi6Oo2LfFZAehjjQMERAvZLEDnQ=
 github.com/nfnt/resize v0.0.0-20180221191011-83c6a9932646/go.mod h1:jpp1/29i3P1S/RLdc7JQKbRpFeM1dOBd8T9ki5s+AY8=

diff --git a/pkg/osquery/extension.go b/pkg/osquery/extension.go
@@ -21,7 +21,6 @@ import (
 	"github.com/kolide/launcher/pkg/osquery/runtime/history"
 	"github.com/kolide/launcher/pkg/service"
 	"github.com/kolide/launcher/pkg/traces"
-	"github.com/mixer/clock"
 	"github.com/osquery/osquery-go/plugin/distributed"
 	"github.com/osquery/osquery-go/plugin/logger"
 	"github.com/pkg/errors"
@@ -81,10 +80,6 @@ type ExtensionOpts struct {
 	// LoggingInterval is the interval at which logs should be flushed to
 	// the server.
 	LoggingInterval time.Duration
-	// Clock is the clock that should be used for time based operations. By
-	// default it will be a normal realtime clock, but a mock clock can be
-	// passed with clock.NewMockClock() for testing purposes.
-	Clock clock.Clock
 	// MaxBufferedLogs is the maximum number of logs to buffer before
 	// purging oldest logs (applies per log type).
 	MaxBufferedLogs int
@@ -116,10 +111,6 @@ func NewExtension(ctx context.Context, client service.KolideService, k types.Kna
 		opts.LoggingInterval = defaultLoggingInterval
 	}
 
-	if opts.Clock == nil {
-		opts.Clock = clock.DefaultClock{}
-	}
-
 	if opts.MaxBufferedLogs == 0 {
 		opts.MaxBufferedLogs = defaultMaxBufferedLogs
 	}
@@ -156,7 +147,7 @@ func NewExtension(ctx context.Context, client service.KolideService, k types.Kna
 
 func (e *Extension) Execute() error {
 	// Process logs until shutdown
-	ticker := e.Opts.Clock.NewTicker(e.Opts.LoggingInterval)
+	ticker := time.NewTicker(e.Opts.LoggingInterval)
 	defer ticker.Stop()
 	for {
 		e.writeAndPurgeLogs()
@@ -168,7 +159,7 @@ func (e *Extension) Execute() error {
 				"osquery extension received shutdown request",
 			)
 			return nil
-		case <-ticker.Chan():
+		case <-ticker.C:
 			// Resume loop
 		}
 	}

diff --git a/pkg/osquery/extension_test.go b/pkg/osquery/extension_test.go
@@ -12,6 +12,7 @@ import (
 	"fmt"
 	"os"
 	"sort"
+	"sync"
 	"testing"
 	"testing/quick"
 	"time"
@@ -25,7 +26,6 @@ import (
 	"github.com/kolide/launcher/pkg/log/multislogger"
 	"github.com/kolide/launcher/pkg/service"
 	"github.com/kolide/launcher/pkg/service/mock"
-	"github.com/mixer/clock"
 	"github.com/osquery/osquery-go/plugin/distributed"
 	"github.com/osquery/osquery-go/plugin/logger"
 	"github.com/stretchr/testify/assert"
@@ -808,18 +808,19 @@ func TestExtensionWriteBufferedLogsDropsBigLog(t *testing.T) {
 
 func TestExtensionWriteLogsLoop(t *testing.T) {
 	var gotStatusLogs, gotResultLogs []string
-	var funcInvokedStatus, funcInvokedResult bool
+	var logLock sync.Mutex
 	var done = make(chan struct{})
 	m := &mock.KolideService{
 		PublishLogsFunc: func(ctx context.Context, nodeKey string, logType logger.LogType, logs []string) (string, string, bool, error) {
 			defer func() { done <- struct{}{} }()
 
+			logLock.Lock()
+			defer logLock.Unlock()
+
 			switch logType {
 			case logger.LogTypeStatus:
-				funcInvokedStatus = true
 				gotStatusLogs = logs
 			case logger.LogTypeString:
-				funcInvokedResult = true
 				gotResultLogs = logs
 			default:
 				t.Error("Unknown log type")
@@ -840,11 +841,9 @@ func TestExtensionWriteLogsLoop(t *testing.T) {
 	k.On("StatusLogsStore").Return(statusLogsStore)
 	k.On("ResultLogsStore").Return(resultLogsStore)
 
-	mockClock := clock.NewMockClock()
-	expectedLoggingInterval := 10 * time.Second
+	expectedLoggingInterval := 5 * time.Second
 	e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{
 		MaxBytesPerBatch: 200,
-		Clock:            mockClock,
 		LoggingInterval:  expectedLoggingInterval,
 	})
 	require.Nil(t, err)
@@ -868,45 +867,39 @@ func TestExtensionWriteLogsLoop(t *testing.T) {
 		<-done
 		<-done
 	})
-	assert.True(t, funcInvokedStatus)
-	assert.True(t, funcInvokedResult)
-	assert.Nil(t, err)
+	// Examine the logs, then reset them for the next test
+	logLock.Lock()
 	assert.Equal(t, expectedStatusLogs[:10], gotStatusLogs)
 	assert.Equal(t, expectedResultLogs[:10], gotResultLogs)
-
-	funcInvokedStatus = false
-	funcInvokedResult = false
 	gotStatusLogs = nil
 	gotResultLogs = nil
+	logLock.Unlock()
 
 	// Should write last 10 logs
-	mockClock.AddTime(expectedLoggingInterval + 1)
+	time.Sleep(expectedLoggingInterval + 1*time.Second)
 	testutil.FatalAfterFunc(t, 1*time.Second, func() {
 		// PublishLogsFunc runs twice of each run of the loop
 		<-done
 		<-done
 	})
-	assert.True(t, funcInvokedStatus)
-	assert.True(t, funcInvokedResult)
-	assert.Nil(t, err)
+	// Examine the logs, then reset them for the next test
+	logLock.Lock()
 	assert.Equal(t, expectedStatusLogs[10:], gotStatusLogs)
 	assert.Equal(t, expectedResultLogs[10:], gotResultLogs)
-
-	funcInvokedStatus = false
-	funcInvokedResult = false
 	gotStatusLogs = nil
 	gotResultLogs = nil
+	logLock.Unlock()
 
 	// No more logs to write
-	mockClock.AddTime(expectedLoggingInterval + 1)
+	time.Sleep(expectedLoggingInterval + 1*time.Second)
 	// Block to ensure publish function could be called if the logic is
 	// incorrect
 	time.Sleep(1 * time.Millisecond)
-	assert.False(t, funcInvokedStatus)
-	assert.False(t, funcInvokedResult)
-	assert.Nil(t, err)
+	// Confirm logs are nil because nothing got published
+	logLock.Lock()
 	assert.Nil(t, gotStatusLogs)
 	assert.Nil(t, gotResultLogs)
+	logLock.Unlock()
 
 	testutil.FatalAfterFunc(t, 3*time.Second, func() {
 		e.Shutdown(errors.New("test error"))