Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: Drop Support for Log Config, Add Log Paths #1469

Merged
merged 9 commits into from
Jul 30, 2024
Merged

feat: Drop Support for Log Config, Add Log Paths #1469

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
b6c9728
feat: drop logging configuration
rschalo Jul 24, 2024
c92614a
update flag description
rschalo Jul 24, 2024
88fb12e
Merge branch 'main' into drop-log-config
rschalo Jul 24, 2024
a22fab4
Merge branch 'main' into drop-log-config
rschalo Jul 25, 2024
e3e4826
resolve conflicts
rschalo Jul 25, 2024
04c9f1e
rerun presubmit
rschalo Jul 25, 2024
5a31421
Merge branch 'main' into drop-log-config
rschalo Jul 25, 2024
7288a87
pr response
rschalo Jul 30, 2024
82635c4
Merge branch 'main' into drop-log-config
rschalo Jul 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 4 additions & 41 deletions pkg/operator/logging/logging.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,7 @@ package logging

import (
"context"
"encoding/json"
"fmt"
"log"
"os"
"strings"

"github.com/go-logr/logr"
"github.com/go-logr/zapr"
Expand All @@ -35,11 +32,6 @@ import (
"sigs.k8s.io/karpenter/pkg/operator/options"
)

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

// NopLogger is used to throw away logs when we don't actually want to log in
// certain portions of the code since logging would be too noisy
var NopLogger = zapr.NewLogger(zap.NewNop())
Expand Down Expand Up @@ -75,18 +67,14 @@ func DefaultZapConfig(ctx context.Context, component string) zap.Config {
EncodeDuration: zapcore.StringDurationEncoder,
EncodeCaller: zapcore.ShortCallerEncoder,
},
OutputPaths: []string{"stdout"},
ErrorOutputPaths: []string{"stderr"},
OutputPaths: strings.Split(options.FromContext(ctx).LogOutputPaths, ","),
ErrorOutputPaths: strings.Split(options.FromContext(ctx).LogErrorOutputPaths, ","),
}
}

// NewLogger returns a configured *zap.SugaredLogger
func NewLogger(ctx context.Context, component string) *zap.Logger {
if logger := loggerFromFile(ctx, component); logger != nil {
logger.Debug(fmt.Sprintf("loaded log configuration from file %q", loggerCfgFilePath))
return logger
}
return defaultLogger(ctx, component)
return WithCommit(lo.Must(DefaultZapConfig(ctx, component).Build())).Named(component)
}

