refact: refact disk quota api #2704

rudyfly · 2019-02-03T04:27:02Z

Ⅰ. Describe what this PR did

for pouch cli command, support two api:

    --disk-quota []string
    --quota-id  string

--disk-quota can be set:

--disk-quota=60G
--disk-quota=/abc=60G
--disk-quota=/&/abc=60G
--disk-quota=.*=60G

--quota-id can be set:

--quota-id=0, or haven't set quota id(--quota-id="")
--quota-id=-1
--quota-id=16777216, or more than 16777216

detail rules you can see disk quota documents.

validate disk quota config when create container,
add test case for disk quota.

Ⅱ. Does this pull request fix one issue?

NONE

Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗？)

added

Ⅳ. Describe how to verify it

run disk quota test case

Ⅴ. Special notes for reviews

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

codecov · 2019-02-03T04:27:03Z

Codecov Report

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

@@           Coverage Diff            @@
##             master   #2704   +/-   ##
========================================
  Coverage          ?   69.3%           
========================================
  Files             ?     278           
  Lines             ?   17424           
  Branches          ?       0           
========================================
  Hits              ?   12076           
  Misses            ?    4023           
  Partials          ?    1325

Flag	Coverage Δ
#criv1alpha2_test	`39.43% <39.68%> (?)`
#integration_test_0	`36.61% <26.98%> (?)`
#integration_test_1	`35.54% <24.6%> (?)`
#integration_test_2	`36.6% <24.6%> (?)`
#integration_test_3	`35.48% <24.6%> (?)`
#node_e2e_test	`35.33% <24.6%> (?)`
#unittest	`27.55% <16.66%> (?)`

Impacted Files	Coverage Δ
apis/opts/diskquota.go	`93.93% <100%> (ø)`
storage/quota/quota.go	`13.45% <100%> (ø)`
daemon/mgr/container_validation.go	`42.92% <40.9%> (ø)`
daemon/mgr/container.go	`60.58% <45.45%> (ø)`
daemon/mgr/container_storage.go	`60.34% <47.14%> (ø)`

fuweid · 2019-02-11T03:23:57Z

daemon/mgr/container_storage.go:476:26:warning: should drop = 0 from declaration of var globalQuotaID; it is the zero value (golint)

The code-check CI fails. Please update the code. Thanks

allencloud · 2019-02-12T03:59:32Z

Diskquota is a very essential feature for users. But I am afraid that this is not a general feature on any os distributions. So, FWIW, shall we make diskquota feature a plugin which could be enabled or disabled explicitly. @rudyfly

zjumoon01 · 2019-02-15T10:20:57Z

docs/features/pouch_with_diskquota.md


-> valid `quota-id` which is larger than 0 is only used in `upgrade` interface. In this scenario of triggering `upgrade` interface, pouchd will remove the old container and use the new image to take place of old container's image, and create a new container which should inherit the original diskquota. Then user can pass an original `quota-id` of original container to take effect on newly created container.
+> 1. `rule1` can't use with `rule2/rule3/rule4` together;
+> 2. if it has set quota-id isn't null or `0`, `--disk-quota` only can been set with one rule, or means the length of disk quota slice is `1`.


sed "if it has set quota-id/quota-id"

zjumoon01 · 2019-02-15T10:25:36Z

docs/features/pouch_with_diskquota.md

+> 2. if it has set quota-id isn't null or `0`, `--disk-quota` only can been set with one rule, or means the length of disk quota slice is `1`.
+> 3. `rule3` use `&` link with directory that must be absolute path, can't be regular expression with `rule4`;
+> 4. no special characters(`* & ;`) can exist in all mount points
+> 5. valid `quota-id` which is larger than `16777216` is only used in `upgrade` interface. In this scenario of triggering `upgrade` interface, pouchd will remove the old container and use the new image to take place of old container's image, and create a new container which should inherit the original diskquota. Then user can pass an original `quota-id` of original container to take effect on newly created container.


Using a given quota-id is a common scene, not only in 'upgrade' interface.

daemon/mgr/container_validation.go

daemon/mgr/container_storage.go

fuweid · 2019-02-26T07:43:39Z

daemon/mgr/container_storage.go

