refactor: make spec created more reasonable

daemon/mgr/container.go

@@ -312,7 +312,7 @@ func (mgr *ContainerManager) StartExec(ctx context.Context, execid string, confi
 		c.meta.Config.User = execConfig.User
 	}
 
-	if err = setupProcessUser(ctx, c.meta, &SpecWrapper{s: &specs.Spec{Process: process}}); err != nil {
+	if err = setupUser(ctx, c.meta, &specs.Spec{Process: process}); err != nil {
 		return err
 	}
 
@@ -607,32 +607,17 @@ func (mgr *ContainerManager) start(ctx context.Context, c *Container, detachKeys
 }
 
 func (mgr *ContainerManager) createContainerdContainer(ctx context.Context, c *Container) error {
-	// new a default spec.
-	s, err := ctrd.NewDefaultSpec(ctx, c.ID())
-	if err != nil {
-		return errors.Wrapf(err, "failed to generate spec: %s", c.ID())
-	}
-
-	var cgroupsParent string
-	if c.meta.HostConfig.CgroupParent != "" {
-		cgroupsParent = c.meta.HostConfig.CgroupParent
-	} else if mgr.Config.CgroupParent != "" {
-		cgroupsParent = mgr.Config.CgroupParent
-	}
-
-	// cgroupsPath must be absolute path
-	// call filepath.Clean is to avoid bad
-	// path just like../../../.../../BadPath
-	if cgroupsParent != "" {
-		if !filepath.IsAbs(cgroupsParent) {
-			cgroupsParent = filepath.Clean("/" + cgroupsParent)
-		}
-
-		s.Linux.CgroupsPath = filepath.Join(cgroupsParent, c.ID())
+	// CgroupParent from HostConfig will be first priority to use,
+	// then will be value from mgr.Config.CgroupParent
+	if c.meta.HostConfig.CgroupParent == "" {
+		c.meta.HostConfig.CgroupParent = mgr.Config.CgroupParent
 	}
 
-	var prioArr []int
-	var argsArr [][]string
+	var (
+		err     error
+		prioArr []int
+		argsArr [][]string
+	)
 	if mgr.containerPlugin != nil {
 		prioArr, argsArr, err = mgr.containerPlugin.PreStart(c)
 		if err != nil {
@@ -641,34 +626,28 @@ func (mgr *ContainerManager) createContainerdContainer(ctx context.Context, c *C
 	}
 
 	sw := &SpecWrapper{
-		s:       s,
 		ctrMgr:  mgr,
 		volMgr:  mgr.VolumeMgr,
 		netMgr:  mgr.NetworkMgr,
 		prioArr: prioArr,
 		argsArr: argsArr,
 	}
 
-	for _, setup := range SetupFuncs() {
-		if err = setup(ctx, c.meta, sw); err != nil {
-			return err
-		}
+	if err = createSpec(ctx, c.meta, sw); err != nil {
+		return err
 	}
 
 	// open container's stdio.
 	io, err := mgr.openContainerIO(c.ID(), c.meta.Config.OpenStdin)
 	if err != nil {
 		return errors.Wrap(err, "failed to open io")
 	}
-	if io.Stdin != nil && io.Stdin.OpenStdin() {
-		s.Process.Terminal = true
-	}
 
 	if err := mgr.Client.CreateContainer(ctx, &ctrd.Container{
 		ID:      c.ID(),
 		Image:   c.Image(),
 		Runtime: c.meta.HostConfig.Runtime,
-		Spec:    s,
+		Spec:    sw.s,
 		IO:      io,
 	}); err != nil {
 		logrus.Errorf("failed to create new containerd container: %s", err.Error())

daemon/mgr/spec.go

@@ -3,7 +3,10 @@ package mgr
 import (
 	"context"
 
+	"github.com/alibaba/pouch/ctrd"
+
 	specs "github.com/opencontainers/runtime-spec/specs-go"
+	"github.com/pkg/errors"
 )
 
 // SpecWrapper wraps the container's specs and add manager operations.
@@ -17,79 +20,46 @@ type SpecWrapper struct {
 	argsArr [][]string
 }
 
-// SetupFunc defines spec setup function type.
-type SetupFunc func(ctx context.Context, m *ContainerMeta, s *SpecWrapper) error
-
-var setupFunc = []SetupFunc{
-	// process
-	setupProcessArgs,
-	setupProcessCwd,
-	setupProcessEnv,
-	setupProcessTTY,
-	setupProcessUser,
-	setupCap,
-	setupNoNewPrivileges,
-	setupOOMScoreAdj,
-
-	// cgroup
-	setupCgroupCPUShare,
-	setupCgroupCPUSet,
-	setupCgroupCPUPeriod,
-	setupCgroupCPUQuota,
-	setupCgroupMemory,
-	setupCgroupMemorySwap,
-	setupCgroupMemorySwappiness,
-	setupDisableOOMKill,
-
-	// namespaces
-	setupUserNamespace,
-	setupNetworkNamespace,
-	setupIpcNamespace,
-	setupPidNamespace,
-	setupUtsNamespace,
-
-	// volume spec
-	setupMounts,
-
-	// network spec
-	setupNetwork,
-
-	// host device spec
-	setupDevices,
-
-	// linux-platform-specifc spec
-	setupSysctl,
-	setupAppArmor,
-	setupCapabilities,
-	setupSeccomp,
-	setupSELinux,
+// createSpec create a runtime-spec.
+func createSpec(ctx context.Context, c *ContainerMeta, specWrapper *SpecWrapper) error {
+	// new a default spec from containerd.
+	s, err := ctrd.NewDefaultSpec(ctx, c.ID)
+	if err != nil {
+		return errors.Wrapf(err, "failed to generate spec: %s", c.ID)
+	}
+	specWrapper.s = s
 
-	// blkio spec
-	setupBlkio,
-	setupDiskQuota,
+	s.Hostname = c.Config.Hostname.String()
+	s.Root = &specs.Root{
+		Path:     c.BaseFS,
+		Readonly: c.HostConfig.ReadonlyRootfs,
+	}
 
-	// IntelRdtL3Cbm
-	setupIntelRdt,
+	// create Spec.Process spec
+	if err := setupProcess(ctx, c, s); err != nil {
+		return err
+	}
 
-	// annotations in spec
-	setupAnnotations,
+	// create Spec.Mounts spec
+	if err := setupMounts(ctx, c, s); err != nil {
+		return err
+	}
 
-	// rootfs spec
-	setupRoot,
+	// create Spec.Annotations
+	if err := setupAnnotations(ctx, c, s); err != nil {
+		return err
+	}
 
-	//hook
-	setupHook,
-}
+	// create Spec.Hooks spec
+	if err := setupHook(ctx, c, specWrapper); err != nil {
+		return err
+	}
 
-// Register is used to registe spec setup function.
-func Register(f SetupFunc) {
-	if setupFunc == nil {
-		setupFunc = make([]SetupFunc, 0)
+	// platform-specifed spec setting
+	// TODO: support window and Solaris platform
+	if err := populatePlatform(ctx, c, specWrapper); err != nil {
+		return err
 	}
-	setupFunc = append(setupFunc, f)
-}
 
-// SetupFuncs returns all the spec setup functions.
-func SetupFuncs() []SetupFunc {
-	return setupFunc
+	return nil
 }

daemon/mgr/spec_annotations.go

@@ -4,16 +4,16 @@ import (
 	"context"
 	"strconv"
 
+	specs "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/sirupsen/logrus"
 )
 
 // setupAnnotations extracts other related options from HostConfig and locate them in spec's annotations which will be dealt by vendored runc.
-func setupAnnotations(ctx context.Context, meta *ContainerMeta, spec *SpecWrapper) error {
-	s := spec.s
-
-	r := meta.HostConfig.Resources
-
-	s.Annotations = make(map[string]string)
+func setupAnnotations(ctx context.Context, c *ContainerMeta, s *specs.Spec) error {
+	if s.Annotations == nil {
+		s.Annotations = make(map[string]string)
+	}
+	r := c.HostConfig.Resources
 
 	if r.MemoryWmarkRatio != nil {
 		s.Annotations["__memory_wmark_ratio"] = strconv.FormatInt(*r.MemoryWmarkRatio, 10)
@@ -28,7 +28,7 @@ func setupAnnotations(ctx context.Context, meta *ContainerMeta, spec *SpecWrappe
 	s.Annotations["__schedule_latency_switch"] = strconv.FormatInt(r.ScheLatSwitch, 10)
 
 	// add additional spec annotations
-	annotations := meta.Config.SpecAnnotation
+	annotations := c.Config.SpecAnnotation
 	for k, v := range annotations {
 		if _, exist := s.Annotations[k]; exist {
 			logrus.Warnf("Duplicate spec annotation: %s=%s", k, v)