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: add options when using volume list #1028

Merged
merged 1 commit into from
Apr 3, 2018

Conversation

rudyfly
Copy link
Collaborator

@rudyfly rudyfly commented Apr 2, 2018

Ⅰ. Describe what this PR did

Add options when using volume list, add volume driver into default
volume list information. Add list volumes function in volume core module.

Ⅱ. Does this pull request fix one issue?

fixes #921

Ⅲ. Describe how you did it

Ⅳ. Describe how to verify it

listing volume format is "DRIVER VOLUME NAME", you can add some options, such as size and mountpoint, list volume with options like this:

# pouch volume list --size --mountpoint
DRIVER   VOLUME NAME     SIZE     MOUNT POINT
local    volume1                      ulimit   /mnt/local/volume1
local    test                             1g       /data/volume/test
local    volume-test               10g      /data/volume/volume-test

Ⅴ. Special notes for reviews

Signed-off-by: Rudy Zhang rudyflyzhang@gmail.com

@HusterWan
Copy link
Contributor

@rudyfly code conflict, please rebase your code, thanks a lot.

@rudyfly rudyfly force-pushed the volume-list branch 2 times, most recently from 733691a to 7d96f9b Compare April 2, 2018 06:07
@@ -191,6 +191,11 @@ func (v *Volume) Key() string {
return v.Name
}

//CreateTime returns the volume's create time.
func (v *Volume) CreateTime() string {
return v.CreationTimestamp.Format("2006-1-2 15:04:05")
Copy link
Collaborator

Choose a reason for hiding this comment

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

I found that in the definition we have:

	// CreationTimestamp is a timestamp representing the server time when this object was
	// created. It is not guaranteed to be set in happens-before order across separate operations.
	// Clients may not set this value. It is represented in RFC3339 form and is in UTC.
	CreationTimestamp *time.Time `json:"CreationTimestamp,omitempty"`

It seems to be a pointer, so I am wondering if we need to detect it is non-nil ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added.

@codecov-io
Copy link

codecov-io commented Apr 2, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@85d808d). Click here to learn what that means.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1028   +/-   ##
=========================================
  Coverage          ?   15.15%           
=========================================
  Files             ?      135           
  Lines             ?     8499           
  Branches          ?        0           
=========================================
  Hits              ?     1288           
  Misses            ?     7110           
  Partials          ?      101
Impacted Files Coverage Δ
cli/volume.go 8.67% <0%> (ø)
daemon/mgr/volume.go 0% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 85d808d...19932e8. Read the comment docs.

Add options when using volume list, add volume driver into default
volume list information. Add list volumes function in volume core module.

Signed-off-by: Rudy Zhang <rudyflyzhang@gmail.com>

for _, v := range volumeList.Volumes {
display.AddRow([]string{v.Name})
displayHead := []string{"DRIVER", "VOLUME NAME"}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you help to change VOLUME NAME into VOLUME_NAME to avoid others to think that NAME is another column? @rudyfly

@allencloud
Copy link
Collaborator

LGTM

@pouchrobot pouchrobot added the LGTM one maintainer or community participant agrees to merge the pull reuqest. label Apr 3, 2018
@allencloud allencloud merged commit 800c68a into AliyunContainerService:master Apr 3, 2018
@rudyfly rudyfly deleted the volume-list branch October 29, 2018 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
areas/storage kind/feature LGTM one maintainer or community participant agrees to merge the pull reuqest. size/XL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[feature required] pouch volume ls output driver
5 participants