Merge pull request #1056 from AkihiroSuda/fix-logging-argv1

logging: ensure that MagicArgv1 is always argv1

Author: AkihiroSuda

README.md

@@ -474,7 +474,7 @@ Metadata flags:
 
 Logging flags:
 - :whale: `--log-driver=(json-file)`: Logging driver for the container (default `json-file`).
-    - :whale: `--log-driver=json-log`: The logs are formatted as JSON. The default logging driver for nerdctl.
+    - :whale: `--log-driver=json-file`: The logs are formatted as JSON. The default logging driver for nerdctl.
       - The `json-file` logging driver supports the following logging options:
           - :whale: `--log-opt=max-size=<MAX-SIZE>`: The maximum size of the log before it is rolled. A positive integer plus a modifier representing the unit of measure (k, m, or g). Defaults to unlimited.
           - :whale: `--log-opt=max-file=<MAX-FILE>`: The maximum number of log files that can be present. If rolling the logs creates excess files, the oldest file is removed. Only effective when `max-size` is also set. A positive integer. Defaults to 1.

cmd/nerdctl/main.go

@@ -71,22 +71,10 @@ func main() {
 }
 
 func xmain() error {
-	loggingMode := false
-	for _, arg := range os.Args {
-		if arg == logging.MagicArgv1 {
-			loggingMode = true
-			break
-		}
-	}
-	if len(os.Args) >= 3 && loggingMode {
+	if len(os.Args) == 3 && os.Args[1] == logging.MagicArgv1 {
 		// containerd runtime v2 logging plugin mode.
-		// "binary://BIN?KEY1=VALUE1&KEY2=VALUE2" URI is parsed into Args {BIN, KEY1, VALUE1, KEY2, VALUE2}.
-		argsMap := make(map[string]string)
-		args := os.Args[1:]
-		for i := 0; i < len(args); i += 2 {
-			argsMap[args[i]] = args[i+1]
-		}
-		return logging.Main(argsMap)
+		// "binary://BIN?KEY=VALUE" URI is parsed into Args {BIN, KEY, VALUE}.
+		return logging.Main(os.Args[2])
 	}
 	// nerdctl CLI mode
 	app, err := newApp()

cmd/nerdctl/run.go

@@ -478,7 +478,23 @@ func createContainer(cmd *cobra.Command, ctx context.Context, client *containerd
 		if err != nil {
 			return nil, "", nil, err
 		}
-		if lu, err := generateLogURI(dataStore, logDriver, logOptMap); err != nil {
+		logConfig := &logging.LogConfig{
+			Drivers: []logging.LogDriverConfig{
+				{
+					Driver: logDriver,
+					Opts:   logOptMap,
+				},
+			},
+		}
+		logConfigB, err := json.Marshal(logConfig)
+		if err != nil {
+			return nil, "", nil, err
+		}
+		logConfigFilePath := logging.LogConfigFilePath(dataStore, ns, id)
+		if err = os.WriteFile(logConfigFilePath, logConfigB, 0600); err != nil {
+			return nil, "", nil, err
+		}
+		if lu, err := generateLogURI(dataStore); err != nil {
 			return nil, "", nil, err
 		} else if lu != nil {
 			logURI = lu.String()
@@ -758,23 +774,14 @@ func withBindMountHostProcfs(_ context.Context, _ oci.Client, _ *containers.Cont
 	return nil
 }
 
-func generateLogURI(dataStore, logDriver string, logOptMap map[string]string) (*url.URL, error) {
-	var selfExe string
-	if logDriver == "json-file" {
-		var err error
-		selfExe, err = os.Executable()
-		if err != nil {
-			return nil, err
-		}
-	} else {
-		return nil, fmt.Errorf("%s is not yet supported", logDriver)
+func generateLogURI(dataStore string) (*url.URL, error) {
+	selfExe, err := os.Executable()
+	if err != nil {
+		return nil, err
 	}
 	args := map[string]string{
 		logging.MagicArgv1: dataStore,
 	}
-	for k, v := range logOptMap {
-		args[k] = v
-	}
 	if runtime.GOOS == "windows" {
 		return nil, nil
 	}

docs/dir.md

@@ -31,6 +31,7 @@ e.g. `/var/lib/nerdctl/1935db59/containers/default/c4ed811cc361d26faffdee8d696dd
 Files:
 - `resolv.conf`: mounted to the container as `/etc/resolv.conf`
 - `hostname`: mounted to the container as `/etc/hostname`
+- `log-config.json`: used for storing the `--log-opts` map of `nerdctl run`
 - `<CID>-json.log`: used by `nerdctl logs`
 - `oci-hook.*.log`: logs of the OCI hook
 

pkg/logging/logging.go

@@ -18,6 +18,7 @@ package logging
 
 import (
 	"context"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"os"
@@ -40,24 +41,64 @@ const (
 // Main is the entrypoint for the containerd runtime v2 logging plugin mode.
 //
 // Should be called only if argv1 == MagicArgv1.
-func Main(argsMap map[string]string) error {
-	fn, err := getLoggerFunc(argsMap)
+func Main(argv2 string) error {
+	fn, err := getLoggerFunc(argv2)
 	if err != nil {
 		return err
 	}
 	logging.Run(fn)
 	return nil
 }
 
-func getLoggerFunc(argsMap map[string]string) (logging.LoggerFunc, error) {
-	if argsMap[MagicArgv1] == "" {
+// LogConfig is marshalled as "log-config.json"
+type LogConfig struct {
+	Drivers []LogDriverConfig `json:"drivers,omitempty"`
+}
+
+// LogDriverConfig is defined per a driver
+type LogDriverConfig struct {
+	Driver string            `json:"driver"`
+	Opts   map[string]string `json:"opts,omitempty"`
+}
+
+// LogConfigFilePath returns the path of log-config.json
+func LogConfigFilePath(dataStore, ns, id string) string {
+	return filepath.Join(dataStore, "containers", ns, id, "log-config.json")
+}
+
+func getLoggerFunc(dataStore string) (logging.LoggerFunc, error) {
+	if dataStore == "" {
 		return nil, errors.New("got empty data store")
 	}
 	return func(_ context.Context, config *logging.Config, ready func() error) error {
 		if config.Namespace == "" || config.ID == "" {
 			return errors.New("got invalid config")
 		}
-		logJSONFilePath := jsonfile.Path(argsMap[MagicArgv1], config.Namespace, config.ID)
+		jsonFileDriverOpts := make(map[string]string)
+		logConfigFilePath := LogConfigFilePath(dataStore, config.Namespace, config.ID)
+		if _, err := os.Stat(logConfigFilePath); err == nil {
+			var logConfig LogConfig
+			logConfigFileB, err := os.ReadFile(logConfigFilePath)
+			if err != nil {
+				return err
+			}
+			if err = json.Unmarshal(logConfigFileB, &logConfig); err != nil {
+				return err
+			}
+			for _, f := range logConfig.Drivers {
+				switch f.Driver {
+				case "json-file":
+					jsonFileDriverOpts = f.Opts
+				default:
+					return fmt.Errorf("unknown driver %q", f.Driver)
+				}
+			}
+		} else if !errors.Is(err, os.ErrNotExist) {
+			// the file does not exist if the container was created with nerdctl < 0.20
+			return err
+		}
+
+		logJSONFilePath := jsonfile.Path(dataStore, config.Namespace, config.ID)
 		if err := os.MkdirAll(filepath.Dir(logJSONFilePath), 0700); err != nil {
 			return err
 		}
@@ -70,7 +111,7 @@ func getLoggerFunc(argsMap map[string]string) (logging.LoggerFunc, error) {
 		//maxSize Defaults to unlimited.
 		var capVal int64
 		capVal = -1
-		if capacity, ok := argsMap[MaxSize]; ok {
+		if capacity, ok := jsonFileDriverOpts[MaxSize]; ok {
 			var err error
 			capVal, err = units.FromHumanSize(capacity)
 			if err != nil {
@@ -82,7 +123,7 @@ func getLoggerFunc(argsMap map[string]string) (logging.LoggerFunc, error) {
 		}
 		l.MaxBytes = capVal
 		maxFile := 1
-		if maxFileString, ok := argsMap[MaxFile]; ok {
+		if maxFileString, ok := jsonFileDriverOpts[MaxFile]; ok {
 			var err error
 			maxFile, err = strconv.Atoi(maxFileString)
 			if err != nil {