func WithCommit(logger *zap.Logger) *zap.Logger {
Expand All @@ -99,31 +87,6 @@ func WithCommit(logger *zap.Logger) *zap.Logger {
return logger.With(zap.String(logkey.Commit, revision))
}

func defaultLogger(ctx context.Context, component string) *zap.Logger {
return WithCommit(lo.Must(DefaultZapConfig(ctx, component).Build())).Named(component)
}

func loggerFromFile(ctx context.Context, component string) *zap.Logger {
raw, err := os.ReadFile(loggerCfgFilePath)
if err != nil {
if os.IsNotExist(err) {
return nil
}
log.Fatalf("retrieving logging configuration file from %q", loggerCfgFilePath)
}
cfg := DefaultZapConfig(ctx, component)
lo.Must0(json.Unmarshal(raw, &cfg))

raw, err = os.ReadFile(loggerCfgDir + fmt.Sprintf("/loglevel.%s", component))
if err != nil && !os.IsNotExist(err) {
log.Fatalf("retrieving logging controller log level file from %q", loggerCfgDir+fmt.Sprintf("/loglevel.%s", component))
}
if raw != nil {
cfg.Level = lo.Must(zap.ParseAtomicLevel(string(raw)))
}
return WithCommit(lo.Must(cfg.Build())).Named(component)
}

type ignoreDebugEventsSink struct {
name string
sink logr.LogSink
Expand Down
4 changes: 4 additions & 0 deletions pkg/operator/options/options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ type Options struct {
DisableLeaderElection bool
MemoryLimit int64
LogLevel string
LogOutputPaths string
LogErrorOutputPaths string
BatchMaxDuration time.Duration
BatchIdleDuration time.Duration
FeatureGates FeatureGates
Expand Down Expand Up @@ -93,6 +95,8 @@ func (o *Options) AddFlags(fs *FlagSet) {
fs.BoolVarWithEnv(&o.DisableLeaderElection, "disable-leader-election", "DISABLE_LEADER_ELECTION", false, "Disable the leader election client before executing the main loop. Disable when running replicated components for high availability is not desired.")
fs.Int64Var(&o.MemoryLimit, "memory-limit", env.WithDefaultInt64("MEMORY_LIMIT", -1), "Memory limit on the container running the controller. The GC soft memory limit is set to 90% of this value.")
fs.StringVar(&o.LogLevel, "log-level", env.WithDefaultString("LOG_LEVEL", "info"), "Log verbosity level. Can be one of 'debug', 'info', or 'error'")
fs.StringVar(&o.LogOutputPaths, "log-output-paths", env.WithDefaultString("LOG_OUTPUT_PATHS", "stdout"), "Optional comma separated paths for directing log output")
fs.StringVar(&o.LogErrorOutputPaths, "log-error-output-paths", env.WithDefaultString("LOG_ERROR_OUTPUT_PATHS", "stderr"), "Optional comma separated paths for logging error output")
fs.DurationVar(&o.BatchMaxDuration, "batch-max-duration", env.WithDefaultDuration("BATCH_MAX_DURATION", 10*time.Second), "The maximum length of a batch window. The longer this is, the more pods we can consider for provisioning at one time which usually results in fewer but larger nodes.")
fs.DurationVar(&o.BatchIdleDuration, "batch-idle-duration", env.WithDefaultDuration("BATCH_IDLE_DURATION", time.Second), "The maximum amount of time with no new pending pods that if exceeded ends the current batching window. If pods arrive faster than this time, the batching window will be extended up to the maxDuration. If they arrive slower, the pods will be batched separately.")
fs.StringVar(&o.FeatureGates.inputStr, "feature-gates", env.WithDefaultString("FEATURE_GATES", "SpotToSpotConsolidation=false"), "Optional features can be enabled / disabled using feature gates. Current options are: SpotToSpotConsolidation")
Expand Down
23 changes: 21 additions & 2 deletions pkg/operator/options/suite_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ var _ = Describe("Options", func() {
"DISABLE_LEADER_ELECTION",
"MEMORY_LIMIT",
"LOG_LEVEL",
"LOG_OUTPUT_PATHS",
"LOG_ERROR_OUTPUT_PATHS",
"BATCH_MAX_DURATION",
"BATCH_IDLE_DURATION",
"FEATURE_GATES",
Expand Down Expand Up @@ -108,6 +110,8 @@ var _ = Describe("Options", func() {
DisableLeaderElection: lo.ToPtr(false),
MemoryLimit: lo.ToPtr[int64](-1),
LogLevel: lo.ToPtr("info"),
LogOutputPaths: lo.ToPtr("stdout"),
LogErrorOutputPaths: lo.ToPtr("stderr"),
BatchMaxDuration: lo.ToPtr(10 * time.Second),
BatchIdleDuration: lo.ToPtr(time.Second),
FeatureGates: test.FeatureGates{
Expand All @@ -117,6 +121,8 @@ var _ = Describe("Options", func() {
})

It("shouldn't overwrite CLI flags with environment variables", func() {
os.Setenv("LOG_OUTPUT_PATHS", "stdout")
os.Setenv("LOG_ERROR_OUTPUT_PATHS", "stderr")
err := opts.Parse(
fs,
"--karpenter-service", "cli",
Expand All @@ -131,6 +137,8 @@ var _ = Describe("Options", func() {
"--disable-leader-election=true",
"--memory-limit", "0",
"--log-level", "debug",
"--log-output-paths", "/etc/k8s/test",
"--log-error-output-paths", "/etc/k8s/testerror",
"--batch-max-duration", "5s",
"--batch-idle-duration", "5s",
"--feature-gates", "SpotToSpotConsolidation=true",
Expand All @@ -149,6 +157,8 @@ var _ = Describe("Options", func() {
DisableLeaderElection: lo.ToPtr(true),
MemoryLimit: lo.ToPtr[int64](0),
LogLevel: lo.ToPtr("debug"),
LogOutputPaths: lo.ToPtr("/etc/k8s/test"),
LogErrorOutputPaths: lo.ToPtr("/etc/k8s/testerror"),
BatchMaxDuration: lo.ToPtr(5 * time.Second),
BatchIdleDuration: lo.ToPtr(5 * time.Second),
FeatureGates: test.FeatureGates{
Expand All @@ -170,6 +180,8 @@ var _ = Describe("Options", func() {
os.Setenv("DISABLE_LEADER_ELECTION", "true")
os.Setenv("MEMORY_LIMIT", "0")
os.Setenv("LOG_LEVEL", "debug")
os.Setenv("LOG_OUTPUT_PATHS", "/etc/k8s/test")
os.Setenv("LOG_ERROR_OUTPUT_PATHS", "/etc/k8s/testerror")
os.Setenv("BATCH_MAX_DURATION", "5s")
os.Setenv("BATCH_IDLE_DURATION", "5s")
os.Setenv("FEATURE_GATES", "SpotToSpotConsolidation=true")
Expand All @@ -192,6 +204,8 @@ var _ = Describe("Options", func() {
DisableLeaderElection: lo.ToPtr(true),
MemoryLimit: lo.ToPtr[int64](0),
LogLevel: lo.ToPtr("debug"),
LogOutputPaths: lo.ToPtr("/etc/k8s/test"),
LogErrorOutputPaths: lo.ToPtr("/etc/k8s/testerror"),
BatchMaxDuration: lo.ToPtr(5 * time.Second),
BatchIdleDuration: lo.ToPtr(5 * time.Second),
FeatureGates: test.FeatureGates{
Expand Down Expand Up @@ -221,12 +235,13 @@ var _ = Describe("Options", func() {
err := opts.Parse(
fs,
"--karpenter-service", "cli",
"--disable-webhook",
"--log-output-paths", "/etc/k8s/test",
"--log-error-output-paths", "/etc/k8s/testerror",
)
Expect(err).To(BeNil())
expectOptionsMatch(opts, test.Options(test.OptionsFields{
ServiceName: lo.ToPtr("cli"),
DisableWebhook: lo.ToPtr(true),
DisableWebhook: lo.ToPtr(false),
WebhookPort: lo.ToPtr(0),
MetricsPort: lo.ToPtr(0),
WebhookMetricsPort: lo.ToPtr(0),
Expand All @@ -237,6 +252,8 @@ var _ = Describe("Options", func() {
DisableLeaderElection: lo.ToPtr(true),
MemoryLimit: lo.ToPtr[int64](0),
LogLevel: lo.ToPtr("debug"),
LogOutputPaths: lo.ToPtr("/etc/k8s/test"),
LogErrorOutputPaths: lo.ToPtr("/etc/k8s/testerror"),
BatchMaxDuration: lo.ToPtr(5 * time.Second),
BatchIdleDuration: lo.ToPtr(5 * time.Second),
FeatureGates: test.FeatureGates{
Expand Down Expand Up @@ -297,6 +314,8 @@ func expectOptionsMatch(optsA, optsB *options.Options) {
Expect(optsA.DisableLeaderElection).To(Equal(optsB.DisableLeaderElection))
Expect(optsA.MemoryLimit).To(Equal(optsB.MemoryLimit))
Expect(optsA.LogLevel).To(Equal(optsB.LogLevel))
Expect(optsA.LogOutputPaths).To(Equal(optsB.LogOutputPaths))
Expect(optsA.LogErrorOutputPaths).To(Equal(optsB.LogErrorOutputPaths))
Expect(optsA.BatchMaxDuration).To(Equal(optsB.BatchMaxDuration))
Expect(optsA.BatchIdleDuration).To(Equal(optsB.BatchIdleDuration))
Expect(optsA.FeatureGates.SpotToSpotConsolidation).To(Equal(optsB.FeatureGates.SpotToSpotConsolidation))
Expand Down
4 changes: 4 additions & 0 deletions pkg/test/options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ type OptionsFields struct {
DisableLeaderElection *bool
MemoryLimit *int64
LogLevel *string
LogOutputPaths *string
LogErrorOutputPaths *string
BatchMaxDuration *time.Duration
BatchIdleDuration *time.Duration
FeatureGates FeatureGates
Expand Down Expand Up @@ -70,6 +72,8 @@ func Options(overrides ...OptionsFields) *options.Options {
DisableLeaderElection: lo.FromPtrOr(opts.DisableLeaderElection, false),
MemoryLimit: lo.FromPtrOr(opts.MemoryLimit, -1),
LogLevel: lo.FromPtrOr(opts.LogLevel, ""),
LogOutputPaths: lo.FromPtrOr(opts.LogOutputPaths, "stdout"),
LogErrorOutputPaths: lo.FromPtrOr(opts.LogErrorOutputPaths, "stderr"),
BatchMaxDuration: lo.FromPtrOr(opts.BatchMaxDuration, 10*time.Second),
BatchIdleDuration: lo.FromPtrOr(opts.BatchIdleDuration, time.Second),
FeatureGates: options.FeatureGates{
Expand Down
Loading