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 request] Support update container's device io limit #2509

Closed
zjumoon01 opened this issue Nov 27, 2018 · 11 comments · Fixed by #2708
Closed

[feature request] Support update container's device io limit #2509

zjumoon01 opened this issue Nov 27, 2018 · 11 comments · Fixed by #2708
Labels
kind/feature kind/feature-request This is a feature request from community for PouchContainer

Comments

@zjumoon01
Copy link
Contributor

Why you need it?

We can set container's block device io limit by when it is created. Such as

pouch run --name test3 --device-read-bps=/dev/xvda:100M registry.hub.docker.com/library/busybox:latest top

There are 4 device io limit options, --device-read-bps/--device-write-bps/--device-read-iops/--device-write-iops. However, we can't update them by using "pouch update" command like other container resource. I will be useful to support update them.

How it could be?

It seems that container update API supports to update device io limit. But pouch update command doesn't.
So pouch update command could be added the four options. After that, we can update(also add and remove) container's device io limit.

Other related information

@pouchrobot pouchrobot added kind/feature kind/feature-request This is a feature request from community for PouchContainer labels Nov 27, 2018
@allencloud
Copy link
Collaborator

I think we have supported all these update feature in the API side https://github.com/alibaba/pouch/blob/master/apis/swagger.yml#L2456-L2479:

  UpdateConfig:
    description: "UpdateConfig holds the mutable attributes of a Container. Those attributes can be updated at runtime."
    allOf:
      - $ref: "#/definitions/Resources"
      - properties:
          RestartPolicy:
            $ref: "#/definitions/RestartPolicy"
          Env:
            description: |
              A list of environment variables to set inside the container in the form `["VAR=value", ...]`. A variable without `=` is removed from the environment, rather than to have an empty value.
            type: "array"
            items:
              type: "string"
          Label:
            description: "List of labels set to container."
            type: "array"
            items:
              type: "string"
          DiskQuota:
            type: "object"
            description: "update disk quota for container"
            x-nullable: true
            additionalProperties:
              type: "string"

But we have not make it supported in COMMAND LINE part. @zjumoon01

Could anyone help to finish this? @fengzixu 🐦 🦁

@houstar
Copy link
Contributor

houstar commented Dec 11, 2018

@allencloud Let's me make it happen

@allencloud
Copy link
Collaborator

@allencloud Let's me make it happen

Thanks a lot. So great to see you could try this. @houstar

@fengzixu
Copy link
Contributor

fengzixu commented Dec 13, 2018

@allencloud sorry. I saw this issue just now. If the @houstar want to implement it, I can help him review his code.

houstar added a commit to houstar/pouch that referenced this issue Dec 15, 2018
fixes AliyunContainerService#2509

Signed-off-by: Leno Hou <lenohou@gmail.com>
houstar added a commit to houstar/pouch that referenced this issue Dec 15, 2018
fixes AliyunContainerService#2509

Signed-off-by: Leno Hou <lenohou@gmail.com>
houstar added a commit to houstar/pouch that referenced this issue Dec 15, 2018
fixes AliyunContainerService#2509

Signed-off-by: Leno Hou <lenohou@gmail.com>
houstar added a commit to houstar/pouch that referenced this issue Dec 16, 2018
fixes AliyunContainerService#2509

Signed-off-by: Leno Hou <lenohou@gmail.com>
@houstar
Copy link
Contributor

houstar commented Dec 17, 2018

@fengzixu Done, Could you take a look ? thanks!

/ Cc @allencloud

houstar added a commit to houstar/pouch that referenced this issue Dec 17, 2018
fixes AliyunContainerService#2509

Signed-off-by: Leno Hou <lenohou@gmail.com>
houstar added a commit to houstar/pouch that referenced this issue Dec 17, 2018
fixes AliyunContainerService#2509

Signed-off-by: Leno Hou <lenohou@gmail.com>
houstar added a commit to houstar/pouch that referenced this issue Dec 17, 2018
fixes AliyunContainerService#2509

Signed-off-by: Leno Hou <lenohou@gmail.com>
houstar added a commit to houstar/pouch that referenced this issue Dec 17, 2018
fixes AliyunContainerService#2509

Signed-off-by: Leno Hou <lenohou@gmail.com>
houstar added a commit to houstar/pouch that referenced this issue Dec 17, 2018
fixes AliyunContainerService#2509

Signed-off-by: Leno Hou <lenohou@gmail.com>
houstar added a commit to houstar/pouch that referenced this issue Dec 17, 2018
fixes AliyunContainerService#2509

Signed-off-by: Leno Hou <lenohou@gmail.com>
houstar added a commit to houstar/pouch that referenced this issue Dec 17, 2018
fixes AliyunContainerService#2509

Signed-off-by: Leno Hou <lenohou@gmail.com>
houstar added a commit to houstar/pouch that referenced this issue Dec 18, 2018
fixes AliyunContainerService#2509

Signed-off-by: Leno Hou <lenohou@gmail.com>
@zjumoon01
Copy link
Contributor Author

@allencloud Let's me make it happen

Thanks a lot. So great to see you could try this. @houstar

pouch cmd doesn't support. And runc doesn't neither. So we should make that in runc. I'm ready to make a PR to runc in this days.

cc @houstar

houstar added a commit to houstar/pouch that referenced this issue Dec 18, 2018
fixes AliyunContainerService#2509

Signed-off-by: Leno Hou <lenohou@gmail.com>
@houstar
Copy link
Contributor

houstar commented Dec 21, 2018

@zjumoon01 What's the progress on the runc side about the 'runc update` the cgroup blkio settings? 👍

@zjumoon01
Copy link
Contributor Author

@zjumoon01 What's the progress on the runc side about the 'runc update` the cgroup blkio settings? 👍

I will push it in this week

@zjumoon01
Copy link
Contributor Author

zjumoon01 commented Dec 25, 2018

@zjumoon01 What's the progress on the runc side about the 'runc update` the cgroup blkio settings? 👍

@houstar I have pushed the feature to project alibaba/runc.
alibaba-archive/runc#15

@houstar
Copy link
Contributor

houstar commented Dec 27, 2018

OK. If runc merged your PR, then I'll vendor this commit and test the integration

@zjumoon01
Copy link
Contributor Author

@houstar alibaba/runc has supported this feature as mentioned above

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature kind/feature-request This is a feature request from community for PouchContainer
Projects
None yet
5 participants