Skip to content

Commit

Permalink
Drop logger config from ConfigMap and update defaults
Browse files Browse the repository at this point in the history
  • Loading branch information
@jonathan-innis
jonathan-innis committed Nov 24, 2023
1 parent c4b2d82 commit 00f6948
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 75 deletions.
83 changes: 13 additions & 70 deletions pkg/operator/logging/logging.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,41 +20,37 @@ import (
"fmt"
"log"
"os"
"time"

"github.com/go-logr/logr"
"github.com/go-logr/zapr"
"github.com/samber/lo"
"go.uber.org/zap"
"go.uber.org/zap/zapcore"
"go.uber.org/zap/zapio"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/informers"
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/rest"
"k8s.io/klog/v2"
"knative.dev/pkg/changeset"
"knative.dev/pkg/logging"
"knative.dev/pkg/logging/logkey"
"knative.dev/pkg/system"

ctrl "sigs.k8s.io/controller-runtime/pkg/log"

"github.com/aws/karpenter-core/pkg/operator/injection"
"github.com/aws/karpenter-core/pkg/operator/options"
)

const (
loggerCfgConfigMapName = "config-logging"
loggerCfgConfigMapKey = "zap-logger-config"
loggerCfgDir = "/etc/karpenter/logging"
loggerCfgFilePath = loggerCfgDir + "/zap-logger-config"
loggerCfgDir = "/etc/karpenter/logging"
loggerCfgFilePath = loggerCfgDir + "/zap-logger-config"
)

