refactor: mv cli params parse and validate part of code into opts package

cli/update.go

@@ -4,6 +4,7 @@ import (
 	"context"
 
 	"github.com/alibaba/pouch/apis/types"
+	"github.com/alibaba/pouch/pkg/opts"
 	"github.com/alibaba/pouch/pkg/reference"
 
 	"github.com/spf13/cobra"
@@ -58,21 +59,21 @@ func (uc *UpdateCommand) updateRun(args []string) error {
 	container := args[0]
 	ctx := context.Background()
 
-	labels, err := parseLabels(uc.labels)
+	labels, err := opts.ParseLabels(uc.labels)
 	if err != nil {
 		return err
 	}
 
-	if err := validateMemorySwappiness(uc.memorySwappiness); err != nil {
+	if err := opts.ValidateMemorySwappiness(uc.memorySwappiness); err != nil {
 		return err
 	}
 
-	memory, err := parseMemory(uc.memory)
+	memory, err := opts.ParseMemory(uc.memory)
 	if err != nil {
 		return err
 	}
 
-	memorySwap, err := parseMemorySwap(uc.memorySwap)
+	memorySwap, err := opts.ParseMemorySwap(uc.memorySwap)
 	if err != nil {
 		return err
 	}
@@ -87,7 +88,7 @@ func (uc *UpdateCommand) updateRun(args []string) error {
 		BlkioWeight:      uc.blkioWeight,
 	}
 
-	restartPolicy, err := parseRestartPolicy(uc.restartPolicy)
+	restartPolicy, err := opts.ParseRestartPolicy(uc.restartPolicy)
 	if err != nil {
 		return err
 	}

daemon/mgr/container_validation.go

@@ -0,0 +1,13 @@
+package mgr
+
+import (
+	"github.com/alibaba/pouch/apis/types"
+)
+
+// verifyContainerSetting is to verify the correctness of hostconfig and config.
+func (mgr *ContainerManager) verifyContainerSetting(hostConfig *types.HostConfig, config *types.ContainerConfig) error {
+	if config != nil {
+		// TODO
+	}
+	return nil
+}

daemon/mgr/spec_devices.go

@@ -7,7 +7,7 @@ import (
 	"path/filepath"
 	"strings"
 
-	"github.com/alibaba/pouch/pkg/runconfig"
+	"github.com/alibaba/pouch/pkg/opts"
 
 	"github.com/opencontainers/runc/libcontainer/configs"
 	"github.com/opencontainers/runc/libcontainer/devices"
@@ -109,7 +109,7 @@ func setupDevices(ctx context.Context, meta *ContainerMeta, spec *SpecWrapper) e
 		}
 	} else {
 		for _, deviceMapping := range meta.HostConfig.Devices {
-			if !runconfig.ValidDeviceMode(deviceMapping.CgroupPermissions) {
+			if !opts.ValidateDeviceMode(deviceMapping.CgroupPermissions) {
 				return fmt.Errorf("%s invalid device mode: %s", deviceMapping.PathOnHost, deviceMapping.CgroupPermissions)
 			}
 			d, dPermissions, err := devicesFromPath(deviceMapping.PathOnHost, deviceMapping.PathInContainer, deviceMapping.CgroupPermissions)

pkg/runconfig/hostconfig.go → pkg/opts/devicemappings.go

@@ -1,4 +1,4 @@
-package runconfig
+package opts
 
 import (
 	"fmt"
@@ -7,8 +7,27 @@ import (
 	"github.com/alibaba/pouch/apis/types"
 )
 
-// ParseDevice parses a device mapping string to a container.DeviceMapping struct
-func ParseDevice(device string) (*types.DeviceMapping, error) {
+// ParseDeviceMappings parse devicemappings
+func ParseDeviceMappings(devices []string) ([]*types.DeviceMapping, error) {
+	results := []*types.DeviceMapping{}
+	for _, device := range devices {
+		deviceMapping, err := parseDevice(device)
+		if err != nil {
+			return nil, fmt.Errorf("failed to parse devices: %v", err)
+		}
+
+		if !ValidateDeviceMode(deviceMapping.CgroupPermissions) {
+			return nil, fmt.Errorf("%s invalid device mode: %s", device, deviceMapping.CgroupPermissions)
+		}
+
+		results = append(results, deviceMapping)
+	}
+	return results, nil
+
+}
+
+// parseDevice parses a device mapping string to a container.DeviceMapping struct
+func parseDevice(device string) (*types.DeviceMapping, error) {
 	src := ""
 	dst := ""
 	permissions := "rwm"
@@ -38,9 +57,9 @@ func ParseDevice(device string) (*types.DeviceMapping, error) {
 	return deviceMapping, nil
 }
 
-// ValidDeviceMode checks if the mode for device is valid or not.
+// ValidateDeviceMode checks if the mode for device is valid or not.
 // valid mode is a composition of r (read), w (write), and m (mknod).
-func ValidDeviceMode(mode string) bool {
+func ValidateDeviceMode(mode string) bool {
 	var legalDeviceMode = map[rune]bool{
 		'r': true,
 		'w': true,

pkg/opts/devicemappings_test.go

@@ -0,0 +1,163 @@
+package opts
+
+import (
+	"fmt"
+	"reflect"
+	"testing"
+
+	"github.com/alibaba/pouch/apis/types"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestParseDeviceMapping(t *testing.T) {
+	type args struct {
+		device []string
+	}
+	tests := []struct {
+		name    string
+		args    args
+		want    []*types.DeviceMapping
+		wantErr bool
+	}{
+		{
+			name: "deviceMapping1",
+			args: args{
+				device: []string{"/dev/deviceName:/dev/deviceName:mrw"},
+			},
+			want: []*types.DeviceMapping{{
+				PathOnHost:        "/dev/deviceName",
+				PathInContainer:   "/dev/deviceName",
+				CgroupPermissions: "mrw",
+			}},
+
+			wantErr: false,
+		},
+		{
+			name: "deviceMapping2",
+			args: args{
+				device: []string{"/dev/deviceName:"},
+			},
+			want: []*types.DeviceMapping{{
+				PathOnHost:        "/dev/deviceName",
+				PathInContainer:   "/dev/deviceName",
+				CgroupPermissions: "rwm",
+			}},
+			wantErr: false,
+		},
+		{
+			name: "deviceMappingWrong1",
+			args: args{
+				device: []string{"/dev/deviceName:/dev/deviceName:rrw"},
+			},
+			wantErr: true,
+		},
+		{
+			name: "deviceMappingWrong2",
+			args: args{
+				device: []string{"/dev/deviceName:/dev/deviceName:arw"},
+			},
+			wantErr: true,
+		},
+		{
+			name: "deviceMappingWrong3",
+			args: args{
+				device: []string{"/dev/deviceName:/dev/deviceName:mrw:mrw"},
+			},
+			wantErr: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := ParseDeviceMappings(tt.args.device)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("parseDeviceMappings() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("parseDeviceMappings() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+type tDeviceModeCase struct {
+	input    string
+	expected bool
+}
+
+type tParseDeviceCase struct {
+	input    string
+	expected *types.DeviceMapping
+	err      error
+}
+
+func Test_parseDevice(t *testing.T) {
+	for _, tc := range []tParseDeviceCase{
+		{
+			input: "/dev/zero:/dev/zero:rwm",
+			expected: &types.DeviceMapping{
+				PathOnHost:        "/dev/zero",
+				PathInContainer:   "/dev/zero",
+				CgroupPermissions: "rwm",
+			},
+			err: nil,
+		}, {
+			input: "/dev/zero:rwm",
+			expected: &types.DeviceMapping{
+				PathOnHost:        "/dev/zero",
+				PathInContainer:   "rwm",
+				CgroupPermissions: "rwm",
+			},
+			err: nil,
+		}, {
+			input: "/dev/zero",
+			expected: &types.DeviceMapping{
+				PathOnHost:        "/dev/zero",
+				PathInContainer:   "/dev/zero",
+				CgroupPermissions: "rwm",
+			},
+			err: nil,
+		}, {
+			input: "/dev/zero:/dev/testzero:rwm",
+			expected: &types.DeviceMapping{
+				PathOnHost:        "/dev/zero",
+				PathInContainer:   "/dev/testzero",
+				CgroupPermissions: "rwm",
+			},
+			err: nil,
+		}, {
+			input:    "/dev/zero:/dev/testzero:rwm:tooLong",
+			expected: nil,
+			err:      fmt.Errorf("invalid device specification: /dev/zero:/dev/testzero:rwm:tooLong"),
+		},
+	} {
+		output, err := parseDevice(tc.input)
+		assert.Equal(t, tc.err, err, tc.input)
+		assert.Equal(t, tc.expected, output, tc.input)
+	}
+}
+
+func TestValidateDeviceMode(t *testing.T) {
+	for _, modeCase := range []tDeviceModeCase{
+		{
+			input:    "rwm",
+			expected: true,
+		}, {
+			input:    "r",
+			expected: true,
+		}, {
+			input:    "rw",
+			expected: true,
+		}, {
+			input:    "rr",
+			expected: false,
+		}, {
+			input:    "rxm",
+			expected: false,
+		},
+	} {
+		isValid := ValidateDeviceMode(modeCase.input)
+		assert.Equal(t, modeCase.expected, isValid, modeCase.input)
+	}
+}

pkg/opts/diskquota.go

@@ -0,0 +1,35 @@
+package opts
+
+import (
+	"fmt"
+	"strings"
+)
+
+// ParseDiskQuota parses diskquota configurations of container.
+func ParseDiskQuota(quotas []string) (map[string]string, error) {
+	var quotaMaps = make(map[string]string)
+
+	for _, quota := range quotas {
+		if quota == "" {
+			return nil, fmt.Errorf("invalid format for disk quota: %s", quota)
+		}
+
+		parts := strings.Split(quota, "=")
+		switch len(parts) {
+		case 1:
+			quotaMaps["/"] = parts[0]
+		case 2:
+			quotaMaps[parts[0]] = parts[1]
+		default:
+			return nil, fmt.Errorf("invalid format for disk quota: %s", quota)
+		}
+	}
+
+	return quotaMaps, nil
+}
+
+// ValidateDiskQuota verifies diskquota configurations of container.
+func ValidateDiskQuota(quotaMaps map[string]string) error {
+	// TODO
+	return nil
+}

pkg/opts/diskquota_test.go

@@ -0,0 +1,36 @@
+package opts
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestParseDiskQuota(t *testing.T) {
+	type args struct {
+		diskquota []string
+	}
+	tests := []struct {
+		name    string
+		args    args
+		want    map[string]string
+		wantErr bool
+	}{
+		// TODO: Add test cases.
+		{name: "test1", args: args{diskquota: []string{""}}, want: nil, wantErr: true},
+		{name: "test2", args: args{diskquota: []string{"foo=foo=foo"}}, want: nil, wantErr: true},
+		{name: "test3", args: args{diskquota: []string{"foo"}}, want: map[string]string{"/": "foo"}, wantErr: false},
+		{name: "test3", args: args{diskquota: []string{"foo=foo"}}, want: map[string]string{"foo": "foo"}, wantErr: false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := ParseDiskQuota(tt.args.diskquota)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("ParseDiskQuota() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("ParseDiskQuota() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

pkg/opts/intel_rdt.go

@@ -0,0 +1,9 @@
+package opts
+
+// ParseIntelRdt parses inter-rdt params of container
+func ParseIntelRdt(intelRdtL3Cbm string) (string, error) {
+	// FIXME: add Intel RDT L3 Cbm validation
+	return intelRdtL3Cbm, nil
+}
+
+// TODO: ValidateInterRdt

pkg/opts/intel_rdt_test.go

@@ -0,0 +1,29 @@
+package opts
+
+import "testing"
+
+func TestParseIntelRdt(t *testing.T) {
+	type args struct {
+		intelRdtL3Cbm string
+	}
+	tests := []struct {
+		name    string
+		args    args
+		want    string
+		wantErr bool
+	}{
+	// TODO: Add test cases.
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := ParseIntelRdt(tt.args.intelRdtL3Cbm)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("parseIntelRdt() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if got != tt.want {
+				t.Errorf("parseIntelRdt() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}