Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: refact the setup spec mount #2653

Merged
merged 1 commit into from
Jan 8, 2019
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 48 additions & 24 deletions daemon/mgr/spec_mount.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"strings"

"github.com/alibaba/pouch/apis/types"
"github.com/pkg/errors"

specs "github.com/opencontainers/runtime-spec/specs-go"
"github.com/sirupsen/logrus"
Expand Down Expand Up @@ -40,10 +41,7 @@ func clearReadonly(m *specs.Mount) {
m.Options = opts
}

// setupMounts create mount spec.
func setupMounts(ctx context.Context, c *Container, s *specs.Spec) error {
var mounts []specs.Mount
// Override the default mounts which are duplicate with user defined ones.
func overrideDefaultMount(mounts []specs.Mount, c *Container, s *specs.Spec) ([]specs.Mount, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can add some test cases for the new function.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It isn't complicated logic, I think it no need to add unit test.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should add, but can be in another pr

for _, sm := range s.Mounts {
dup := false
for _, cm := range c.Mounts {
Expand All @@ -55,21 +53,14 @@ func setupMounts(ctx context.Context, c *Container, s *specs.Spec) error {
if dup {
continue
}
if sm.Destination == "/dev/shm" && c.HostConfig.ShmSize != nil &&
*c.HostConfig.ShmSize != 0 {
for idx, v := range sm.Options {
if strings.Contains(v, "size=") {
sm.Options[idx] = fmt.Sprintf("size=%s", strconv.FormatInt(*c.HostConfig.ShmSize, 10))
}
}
}

mounts = append(mounts, sm)
}

if c.HostConfig == nil {
return nil
}
// user defined mount
return mounts, nil
}

func mergeContainerMount(mounts []specs.Mount, c *Container, s *specs.Spec) ([]specs.Mount, error) {
for _, mp := range c.Mounts {
if trySetupNetworkMount(mp, c) {
// ignore the network mount, we will handle it later.
Expand All @@ -79,7 +70,7 @@ func setupMounts(ctx context.Context, c *Container, s *specs.Spec) error {
// check duplicate mountpoint
for _, sm := range mounts {
if sm.Destination == mp.Destination {
return fmt.Errorf("duplicate mount point: %s", mp.Destination)
return nil, fmt.Errorf("duplicate mount point: %s", mp.Destination)
}
}

Expand Down Expand Up @@ -121,21 +112,54 @@ func setupMounts(ctx context.Context, c *Container, s *specs.Spec) error {
mounts = append(mounts, generateNetworkMounts(c)...)
}

s.Mounts = sortMounts(mounts)
return mounts, nil
}

// setupMounts create mount spec.
func setupMounts(ctx context.Context, c *Container, s *specs.Spec) error {
var (
mounts []specs.Mount
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to define this parameter, WDYT?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it need.

err error
)

// Override the default mounts which are duplicate with user defined ones.
mounts, err = overrideDefaultMount(mounts, c, s)
if err != nil {
return errors.Wrap(err, "failed to override default spec mounts")
}

// user defined mount
mounts, err = mergeContainerMount(mounts, c, s)
if err != nil {
return errors.Wrap(err, "failed to merge container mounts")
}

if c.HostConfig.Privileged {
for i := range s.Mounts {
// modify share memory size, and change rw mode for privileged mode.
for i := range mounts {
if mounts[i].Destination == "/dev/shm" && c.HostConfig.ShmSize != nil &&
*c.HostConfig.ShmSize != 0 {
for idx, v := range mounts[i].Options {
if strings.Contains(v, "size=") {
mounts[i].Options[idx] = fmt.Sprintf("size=%s",
strconv.FormatInt(*c.HostConfig.ShmSize, 10))
}
}
}

if c.HostConfig.Privileged {
// Clear readonly for /sys.
if s.Mounts[i].Destination == "/sys" && !s.Root.Readonly {
clearReadonly(&s.Mounts[i])
if mounts[i].Destination == "/sys" && !s.Root.Readonly {
clearReadonly(&mounts[i])
}

// Clear readonly for cgroup
if s.Mounts[i].Type == "cgroup" {
clearReadonly(&s.Mounts[i])
if mounts[i].Type == "cgroup" {
clearReadonly(&mounts[i])
}
}
}

s.Mounts = sortMounts(mounts)
return nil
}

Expand Down