func DefaultZapConfig(component string) zap.Config {
func DefaultZapConfig(ctx context.Context, component string) zap.Config {
logLevel := lo.Ternary(component != "webhook", zap.NewAtomicLevelAt(zap.InfoLevel), zap.NewAtomicLevelAt(zap.ErrorLevel))
if l := options.FromContext(ctx).LogLevel; l != "" && component != "webhook" {
// Webhook log level can only be configured directly through the zap-config
// Webhooks are deprecated, so support for changing their log level is also deprecated
logLevel = lo.Must(zap.ParseAtomicLevel(l))
}
return zap.Config{
Level: lo.Ternary(component != "webhook", zap.NewAtomicLevelAt(zap.InfoLevel), zap.NewAtomicLevelAt(zap.ErrorLevel)),
Level: logLevel,
Development: false,
DisableCaller: true,
DisableStacktrace: true,
Expand Down Expand Up @@ -83,18 +79,11 @@ func DefaultZapConfig(component string) zap.Config {
}

// NewLogger returns a configured *zap.SugaredLogger
func NewLogger(ctx context.Context, component string, kubernetesInterface kubernetes.Interface) *zap.SugaredLogger {
func NewLogger(ctx context.Context, component string) *zap.SugaredLogger {
if logger := loggerFromFile(ctx, component); logger != nil {
logger.Debugf("loaded log configuration from file %q", loggerCfgFilePath)
return logger
}
// TODO @joinnis: Drop support for loading logging configuration from the apiserver discovered ConfigMap when
// dropping alpha support. At that point, we will only support the environment variables and file-based config
if logger := loggerFromConfigMap(ctx, component, kubernetesInterface); logger != nil {
logger.Debugf("loaded log configuration from configmap %q", types.NamespacedName{Namespace: system.Namespace(), Name: loggerCfgConfigMapName})
logger.Error("loading log configuration through the configmap is deprecated, use file or environment-variable based configuration instead")
return logger
}
return defaultLogger(ctx, component)
}

Expand All @@ -109,15 +98,7 @@ func WithCommit(logger *zap.SugaredLogger) *zap.SugaredLogger {
}

func defaultLogger(ctx context.Context, component string) *zap.SugaredLogger {
cfg := DefaultZapConfig(component)
if l := options.FromContext(ctx).LogLevel; l != "" {
// Webhook log level can only be configured directly through the zap-config
// Webhooks are deprecated, so support for changing their log level is also deprecated
if component != "webhook" {
cfg.Level = lo.Must(zap.ParseAtomicLevel(l))
}
}
return WithCommit(lo.Must(cfg.Build()).Sugar()).Named(component)
return WithCommit(lo.Must(DefaultZapConfig(ctx, component).Build()).Sugar()).Named(component)
}

func loggerFromFile(ctx context.Context, component string) *zap.SugaredLogger {
Expand All @@ -128,7 +109,7 @@ func loggerFromFile(ctx context.Context, component string) *zap.SugaredLogger {
}
log.Fatalf("retrieving logging configuration file from %q", loggerCfgFilePath)
}
cfg := DefaultZapConfig(component)
cfg := DefaultZapConfig(ctx, component)
lo.Must0(json.Unmarshal(raw, &cfg))

raw, err = os.ReadFile(loggerCfgDir + fmt.Sprintf("/loglevel.%s", component))
Expand All @@ -138,44 +119,6 @@ func loggerFromFile(ctx context.Context, component string) *zap.SugaredLogger {
if raw != nil {
cfg.Level = lo.Must(zap.ParseAtomicLevel(string(raw)))
}
if l := options.FromContext(ctx).LogLevel; l != "" {
// Webhook log level can only be configured directly through the zap-config
// Webhooks are deprecated, so support for changing their log level is also deprecated
if component != "webhook" {
cfg.Level = lo.Must(zap.ParseAtomicLevel(l))
}
}
return WithCommit(lo.Must(cfg.Build()).Sugar()).Named(component)
}

func loggerFromConfigMap(ctx context.Context, component string, kubernetesInterface kubernetes.Interface) *zap.SugaredLogger {
cancelCtx, cancel := context.WithTimeout(ctx, time.Second*5)
defer cancel()

factory := informers.NewSharedInformerFactoryWithOptions(kubernetesInterface, time.Second*30, informers.WithNamespace(system.Namespace()))
informer := factory.Core().V1().ConfigMaps().Informer()
factory.Start(cancelCtx.Done())

cm, err := injection.WaitForConfigMap(cancelCtx, loggerCfgConfigMapName, informer)
if err != nil {
if errors.IsNotFound(err) {
return nil
}
log.Fatalf("retrieving logging config map from %q", types.NamespacedName{Namespace: system.Namespace(), Name: loggerCfgConfigMapName})
}
cfg := DefaultZapConfig(component)
lo.Must0(json.Unmarshal([]byte(cm.Data[loggerCfgConfigMapKey]), &cfg))

if v := cm.Data[fmt.Sprintf("loglevel.%s", component)]; v != "" {
cfg.Level = lo.Must(zap.ParseAtomicLevel(v))
}
if l := options.FromContext(ctx).LogLevel; l != "" {
// Webhook log level can only be configured directly through the zap-config
// Webhooks are deprecated, so support for changing their log level is also deprecated
if component != "webhook" {
cfg.Level = lo.Must(zap.ParseAtomicLevel(l))
}
}
return WithCommit(lo.Must(cfg.Build()).Sugar()).Named(component)
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/operator/operator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func NewOperator() (context.Context, *Operator) {
}

// Logging
logger := logging.NewLogger(ctx, component, kubernetesInterface)
logger := logging.NewLogger(ctx, component)
ctx = knativelogging.WithLogger(ctx, logger)
logging.ConfigureGlobalLoggers(ctx)

Expand Down Expand Up @@ -229,7 +229,7 @@ func (o *Operator) Start(ctx context.Context) {
wg.Add(1)
go func() {
defer wg.Done()
webhooks.Start(ctx, o.GetConfig(), o.KubernetesInterface, o.webhooks...)
webhooks.Start(ctx, o.GetConfig(), o.webhooks...)
}()
}
wg.Wait()
Expand Down
5 changes: 2 additions & 3 deletions pkg/webhooks/webhooks.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (
"go.uber.org/zap"
"golang.org/x/sync/errgroup"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/rest"
"knative.dev/pkg/configmap"
"knative.dev/pkg/controller"
Expand Down Expand Up @@ -85,9 +84,9 @@ func NewConfigValidationWebhook(ctx context.Context, _ configmap.Watcher) *contr

// Start copies the relevant portions for starting the webhooks from sharedmain.MainWithConfig
// https://github.com/knative/pkg/blob/0f52db700d63/injection/sharedmain/main.go#L227
func Start(ctx context.Context, cfg *rest.Config, kubernetesInterface kubernetes.Interface, ctors ...knativeinjection.ControllerConstructor) {
func Start(ctx context.Context, cfg *rest.Config, ctors ...knativeinjection.ControllerConstructor) {
ctx, startInformers := knativeinjection.EnableInjectionOrDie(ctx, cfg)
logger := logging.NewLogger(ctx, component, kubernetesInterface)
logger := logging.NewLogger(ctx, component)
ctx = knativelogging.WithLogger(ctx, logger)

cmw := sharedmain.SetupConfigMapWatchOrDie(ctx, knativelogging.FromContext(ctx))
Expand Down

0 comments on commit 00f6948

Please sign in to comment.