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

feature: set volumes to /etc/mtab #1170

Merged
merged 1 commit into from
May 8, 2018

Conversation

rudyfly
Copy link
Collaborator

@rudyfly rudyfly commented Apr 20, 2018

Ⅰ. Describe what this PR did

set volumes to /etc/mtab in container, it makes the "df" command to show
the volume's information.

Ⅱ. Does this pull request fix one issue?

Ⅲ. Describe how you did it

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

Signed-off-by: Rudy Zhang [email protected]

@allencloud
Copy link
Collaborator

Golint fails:

/go/bin/golint
daemon/mgr/spec_volume.go:93:5: var rootId should be rootID
daemon/mgr/spec_volume.go:113:6: var tmpId should be tmpID

And please also check the integration test. @rudyfly

@rudyfly rudyfly force-pushed the volume branch 3 times, most recently from 6e05eba to f6a840b Compare April 21, 2018 13:51

"github.com/alibaba/pouch/storage/quota"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add a blank line below this line. Thanks a lot.

@allencloud
Copy link
Collaborator

allencloud commented Apr 22, 2018

This will be conflict with pr #1154. Please pay attention.

@codecov-io
Copy link

codecov-io commented May 3, 2018

Codecov Report

Merging #1170 into master will decrease coverage by 0.05%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1170      +/-   ##
==========================================
- Coverage   15.21%   15.15%   -0.06%     
==========================================
  Files         172      172              
  Lines       10735    10772      +37     
==========================================
  Hits         1633     1633              
- Misses       8981     9018      +37     
  Partials      121      121
Impacted Files Coverage Δ
daemon/mgr/container.go 0% <0%> (ø) ⬆️
daemon/mgr/spec_hook.go 0% <0%> (ø) ⬆️

@rudyfly rudyfly changed the title [WIP]feature: set volumes to /etc/mtab feature: set volumes to /etc/mtab May 4, 2018
continue
}

tempLine := fmt.Sprintf("/dev/v%02dd %s ext4 rw 0 0\n", i, m.Destination)
Copy link
Contributor

Choose a reason for hiding this comment

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

variable i should increase?

@@ -10,6 +12,7 @@ import (

"github.com/alibaba/pouch/storage/quota"
Copy link
Contributor

Choose a reason for hiding this comment

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

add a blank line below this line, thanks a lot

set volumes to /etc/mtab in container, it makes the "df" command to show
the volume's information.

Signed-off-by: Rudy Zhang <[email protected]>
@shaloulcy
Copy link
Contributor

LGTM

@rudyfly rudyfly merged commit 77ab865 into AliyunContainerService:master May 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants