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

Docker limits realtime-update #5906

Closed
huapox opened this issue Oct 15, 2021 · 14 comments
Closed

Docker limits realtime-update #5906

huapox opened this issue Oct 15, 2021 · 14 comments

Comments

@huapox
Copy link
Contributor

huapox commented Oct 15, 2021

Update docker limits with cpu/mem/mem.reservation in realtime.

@huapox
Copy link
Contributor Author

huapox commented Oct 15, 2021

Current implemented like this:

2021-10-15_105236

Latter with a PR. Please audit this feature and code, thx~

@huapox
Copy link
Contributor Author

huapox commented Oct 15, 2021

PR #5909

@huib-portainer
Copy link
Contributor

Hi, thanks for the contribution!

I'm trying to understand a little bit more about your use case. Would be able to explain what you're trying to achieve with the change?

For instance with the current version of Portainer, if I wanted to update the resources I'd do:

  1. Go to the Containers list
  2. Open the container details
  3. Push the Duplicate/Edit button
  4. Go to the Runtime & Resources tab
  5. Change the values
  6. Push the Deploy the container button
  7. Push the Replace button

So I'm curious about the workflow that you have in mind.

@huapox
Copy link
Contributor Author

huapox commented Oct 18, 2021

So I'm curious about the workflow that you have in mind.

Thx, for the reply.

This provide a way that Just update the Limits but no need to recreate the Container:

  • The former btn "Deploy the container": re-create new container with changed params in the page.
  • Add new btn "Update Limits": just call the existed pt's API(exact with docker update --cpus=xx -m=xxx --memory-reservation=xxx)

The 2nd one is in-realtime, without the need of restart container, still that without the need of restart business-app's progress.

$ docker update --help
Options:
      --blkio-weight uint16        Block IO (relative weight), between 10 and 1000, or 0 to disable (default 0)
      --cpu-period int             Limit CPU CFS (Completely Fair Scheduler) period
      --cpu-quota int              Limit CPU CFS (Completely Fair Scheduler) quota
      --cpu-rt-period int          Limit the CPU real-time period in microseconds
      --cpu-rt-runtime int         Limit the CPU real-time runtime in microseconds
  -c, --cpu-shares int             CPU shares (relative weight)
      --cpus decimal               Number of CPUs
      --cpuset-cpus string         CPUs in which to allow execution (0-3, 0,1)
      --cpuset-mems string         MEMs in which to allow execution (0-3, 0,1)
      --kernel-memory bytes        Kernel memory limit
  -m, --memory bytes               Memory limit
      --memory-reservation bytes   Memory soft limit
      --memory-swap bytes          Swap limit equal to memory plus swap: '-1' to enable unlimited swap
      --restart string             Restart policy to apply when a container exits

@huib-portainer
Copy link
Contributor

For some reason the CI system hasn't actually created a build for it, which would be useful for anyone that wants to try it out. So we might have to add another commit.

At the moment I'm still considering the benefits of not having to recreate a container vs what side effects there could be from opening up updating the container. And does it work equally for Docker standalone and Swarm?
There's also the impact on keeping things simple for novice users, where we don't want to overwhelm them with a cluttered UI.

@SvenDowideit @d1mnewz any thoughts?

@d1mnewz
Copy link

d1mnewz commented Oct 19, 2021

@huib-portainer IMO from the novice user's POV nothing will change, they should not be caring so much about restart vs update on the fly.

I do not have an idea how updating works with Swarm, though.

Also, I'm into this feature because Kubernetes does exactly the same thing — updating deployment limits on the fly, without redeploying the pod.

@huib-portainer
Copy link
Contributor

If they don't care about the restart vs update, then why add the complexity to the UI?

@SvenDowideit
Copy link
Contributor

I guess my first thought is - with the UX that @huib-portainer has, why would that redeploy, and not detect that it can do it using docker update?

my second thought is OMG that's a lot of UX steps to do something that the user would expect is simple.

as a user, I would care about needlessly restarting - for example, it takes over a minute to restart the gitlab omnibus container - and if I'm hitting resource constraints, I'd like to goto portainer, see that I'm running out of X, and right there, update the limits on the already existing container - and then be able to assume that this has also been stored in portainer, so that if i ever need to restart it, the right limits are applied.

@huib-portainer
Copy link
Contributor

@yi-portainer the additional commits haven't actually solved the issue with the CI.
Is there a way to force the build?

@huib-portainer
Copy link
Contributor

I've managed to get the CI working again, so this can be tried by using the image portainerci/portainer:pr5909.

The current PR will show as:
image

Looking at the UI, the first section will only update the container, while the second section will redeploy the container. And the third section defines how a container gets redeployed as well.

To me at the moment that doesn't feel very coherent. But I don't see an easy way out for it either.
@ronmarlacamiento thoughts?

@huib-portainer
Copy link
Contributor

Actually, if we moved that top panel in the Runtime & Resources tab as follows:

image

Then it would be consistent with how the create container screen works.

@huapox
Copy link
Contributor Author

huapox commented Oct 28, 2021

Actually, if we moved that top panel in the Runtime & Resources tab as follows:

Okay, I'll change the latter.

@huib-portainer
Copy link
Contributor

Awesome, thanks!

@huapox
Copy link
Contributor Author

huapox commented Nov 9, 2021

thx all, Updated

updateLimit_uiPretty

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants