Skip to content

Commit

Permalink
Merge pull request juju#16915 from SimonRichardson/3.1-into-3.3
Browse files Browse the repository at this point in the history
juju#16915

- juju#16905 from SimonRichardson/fix-client-leaks
- juju#16898 from SimonRichardson/suppress-klog-log-message
- juju#16890 from nvinuesa/revert-minikube-snap
- juju#16844 from nvinuesa/juju-5352
  • Loading branch information
jujubot authored Feb 9, 2024
2 parents a90328c + 58f22a3 commit 45fe48b
Show file tree
Hide file tree
Showing 6 changed files with 109 additions and 26 deletions.
14 changes: 14 additions & 0 deletions apiserver/facades/agent/secretsmanager/mocks/crossmodel.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions apiserver/facades/agent/secretsmanager/secrets.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ var logger = loggo.GetLoggerWithLabels("juju.apiserver.secretsmanager", corelogg
type CrossModelSecretsClient interface {
GetRemoteSecretContentInfo(uri *coresecrets.URI, revision int, refresh, peek bool, sourceControllerUUID, appToken string, unitId int, macs macaroon.Slice) (*secrets.ContentParams, *secretsprovider.ModelBackendConfig, int, bool, error)
GetSecretAccessScope(uri *coresecrets.URI, appToken string, unitId int) (string, error)
Close() error
}

// SecretsManagerAPI is the implementation for the SecretsManager facade.
Expand Down Expand Up @@ -472,6 +473,8 @@ func (s *SecretsManagerAPI) getRemoteSecretContent(uri *coresecrets.URI, refresh
if err != nil {
return nil, nil, false, errors.Annotate(err, "creating remote secret client")
}
defer func() { _ = extClient.Close() }()

consumerApp := commonsecrets.AuthTagApp(s.authTag)
token, err := s.crossModelState.GetToken(names.NewApplicationTag(consumerApp))
if err != nil {
Expand Down
4 changes: 4 additions & 0 deletions apiserver/facades/agent/secretsmanager/secrets_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1293,6 +1293,7 @@ func (s *SecretsManagerSuite) TestGetSecretContentCrossModelExistingConsumerNoRe
mac := apitesting.MustNewMacaroon("id")

s.remoteClient = mocks.NewMockCrossModelSecretsClient(ctrl)
s.remoteClient.EXPECT().Close().Return(nil)

s.crossModelState.EXPECT().GetToken(names.NewApplicationTag("mariadb")).Return("token", nil)
s.secretsConsumer.EXPECT().GetSecretConsumer(uri, consumer).Return(&coresecrets.SecretConsumerMetadata{
Expand Down Expand Up @@ -1359,6 +1360,7 @@ func (s *SecretsManagerSuite) TestGetSecretContentCrossModelExistingConsumerNoRe
mac := apitesting.MustNewMacaroon("id")

s.remoteClient = mocks.NewMockCrossModelSecretsClient(ctrl)
s.remoteClient.EXPECT().Close().Return(nil)

s.crossModelState.EXPECT().GetToken(names.NewApplicationTag("mariadb")).Return("token", nil)
s.secretsConsumer.EXPECT().GetSecretConsumer(uri, consumer).Return(&coresecrets.SecretConsumerMetadata{
Expand Down Expand Up @@ -1430,6 +1432,7 @@ func (s *SecretsManagerSuite) TestGetSecretContentCrossModelExistingConsumerRefr
mac := apitesting.MustNewMacaroon("id")

s.remoteClient = mocks.NewMockCrossModelSecretsClient(ctrl)
s.remoteClient.EXPECT().Close().Return(nil)

s.crossModelState.EXPECT().GetToken(names.NewApplicationTag("mariadb")).Return("token", nil)
s.secretsConsumer.EXPECT().GetSecretConsumer(uri, consumer).Return(&coresecrets.SecretConsumerMetadata{
Expand Down Expand Up @@ -1501,6 +1504,7 @@ func (s *SecretsManagerSuite) TestGetSecretContentCrossModelNewConsumer(c *gc.C)
mac := apitesting.MustNewMacaroon("id")

s.remoteClient = mocks.NewMockCrossModelSecretsClient(ctrl)
s.remoteClient.EXPECT().Close().Return(nil)

s.crossModelState.EXPECT().GetToken(names.NewApplicationTag("mariadb")).Return("token", nil)
s.secretsConsumer.EXPECT().GetSecretConsumer(uri, consumer).Return(nil, errors.NotFoundf(""))
Expand Down
111 changes: 86 additions & 25 deletions caas/kubernetes/provider/klog.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,11 @@ package provider

import (
"strings"
"time"

"github.com/go-logr/logr"
"github.com/juju/loggo"
)

type KlogMessagePrefixes []string

var (
klogIgnorePrefixes = KlogMessagePrefixes{
"lost connection to pod",
"an error occurred forwarding",
"error copying from remote stream to local connection",
"error copying from local connection to remote stream",
}
"golang.org/x/time/rate"
)

// klogAdapter is an adapter for Kubernetes logger onto juju loggo. We use this
Expand All @@ -34,16 +25,15 @@ func newKlogAdapter() logr.Logger {
})
}

func (k *klogAdapter) Init(info logr.RuntimeInfo) {
}
func (k *klogAdapter) Init(info logr.RuntimeInfo) {}

// Enabled see https://pkg.go.dev/github.com/go-logr/logr#Logger
func (k *klogAdapter) Enabled(level int) bool {
return true
}

// Error see https://pkg.go.dev/github.com/go-logr/logr#Logger
func (k *klogAdapter) Error(err error, msg string, keysAndValues ...interface{}) {
func (k *klogAdapter) Error(err error, msg string, keysAndValues ...any) {
if err != nil {
k.Logger.Errorf(msg+": "+err.Error(), keysAndValues...)
return
Expand All @@ -56,17 +46,13 @@ func (k *klogAdapter) Error(err error, msg string, keysAndValues ...interface{})
}

// Info see https://pkg.go.dev/github.com/go-logr/logr#Logger
func (k *klogAdapter) Info(level int, msg string, keysAndValues ...interface{}) {
k.Logger.Infof(msg, keysAndValues...)
}

func (k KlogMessagePrefixes) Matches(log string) bool {
for _, v := range k {
if strings.HasPrefix(log, v) {
return true
}
func (k *klogAdapter) Info(level int, msg string, keysAndValues ...any) {
if prefix, ok := klogSuppressedPrefixes.Matches(msg); ok && prefix != nil {
prefix.Do(k.Logger.Infof, msg, keysAndValues...)
return
}
return false

k.Logger.Infof(msg, keysAndValues...)
}

// V see https://pkg.go.dev/github.com/go-logr/logr#Logger
Expand All @@ -75,11 +61,86 @@ func (k *klogAdapter) V(level int) logr.LogSink {
}

// WithValues see https://pkg.go.dev/github.com/go-logr/logr#Logger
func (k *klogAdapter) WithValues(keysAndValues ...interface{}) logr.LogSink {
func (k *klogAdapter) WithValues(keysAndValues ...any) logr.LogSink {
return k
}

// WithName see https://pkg.go.dev/github.com/go-logr/logr#Logger
func (k *klogAdapter) WithName(name string) logr.LogSink {
return k
}

// klogMessagePrefixes is a list of prefixes to ignore.
type klogMessagePrefixes []string

var (
klogIgnorePrefixes = klogMessagePrefixes{
"lost connection to pod",
"an error occurred forwarding",
"error copying from remote stream to local connection",
"error copying from local connection to remote stream",
}
)

func (k klogMessagePrefixes) Matches(log string) bool {
for _, v := range k {
if strings.HasPrefix(log, v) {
return true
}
}
return false
}

// klogSuppressMessagePrefix is a log suppression type that suppresses log
// messages with a given prefix at a given rate. If the rate is nil, the
// suppression is disabled.
type klogSuppressMessagePrefix struct {
prefix string
rate *rate.Sometimes
}

// Do calls the logger function if the rate allows it. If the rate is nil, the
// function is called directly, thus bypassing the rate.
func (k klogSuppressMessagePrefix) Do(loggerFn func(string, ...any), msg string, args ...any) {
// If we don't have a rate, just call the function directly.
if k.rate == nil {
loggerFn(msg, args)
return
}

// If we have a rate, use it to suppress the message.
k.rate.Do(func() {
loggerFn(msg, args)
})
}

// klogSuppressMessagePrefixes is a list of prefixes to suppress and their
// suppression rates.
type klogSuppressMessagePrefixes []*klogSuppressMessagePrefix

var (
klogSuppressedPrefixes = klogSuppressMessagePrefixes{
&klogSuppressMessagePrefix{
prefix: "Use tokens from the TokenRequest API or manually created secret-based tokens instead of auto-generated secret-based tokens",
// We suppress the message at a rate of 1 per 5 minute, but we
// allow the first message to go through.
rate: &rate.Sometimes{
First: 1,
Interval: time.Minute * 5,
},
},
}
)

// Matches returns the prefix and whether it matches the log message.
func (k klogSuppressMessagePrefixes) Matches(log string) (*klogSuppressMessagePrefix, bool) {
for _, p := range k {
if p == nil {
continue
}
if strings.HasPrefix(log, p.prefix) {
return p, true
}
}
return nil, false
}
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ require (
golang.org/x/oauth2 v0.15.0
golang.org/x/sync v0.5.0
golang.org/x/sys v0.15.0
golang.org/x/time v0.5.0
golang.org/x/tools v0.16.1
google.golang.org/api v0.154.0
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c
Expand Down Expand Up @@ -292,7 +293,6 @@ require (
golang.org/x/mod v0.14.0 // indirect
golang.org/x/term v0.15.0 // indirect
golang.org/x/text v0.14.0 // indirect
golang.org/x/time v0.5.0 // indirect
google.golang.org/appengine v1.6.8 // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20231127180814-3a041ad873d4 // indirect
google.golang.org/grpc v1.59.0 // indirect
Expand Down
1 change: 1 addition & 0 deletions snap/snapcraft.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -534,3 +534,4 @@ plugs:
interface: personal-files
read:
- $HOME/.novarc

0 comments on commit 45fe48b

Please sign in to comment.