-	)
-
-	if c.Config.QuotaID != "" {
+	if c.Config.QuotaID != "" && c.Config.QuotaID != "0" {


maybe we need lock here? This action will take long time. If we don't add lock, there maybe some confusing with concurrent update request

update and create should be conflict, so I think no need to add lock here

daemon/mgr/container_validation.go

zjumoon01 · 2019-02-26T09:52:53Z

docs/features/pouch_with_diskquota.md

+There are four ways to identify the input format:
+
+* rule1: `--disk-quota=10GB` : maps container rootfs and all potential volumes binded inside;
+* rule2: `--disk-quota=/abc=10GB` : absolute path matching, maps only mount point `/abc` has been limited, container rootfs and any volume haven't been limited;


any 'other' volumes haven't been limited

docs/features/pouch_with_diskquota.md

fuweid · 2019-02-28T02:17:59Z

test/cli_run_volume_test.go

 		"--name", containerName, busyboxImage, "df")
 	defer DelContainerForceMultyTime(c, containerName)
 	ret.Assert(c, icmd.Success)

 	out := ret.Stdout()
+	fmt.Printf("%s\n", out)


remove this debug line?

for pouch cli command, support two api: ```bash --disk-quota []string --quota-id string ``` --disk-quota can be set: 1. --disk-quota=60G 2. --disk-quota=/abc=60G 3. --disk-quota=/&/abc=60G 4. --disk-quota=.*=60G --quota-id can be set: 1. --quota-id=0, or haven't set quota id(--quota-id="") 2. --quota-id=-1 3. --quota-id=16777216, or more than 16777216 detail rules you can see disk quota documents. validate disk quota config when create container, add test case for disk quota. Signed-off-by: Rudy Zhang <rudyflyzhang@gmail.com>

fuweid

LGTM

pouchrobot added the size/XL label Feb 3, 2019

rudyfly requested review from HusterWan, allencloud and fuweid February 3, 2019 04:27

weekly-digest bot mentioned this pull request Feb 10, 2019

Weekly Digest (3 February, 2019 - 10 February, 2019) #2707

Closed

rudyfly force-pushed the diskquota branch 3 times, most recently from 66bebb9 to 37c682a Compare February 15, 2019 09:15

zjumoon01 reviewed Feb 15, 2019

View reviewed changes

weekly-digest bot mentioned this pull request Feb 17, 2019

Weekly Digest (10 February, 2019 - 17 February, 2019) #2714

Closed

rudyfly force-pushed the diskquota branch 2 times, most recently from 66deaaf to 505019a Compare February 20, 2019 12:44

weekly-digest bot mentioned this pull request Feb 24, 2019

Weekly Digest (17 February, 2019 - 24 February, 2019) #2723

Closed

rudyfly force-pushed the diskquota branch from 505019a to e5a1f4f Compare February 25, 2019 12:54

rudyfly changed the title ~~[WIP]refact: refact disk quota api~~ refact: refact disk quota api Feb 25, 2019

rudyfly force-pushed the diskquota branch from e5a1f4f to 852c9ae Compare February 25, 2019 13:33

fuweid reviewed Feb 26, 2019

View reviewed changes

daemon/mgr/container_validation.go Show resolved Hide resolved

fuweid reviewed Feb 26, 2019

View reviewed changes

rudyfly force-pushed the diskquota branch from 852c9ae to 5ee20a6 Compare February 26, 2019 13:19

zjumoon01 reviewed Feb 27, 2019

View reviewed changes

fuweid reviewed Feb 28, 2019

View reviewed changes

docs/features/pouch_with_diskquota.md Outdated Show resolved Hide resolved

fuweid reviewed Feb 28, 2019

View reviewed changes

rudyfly force-pushed the diskquota branch from 5ee20a6 to 0a1f967 Compare February 28, 2019 02:39

fuweid reviewed Feb 28, 2019

View reviewed changes

fuweid merged commit a373ba9 into AliyunContainerService:master Feb 28, 2019

pouchrobot mentioned this pull request Mar 1, 2019

WeeklyReport of PouchContainer from 2019-02-22 to 2019-03-01 #2731

Closed

weekly-digest bot mentioned this pull request Mar 3, 2019

Weekly Digest (24 February, 2019 - 3 March, 2019) #2732

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refact: refact disk quota api #2704

refact: refact disk quota api #2704

rudyfly commented Feb 3, 2019 •

edited

Loading

codecov bot commented Feb 3, 2019 •

edited

Loading

fuweid commented Feb 11, 2019

allencloud commented Feb 12, 2019

zjumoon01 Feb 15, 2019

rudyfly Feb 20, 2019

zjumoon01 Feb 15, 2019

rudyfly Feb 20, 2019

fuweid Feb 26, 2019

rudyfly Feb 26, 2019

zjumoon01 Feb 26, 2019

fuweid Feb 28, 2019

rudyfly Feb 28, 2019

fuweid left a comment

refact: refact disk quota api #2704

refact: refact disk quota api #2704

Conversation

rudyfly commented Feb 3, 2019 • edited Loading

Ⅰ. Describe what this PR did

Ⅱ. Does this pull request fix one issue?

Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗？)

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

codecov bot commented Feb 3, 2019 • edited Loading

Codecov Report

fuweid commented Feb 11, 2019

allencloud commented Feb 12, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fuweid left a comment

Choose a reason for hiding this comment

rudyfly commented Feb 3, 2019 •

edited

Loading

codecov bot commented Feb 3, 2019 •

edited

Loading