Skip to content
Snippets Groups Projects

[WIP] docker: support memory/memory-swap settings

Close: #83

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Hmmm, I'm sending a pull request to a repository owned by gitlab, but I'm the one to provide a runner for the CI tests? What if I provide a fake one that always return success here?

  • docker inspect shows Memory and MemorySwap is still assigned to 0 with:

    # cat config.toml 
    concurrent = 1
    
    [[runners]]
      name = "20fdb8d900d1"
      url = "https://ci.gitlab.com/"
      token = "a40e4222ff29f2661e8401363b9e02"
      limit = 1
      executor = "docker"
      [runners.docker]
        image = "golang:latest"
        privileged = false
        memory = 104857600
        memory_swap = -1
        allowed_images = ["golang:latest", "golang"]
  • @vicamo You have to bump dependency on go-dockerclient.

  • @vicamo

    Can we have the support for kB, MB...? What about limits for services?

  • @ayufan ,

    1. always doable, docker command line tool has that, too.
    2. I'm not sure if you mean limiting all memory resources used by all services as a whole, but AFAIK there is no way to do that due to the multi-container nature. Instead, like what you can do in a docker-compose.yml file, you can specify memory constraints for each container by another memory_limit directive.

    Speak of another directive, there are still many other resource management options, e.g. --cpu-shares, --blkio-weight, .... I'm not sure if we're interested in implementing them one by one. Instead, how about simply specifying a docker-compose.yml file path for docker runners? Make that new directive conflict with image, hostname, ... and all other directive which were meant for specifying attributes of the result container.

    However, we have to deal with that allowed_images thing. Even there is an article documenting how a .gitlab-ci.yml should look like, I still can't find a full reference to all legal directives for it. From that for gitlab-ci-multi-runner I see there is a directive for specifying the tests should be executed in a container of golang image. But I have no idea what's its relationship between the same directive under [runners.docker]. And it's even more strange that I have to specify it again in the allowed_images directive as shown in comment #3. Really, your team should really spend some more time on documentation if you really want people to contribute more. The Contributing section of this project mentions only the official location, issue tracker and merge requests location that I already know at the time I reach the project page.

    And I even have to assign a runner myself! I just have a quick check: among the 11 merged requests only one has CI success, which merged a typos branch to master of the same repository. I think this should have been the gating issue for v0.1 release for a CI runner git repository.

  • Maybe create yet another docker-compose runner type instead? And yet another docker-exec type for #56 (closed).

  • Added 2 commits:

    • 0d32716e - bump fsouza/go-dockerclient for memory constraints
    • d3c483c8 - docker: support memory/memory-swap settings
  • Added 2 commits:

    • fd4424f8 - bump fsouza/go-dockerclient for memory constraints
    • 6ca5c482 - docker: support memory/memory-swap settings
  • Added 1 commit:

    • 0bbf4c86 - docker: support memory/memory-swap settings
  • Added 1 commit:

    • 4ad8fc5b - docker: support memory/memory-swap settings
  • always doable, docker command line tool has that, too.

    Thanks!

    I'm not sure if you mean limiting all memory resources used by all services as a whole, but AFAIK there is no way to do that due to the multi-container nature. Instead, like what you can do in a docker-compose.yml file, you can specify memory constraints for each container by another memory_limit directive.

    Unfortunately I saw that too. It's pity that it's impossible to use the same cgroup for limiting CPU and Memory. I guess there's no good solution for that now.

    Speak of another directive, there are still many other resource management options, e.g. --cpu-shares, --blkio-weight, .... I'm not sure if we're interested in implementing them one by one. Instead, how about simply specifying a docker-compose.yml file path for docker runners? Make that new directive conflict with image, hostname, ... and all other directive which were meant for specifying attributes of the result container.

    I would rather not implement parts of docker-compose.yml in runner's code and make additional distinction between docker-compose vs docker. This introduces unneeded complexity. This is external tool that can be used as part of build process if you have such requirement.

    Maybe we could think about some generic (map[string]interface{}) option. We would use that to fill Docker API request by deserialising structures. Later we could deprecate current hand made options.

    However, we have to deal with that allowed_images thing. Even there is an article documenting how a .gitlab-ci.yml should look like, I still can't find a full reference to all legal directives for it. From that for gitlab-ci-multi-runner I see there is a directive for specifying the tests should be executed in a container of golang image. But I have no idea what's its relationship between the same directive under [runners.docker]. And it's even more strange that I have to specify it again in the allowed_images directive as shown in comment #3 (closed). Really, your team should really spend some more time on documentation if you really want people to contribute more. The Contributing section of this project mentions only the official location, issue tracker and merge requests location that I already know at the time I reach the project page.

    This is not yet released feature, but documented one: doc.gitlab.com/ci/builds_configuration/docker.html. We are currently working on making the CI and Runner documentation more user-friendly: https://gitlab.com/gitlab-org/gitlab-ci/merge_requests/205. If you have ideas how to make them better I will be happy to discuss it.

    And I even have to assign a runner myself! I just have a quick check: among the 11 merged requests only one has CI success, which merged a typos branch to master of the same repository. I think this should have been the gating issue for v0.1 release for a CI runner git repository.

    Yes, this is inconvenience. GitLab Inc. currently offers a bunch of public runners that can be used, which most likely in the near future will be enabled by default for all projects. The more information about the CI offering will be most announced with next release: when we will have more awesome Docker features.

  • Maybe we could think about some generic (map[string]interface{}) option. We would use that to fill Docker API request by deserialising structures. Later we could deprecate current hand made options.

    Sorry, I don't quite understand you. Something like gitlab-ci-multi-runner register --executor "docker" -- --memory <N> --memory-swap <M>? But go-dockerclient library just doesn't allow such thing?

    And I even have to assign a runner myself! I just have a quick check: among the 11 merged requests only one has CI success, which merged a typos branch to master of the same repository. I think this should have been the gating issue for v0.1 release for a CI runner git repository.

    Yes, this is inconvenience. GitLab Inc. currently offers a bunch of public runners that can be used, which most likely in the near future will be enabled by default for all projects. The more information about the CI offering will be most announced with next release: when we will have more awesome Docker features.

    I mean, gitlab dispatches build tasks for pull requests to a wrong repository. gitlab.org/gitlab-ci-multi-runner has obviously setup a runner so when you push a typos branch to this repository, there is a CI job automatically dispatched. So the point is, for every merge request to this same repository, that runner should be used. I may assign a runner to my own fork, and that should also be activated when I pushed some commits to it. But when I create a merge request to gitlab.org/gitlab-ci-multi-runner, it's gitlab.org/gitlab-ci-multi-runner's runner's job to run the tests. I hope this is clear enough. Should really be an issue open in gitlab itself though.

  • So the point is, for every merge request to this same repository, that runner should be used

    This is one ways to support Merge Requests from foreign repositories, but there's security problem with that approach: the runner may be used as specific one that can run in insecure mode (ie. shell, docker privileged). This is really not good idea to run untrusted code on parent's runner. This where shared runners can be used. We are working currently towards enabling shared runners by default for everyone.

    gitlab-ci-multi-runner register --executor "docker" -- --memory <N> --memory-swap <M>

    Almost, but that's idea.

    But go-dockerclient library just doesn't allow such thing?

    We can deserialise HostConfig structure.

  • Kamil Trzcińśki Milestone changed to v0.6.0

    Milestone changed to v0.6.0

  • Last communication on this PR was >20 days ago. This is a major drawback of this runner. Can I help with this PR?

  • @zimmskal As I've commented on one of your previous comment somewhere else, please don't hesitate to bring up your idea/work. I'm closing this to make it more clear to you.

  • username-removed-184241 Status changed to closed

    Status changed to closed

  • I am super-confused right now. What is wrong with this PR? @ayufan since you have the last word on this: Which direction should an implementation for this missing feature go? It is clear that there are more arguments that need to be implemented and there will be even more coming. So a generic solution where you maybe simply white-flag new arguments would fit best. I am also not familiar with the runner code, so a discussion about this is needed anyway.

Please register or sign in to reply
Loading