refactor: change for the reviewer

1. s/optionsForContainerio/logOptionsForContainerio/g
2. make the convertKVStringsToMap into pkg/utils
3. fix typo

Signed-off-by: Wei Fu <[email protected]>

Author: Wei Fu

apis/opts/log_options.go

@@ -1,14 +1,14 @@
 package opts
 
 import (
-	"errors"
 	"fmt"
-	"strings"
+
+	"github.com/alibaba/pouch/pkg/utils"
 )
 
 // ParseLogOptions parses [key=value] slice-type log options into map.
 func ParseLogOptions(driver string, logOpts []string) (map[string]string, error) {
-	opts, err := convertKVStringsToMap(logOpts)
+	opts, err := utils.ConvertKVStringsToMap(logOpts)
 	if err != nil {
 		return nil, err
 	}
@@ -18,19 +18,3 @@ func ParseLogOptions(driver string, logOpts []string) (map[string]string, error)
 	}
 	return opts, nil
 }
-
-// convertKVStringsToMap converts ["key=value"] into {"key":"value"}
-//
-// TODO(fuwei): make it common in the opts.ParseXXX().
-func convertKVStringsToMap(values []string) (map[string]string, error) {
-	kvs := make(map[string]string, len(values))
-
-	for _, value := range values {
-		terms := strings.SplitN(value, "=", 2)
-		if len(terms) != 2 {
-			return nil, errors.New("the format must be key=value")
-		}
-		kvs[terms[0]] = terms[1]
-	}
-	return kvs, nil
-}

daemon/logger/syslog/validate.go

@@ -25,7 +25,7 @@ var (
 	// ErrInvalidSyslogFormat represents the invalid format.
 	ErrInvalidSyslogFormat = errors.New("invalid syslog format")
 
-	// ErrFailedToLoadX509KeyPair is to used to indicate that it's failed to load x590 key pair.
+	// ErrFailedToLoadX509KeyPair is to used to indicate that it's failed to load x509 key pair.
 	ErrFailedToLoadX509KeyPair = errors.New("fail to load x509 key pair")
 
 	fmtErrInvalidAddressFormat = "syslog-address must be in form proto://address, but got %v"

daemon/mgr/container.go

@@ -1475,7 +1475,7 @@ func (mgr *ContainerManager) openContainerIO(c *Container) (*containerio.IO, err
 		containerio.WithStdin(c.Config.OpenStdin),
 	}
 
-	options = append(options, optionsForContainerio(c)...)
+	options = append(options, logOptionsForContainerio(c)...)
 
 	io := containerio.NewIO(containerio.NewOption(options...))
 	mgr.IOs.Put(c.ID, io)
@@ -1612,7 +1612,7 @@ func (mgr *ContainerManager) openAttachIO(c *Container, attach *AttachConfig) (*
 		containerio.WithID(c.ID),
 		containerio.WithLoggerInfo(logInfo),
 	}
-	options = append(options, optionsForContainerio(c)...)
+	options = append(options, logOptionsForContainerio(c)...)
 
 	if attach != nil {
 		options = append(options, attachConfigToOptions(attach)...)

daemon/mgr/container_logger.go

@@ -8,7 +8,7 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
-func optionsForContainerio(c *Container) []func(*containerio.Option) {
+func logOptionsForContainerio(c *Container) []func(*containerio.Option) {
 	optFuncs := make([]func(*containerio.Option), 0, 1)
 
 	cfg := c.HostConfig.LogConfig

pkg/utils/utils.go

@@ -336,3 +336,17 @@ func SetOOMScore(pid, score int) error {
 	f.Close()
 	return err
 }
+
+// ConvertKVStringsToMap converts ["key=value"] into {"key":"value"}
+func ConvertKVStringsToMap(values []string) (map[string]string, error) {
+	kvs := make(map[string]string, len(values))
+
+	for _, value := range values {
+		terms := strings.SplitN(value, "=", 2)
+		if len(terms) != 2 {
+			return nil, errors.New("the format must be key=value")
+		}
+		kvs[terms[0]] = terms[1]
+	}
+	return kvs, nil
+}

pkg/utils/utils_test.go

@@ -430,3 +430,54 @@ func TestParseTimestamp(t *testing.T) {
 		}
 	}
 }
+
+func TestConvertKVStringsToMap(t *testing.T) {
+	type tCases struct {
+		input    []string
+		expected map[string]string
+		hasError bool
+	}
+
+	for idx, tc := range []tCases{
+		{
+			input:    nil,
+			expected: map[string]string{},
+			hasError: false,
+		}, {
+			input:    []string{"withoutValue"},
+			expected: nil,
+			hasError: true,
+		}, {
+			input: []string{"key=value"},
+			expected: map[string]string{
+				"key": "value",
+			},
+			hasError: false,
+		}, {
+			input: []string{"key=key=value"},
+			expected: map[string]string{
+				"key": "key=value",
+			},
+			hasError: false,
+		}, {
+			input: []string{"test=1", "flag=oops", "test=2"},
+			expected: map[string]string{
+				"test": "2",
+				"flag": "oops",
+			},
+			hasError: false,
+		},
+	} {
+		got, err := ConvertKVStringsToMap(tc.input)
+		if err == nil && tc.hasError {
+			t.Fatalf("[%d case] should have error here, but got nothing", idx)
+		}
+		if err != nil && !tc.hasError {
+			t.Fatalf("[%d case] should have no error here, but got error(%v)", idx, err)
+		}
+
+		if !reflect.DeepEqual(got, tc.expected) {
+			t.Fatalf("[%d case] should have (%v), but got (%v)", idx, tc.expected, got)
+		}
+	}
+}