Skip to content

Commit

Permalink
logs: replace our custom flush daemon with klog's daemon
Browse files Browse the repository at this point in the history
The advantage is that klog properly handles restarting of the daemon
with a new interval and the daemon can be stopped.

Stopping the daemon solves a data race that the tests had when modifying the
Logger's flush function while goroutines from previous tests were still
running.
  • Loading branch information
pohly committed Mar 21, 2022
1 parent f8bb67c commit 0b7d303
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 4 deletions.
3 changes: 1 addition & 2 deletions staging/src/k8s.io/component-base/logs/logs.go
Expand Up @@ -26,7 +26,6 @@ import (
"time"

"github.com/spf13/pflag"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/klog/v2"
)

Expand Down Expand Up @@ -175,7 +174,7 @@ func InitLogs() {
if logFlushFreqAdded {
// The flag from this file was activated, so use it now.
// Otherwise LoggingConfiguration.Apply will do this.
go wait.Forever(FlushLogs, logFlushFreq)
klog.StartFlushDaemon(logFlushFreq)
}
}

Expand Down
3 changes: 1 addition & 2 deletions staging/src/k8s.io/component-base/logs/options.go
Expand Up @@ -22,7 +22,6 @@ import (
"github.com/spf13/pflag"

utilerrors "k8s.io/apimachinery/pkg/util/errors"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/component-base/config"
"k8s.io/component-base/config/v1alpha1"
"k8s.io/component-base/logs/registry"
Expand Down Expand Up @@ -90,5 +89,5 @@ func (o *Options) apply() {
if err := loggingFlags.Lookup("vmodule").Value.Set(o.Config.VModule.String()); err != nil {
panic(fmt.Errorf("internal error while setting klog vmodule: %v", err))
}
go wait.Forever(FlushLogs, o.Config.FlushFrequency)
klog.StartFlushDaemon(o.Config.FlushFrequency)
}
2 changes: 2 additions & 0 deletions staging/src/k8s.io/component-base/logs/options_test.go
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/stretchr/testify/assert"

"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/klog/v2"
)

func TestFlags(t *testing.T) {
Expand Down Expand Up @@ -89,6 +90,7 @@ func TestOptions(t *testing.T) {
t.Errorf("Wrong Validate() result for %q. expect %v, got %v", tc.name, tc.want, o)
}
err := o.ValidateAndApply()
defer klog.StopFlushDaemon()

if !assert.ElementsMatch(t, tc.errs.ToAggregate(), err) {
t.Errorf("Wrong Validate() result for %q.\n expect:\t%+v\n got:\t%+v", tc.name, tc.errs, err)
Expand Down

0 comments on commit 0b7d303

Please sign in to comment.