Comprehensive logging improvements (#535)

Makefile

@@ -43,7 +43,6 @@ licenses: ## Verifies dependency licenses and requires GITHUB_TOKEN to be set
 	golicense hack/license-config.hcl karpenter
 
 apply: ## Deploy the controller into your ~/.kube/config cluster
-	kubectl create ns karpenter || true
 	helm template karpenter charts/karpenter --namespace karpenter \
 		$(HELM_OPTS) \
 		--set controller.image=ko://github.com/awslabs/karpenter/cmd/controller \
@@ -52,7 +51,6 @@ apply: ## Deploy the controller into your ~/.kube/config cluster
 
 delete: ## Delete the controller from your ~/.kube/config cluster
 	helm template karpenter charts/karpenter --namespace karpenter | kubectl delete -f -
-	kubectl delete ns karpenter
 
 codegen: ## Generate code. Must be run if changes are made to ./pkg/apis/...
 	controller-gen \

charts/karpenter/templates/logging.yaml → charts/karpenter/templates/100-config-logging.yaml

@@ -3,11 +3,16 @@ kind: ConfigMap
 metadata:
   name: config-logging
   namespace: {{ .Release.Namespace }}
+  labels:
+    app.kubernetes.io/part-of: karpenter
 data:
   # https://github.com/uber-go/zap/blob/aa3e73ec0896f8b066ddf668597a02f89628ee50/config.go
   zap-logger-config: |
     {
       "level": "info",
+      "development": false,
+      "disableStacktrace": true,
+      "disableCaller": true,
       "sampling": {
         "initial": 100,
         "thereafter": 100
@@ -18,14 +23,14 @@ data:
       "encoderConfig": {
         "timeKey": "time",
         "levelKey": "level",
-        "nameKey": "name",
+        "nameKey": "logger",
         "callerKey": "caller",
         "messageKey": "message",
         "stacktraceKey": "stacktrace",
         "levelEncoder": "capital",
-        "timeEncoder": "iso8601",
+        "timeEncoder": "iso8601"
       }
     }
   # Log level overrides
-  loglevel.controller: "debug"
-  loglevel.webhook: "debug"
+  loglevel.controller: info # debug
+  loglevel.webhook: info # debug

charts/karpenter/templates/controller/deployment.yaml

@@ -49,9 +49,13 @@ spec:
               path: /healthz
               port: 8081
           env:
-          {{- with .Values.controller.env }}
-          {{- toYaml . | nindent 10 }}
-          {{- end }}
+            - name: SYSTEM_NAMESPACE
+              valueFrom:
+                fieldRef:
+                  fieldPath: metadata.namespace
+            {{- with .Values.controller.env }}
+            {{- toYaml . | nindent 10 }}
+            {{- end }}
       # https://github.com/aws/amazon-eks-pod-identity-webhook/issues/8#issuecomment-636888074
       securityContext:
         fsGroup: 1000
@@ -63,7 +67,7 @@ spec:
         {{- if .Values.controller.affinity }}
           {{- toYaml .Values.controller.affinity | nindent 8 }}
         {{- else }}
-        nodeAffinity: 
+        nodeAffinity:
           requiredDuringSchedulingIgnoredDuringExecution:
             nodeSelectorTerms:
             - matchExpressions:

charts/karpenter/templates/webhook/rbac.yaml

@@ -31,14 +31,9 @@ metadata:
   name: karpenter-webhook
   namespace: {{ .Release.Namespace }}
 rules:
-- apiGroups:
-  - ""
-  resources:
-  - configmaps
-  verbs:
-  - get
-  - list
-  - watch
+- apiGroups: [""]
+  resources: ["configmaps", "namespaces"]
+  verbs: ["get", "list", "watch"]
 - apiGroups:
   - ""
   resources:

charts/karpenter/templates/webhook/webhooks.yaml

@@ -50,3 +50,19 @@ webhooks:
     - CREATE
     - UPDATE
     - DELETE
+apiVersion: admissionregistration.k8s.io/v1
+kind: ValidatingWebhookConfiguration
+metadata:
+  name: validation.webhook.config.karpenter.sh
+webhooks:
+- admissionReviewVersions: ["v1"]
+  clientConfig:
+    service:
+      name: karpenter-webhook
+      namespace: '{{ .Release.Namespace }}'
+  failurePolicy: Fail
+  sideEffects: None
+  name: validation.webhook.config.karpenter.sh
+  objectSelector:
+    matchLabels:
+      app.kubernetes.io/part-of: karpenter

cmd/controller/main.go

@@ -15,6 +15,7 @@ limitations under the License.
 package main
 
 import (
+	"context"
 	"flag"
 	"fmt"
 
@@ -26,20 +27,25 @@ import (
 	"github.com/awslabs/karpenter/pkg/controllers/expiration"
 	"github.com/awslabs/karpenter/pkg/controllers/reallocation"
 	"github.com/awslabs/karpenter/pkg/controllers/termination"
-	"github.com/awslabs/karpenter/pkg/utils/log"
-
-	"go.uber.org/zap/zapcore"
+	"github.com/go-logr/zapr"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/kubernetes"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	_ "k8s.io/client-go/plugin/pkg/client/auth/gcp"
+	"k8s.io/client-go/rest"
+	"knative.dev/pkg/configmap/informer"
+	"knative.dev/pkg/injection"
+	"knative.dev/pkg/injection/sharedmain"
+	"knative.dev/pkg/logging"
+	"knative.dev/pkg/signals"
+	"knative.dev/pkg/system"
 	controllerruntime "sigs.k8s.io/controller-runtime"
-	controllerruntimezap "sigs.k8s.io/controller-runtime/pkg/log/zap"
 )
 
 var (
-	scheme  = runtime.NewScheme()
-	options = Options{}
+	scheme    = runtime.NewScheme()
+	options   = Options{}
+	component = "controller"
 )
 
 func init() {
@@ -49,40 +55,53 @@ func init() {
 
 // Options for running this binary
 type Options struct {
-	EnableVerboseLogging bool
-	MetricsPort          int
-	HealthProbePort      int
+	MetricsPort     int
+	HealthProbePort int
 }
 
 func main() {
-	flag.BoolVar(&options.EnableVerboseLogging, "verbose", false, "Enable verbose logging")
 	flag.IntVar(&options.MetricsPort, "metrics-port", 8080, "The port the metric endpoint binds to for operating metrics about the controller itself")
 	flag.IntVar(&options.HealthProbePort, "health-probe-port", 8081, "The port the health probe endpoint binds to for reporting controller health")
 	flag.Parse()
 
-	log.Setup(
-		controllerruntimezap.UseDevMode(options.EnableVerboseLogging),
-		controllerruntimezap.ConsoleEncoder(),
-		controllerruntimezap.StacktraceLevel(zapcore.DPanicLevel),
-	)
-	manager := controllers.NewManagerOrDie(controllerruntime.GetConfigOrDie(), controllerruntime.Options{
+	config := controllerruntime.GetConfigOrDie()
+	clientSet := kubernetes.NewForConfigOrDie(config)
+
+	// 1. Setup logger and watch for changes to log level
+	ctx := LoggingContextOrDie(config, clientSet)
+
+	// 2. Setup controller runtime controller
+	cloudProvider := registry.NewCloudProvider(ctx, cloudprovider.Options{ClientSet: clientSet})
+	manager := controllers.NewManagerOrDie(config, controllerruntime.Options{
+		Logger:                 zapr.NewLogger(logging.FromContext(ctx).Desugar()),
 		LeaderElection:         true,
 		LeaderElectionID:       "karpenter-leader-election",
 		Scheme:                 scheme,
 		MetricsBindAddress:     fmt.Sprintf(":%d", options.MetricsPort),
 		HealthProbeBindAddress: fmt.Sprintf(":%d", options.HealthProbePort),
 	})
-
-	clientSet := kubernetes.NewForConfigOrDie(manager.GetConfig())
-	cloudProvider := registry.NewCloudProvider(cloudprovider.Options{ClientSet: clientSet})
-	ctx := controllerruntime.SetupSignalHandler()
-
 	if err := manager.RegisterControllers(ctx,
 		expiration.NewController(manager.GetClient()),
 		allocation.NewController(manager.GetClient(), clientSet.CoreV1(), cloudProvider),
 		reallocation.NewController(manager.GetClient(), clientSet.CoreV1(), cloudProvider),
-		termination.NewController(manager.GetClient(), clientSet.CoreV1(), cloudProvider),
+		termination.NewController(ctx, manager.GetClient(), clientSet.CoreV1(), cloudProvider),
 	).Start(ctx); err != nil {
 		panic(fmt.Sprintf("Unable to start manager, %s", err.Error()))
 	}
 }
+
+// LoggingContextOrDie injects a logger into the returned context. The logger is
+// configured by the ConfigMap `config-logging` and live updates the level.
+func LoggingContextOrDie(config *rest.Config, clientSet *kubernetes.Clientset) context.Context {
+	ctx, startinformers := injection.EnableInjectionOrDie(signals.NewContext(), config)
+	logger, atomicLevel := sharedmain.SetupLoggerOrDie(ctx, component)
+	ctx = logging.WithLogger(ctx, logger)
+	rest.SetDefaultWarningHandler(&logging.WarningHandler{Logger: logger})
+	cmw := informer.NewInformedWatcher(clientSet, system.Namespace())
+	sharedmain.WatchLoggingConfigOrDie(ctx, cmw, logger, atomicLevel, component)
+	if err := cmw.Start(ctx.Done()); err != nil {
+		logger.Fatalf("Failed to watch logging configuration, %s", err.Error())
+	}
+	startinformers()
+	return ctx
+}

cmd/webhook/main.go

@@ -26,10 +26,12 @@ import (
 	"knative.dev/pkg/controller"
 	"knative.dev/pkg/injection"
 	"knative.dev/pkg/injection/sharedmain"
+	"knative.dev/pkg/logging"
 	"knative.dev/pkg/signals"
 	"knative.dev/pkg/system"
 	"knative.dev/pkg/webhook"
 	"knative.dev/pkg/webhook/certificates"
+	"knative.dev/pkg/webhook/configmaps"
 	"knative.dev/pkg/webhook/resourcesemantics/defaulting"
 	"knative.dev/pkg/webhook/resourcesemantics/validation"
 )
@@ -47,26 +49,25 @@ func main() {
 	flag.Parse()
 
 	config := injection.ParseAndGetRESTConfigOrDie()
+	ctx := webhook.WithOptions(injection.WithNamespaceScope(signals.NewContext(), system.Namespace()), webhook.Options{
+		Port:        options.Port,
+		ServiceName: "karpenter-webhook",
+		SecretName:  "karpenter-webhook-cert",
+	})
 
 	// Register the cloud provider to attach vendor specific validation logic.
-	registry.NewCloudProvider(cloudprovider.Options{ClientSet: kubernetes.NewForConfigOrDie(config)})
+	registry.NewCloudProvider(ctx, cloudprovider.Options{ClientSet: kubernetes.NewForConfigOrDie(config)})
 
 	// Controllers and webhook
-	sharedmain.MainWithConfig(
-		webhook.WithOptions(injection.WithNamespaceScope(signals.NewContext(), system.Namespace()), webhook.Options{
-			Port:        options.Port,
-			ServiceName: "karpenter-webhook",
-			SecretName:  "karpenter-webhook-cert",
-		}),
-		"karpenter.webhooks",
-		config,
+	sharedmain.MainWithConfig(ctx, "webhook", config,
 		certificates.NewController,
-		NewCRDDefaultingWebhook,
-		NewCRDValidationWebhook,
+		newCRDDefaultingWebhook,
+		newCRDValidationWebhook,
+		newConfigValidationController,
 	)
 }
 
-func NewCRDDefaultingWebhook(ctx context.Context, w configmap.Watcher) *controller.Impl {
+func newCRDDefaultingWebhook(ctx context.Context, w configmap.Watcher) *controller.Impl {
 	return defaulting.NewAdmissionController(ctx,
 		"defaulting.webhook.provisioners.karpenter.sh",
 		"/default-resource",
@@ -76,7 +77,7 @@ func NewCRDDefaultingWebhook(ctx context.Context, w configmap.Watcher) *controll
 	)
 }
 
-func NewCRDValidationWebhook(ctx context.Context, w configmap.Watcher) *controller.Impl {
+func newCRDValidationWebhook(ctx context.Context, w configmap.Watcher) *controller.Impl {
 	return validation.NewAdmissionController(ctx,
 		"validation.webhook.provisioners.karpenter.sh",
 		"/validate-resource",
@@ -86,4 +87,14 @@ func NewCRDValidationWebhook(ctx context.Context, w configmap.Watcher) *controll
 	)
 }
 
+func newConfigValidationController(ctx context.Context, cmw configmap.Watcher) *controller.Impl {
+	return configmaps.NewAdmissionController(ctx,
+		"validation.webhook.config.karpenter.sh",
+		"/config-validation",
+		configmap.Constructors{
+			logging.ConfigMapName(): logging.NewConfigFromConfigMap,
+		},
+	)
+}
+
 func InjectContext(ctx context.Context) context.Context { return ctx }