update docs and CNI logging (#2433)

README.md

@@ -316,7 +316,7 @@ Default: `DEBUG`
 
 Valid Values: `DEBUG`, `INFO`, `WARN`, `ERROR`, `FATAL`. (Not case sensitive)
 
-Specifies the loglevel for `ipamd` and `cni-metric-helper`.
+Specifies the log level for `ipamd` and `cni-metric-helper`.
 
 ---
 
@@ -326,9 +326,13 @@ Type: String
 
 Default: `/host/var/log/aws-routed-eni/ipamd.log`
 
-Valid Values: `stdout` or a file path
+Valid Values: `stdout`, `stderr`, or a file path
 
-Specifies where to write the logging output of `ipamd`. Either to stdout or to override the default file (i.e., `/var/log/aws-routed-eni/ipamd.log`).
+Specifies where to write the logging output of `ipamd`: `stdout`, `stderr`, or a file path other than the default (`/var/log/aws-routed-eni/ipamd.log`).
+
+Note: `/host/var/log/...` is the container file-system path, which maps to `/var/log/...` on the node.
+
+Note: The IPAMD process runs within the `aws-node` pod, so writing to `stdout` or `stderr` will write to `aws-node` pod logs.
 
 ---
 
@@ -338,12 +342,15 @@ Type: String
 
 Default: `/var/log/aws-routed-eni/plugin.log`
 
-Valid Values: `stderr` or a file path
+Valid Values: `stderr` or a file path. Note that setting to the empty string is an alias for `stderr`, and this comes from upstream kubernetes best practices.
+
+Specifies where to write the logging output for `aws-cni` plugin: `stderr` or a file path other than the default (`/var/log/aws-routed-eni/plugin.log`).
+
+Note: `stdout` cannot be supported for plugin log. Please refer to [#1248](https://github.com/aws/amazon-vpc-cni-k8s/issues/1248) for more details.
 
-Specifies where to write the logging output for `aws-cni` plugin. Either to `stderr` or to override the default file (i.e., `/var/log/aws-routed-eni/plugin.log`).
-`Stdout` cannot be supported for plugin log, please refer to [#1248](https://github.com/aws/amazon-vpc-cni-k8s/issues/1248) for more details.
+Note: In EKS 1.24+, the CNI plugin is exec'ed by the container runtime, so `stderr` is for the container-runtime process, NOT the `aws-node` pod. In older versions, the CNI plugin was exec'ed by kubelet, so `stderr` is for the kubelet process.
 
-Note: If chaining an external plugin (i.e Cilium) that does not provide a `pluginLogFile` in its config file, the CNI plugin will by default write to `os.Stderr`. The output of `cmdAdd` are available in the Kubelet logs.
+Note: If chaining an external plugin (i.e. Cilium) that does not provide a `pluginLogFile` in its config file, the CNI plugin will by default write to `os.Stderr`.
 
 ---
 

cmd/cni-metrics-helper/metrics/pod_watcher.go

@@ -56,6 +56,6 @@ func (d *defaultPodWatcher) GetCNIPods(ctx context.Context) ([]string, error) {
 		CNIPods = append(CNIPods, pod.Name)
 	}
 
-	d.log.Infof("Total aws-node pod count:- ", len(CNIPods))
+	d.log.Infof("Total aws-node pod count: %d", len(CNIPods))
 	return CNIPods, nil
 }

pkg/utils/logger/zaplogger.go

@@ -130,14 +130,14 @@ func (logConfig *Configuration) newZapLogger() *structuredLogger {
 func getPluginLogFilePath(logFilePath string) zapcore.WriteSyncer {
 	var writer zapcore.WriteSyncer
 
-	if logFilePath == "" {
+	// When path is explicitly empty, write to stderr
+	if logFilePath == "" || strings.ToLower(logFilePath) == "stderr" {
 		writer = zapcore.Lock(os.Stderr)
-	} else if strings.ToLower(logFilePath) != "stdout" {
-		writer = getLogWriter(logFilePath)
-	} else {
+	} else if strings.ToLower(logFilePath) == "stdout" {
 		writer = zapcore.Lock(os.Stdout)
+	} else {
+		writer = getLogWriter(logFilePath)
 	}
-
 	return writer
 }