Skip to content
This repository has been archived by the owner on Jan 25, 2022. It is now read-only.

handle grouped cgroup mounts (e.g. cpu,cpuacct) #62

Closed
wants to merge 1 commit into from
Closed

handle grouped cgroup mounts (e.g. cpu,cpuacct) #62

wants to merge 1 commit into from

Conversation

databus23
Copy link
Contributor

While trying to get garden running on CoreOS I noticed that the linux_backend does not account for grouped cgroup mounts which seem to become more prevalent these days:

cat /proc/1/cgroup
9:perf_event:/
8:cpuset:/
7:devices:/
6:memory:/
5:freezer:/
4:cpu,cpuacct:/
3:blkio:/
2:net_cls,net_prio:/
1:name=systemd:/

The attached changes make garden-linux start in environments with multi cgroup mounts. For systems without multi group mounts this doesn't change anything.

@cfdreddbot
Copy link
Collaborator

Hey databus23!

Thanks for submitting this pull request!

All pull request authors must have a Contributor License Agreement (CLA) on-file with us. Please sign the appropriate CLA (individual or corporate).

When sending signed CLA please provide your github username in case of individual CLA or the list of github usernames that can make pull requests on behalf of your organization.

If you are confident that you're covered under a Corporate CLA, please make sure you've publicized your membership in the appropriate Github Org, per these instructions.

@cf-gitbot
Copy link

We have created an issue in Pivotal Tracker to manage this. You can view the current status of your issue at: https://www.pivotaltracker.com/story/show/114693593.

Signed-off-by: Fabian Ruff <fabian@progra.de>
@julz
Copy link
Contributor

julz commented Mar 9, 2016

Hi @databus23 could you make your membership of the SAP organisation public in your github profile so cfdreddbot can tell that you're a member of an organisation that has signed the CLA please?

@databus23
Copy link
Contributor Author

@julz I signed an individual CLA last week. The contribution was performed off work. I'm currently not part of the SAP organization and don't even know who inside of SAP would be able to add me to the SAP organization.

@julz
Copy link
Contributor

julz commented Mar 11, 2016

@databus23 - great, thanks! Going to close and re-open the issue to see if that'll get dreddbot to re-judge :)

@julz julz closed this Mar 11, 2016
@julz julz reopened this Mar 11, 2016
@cfdreddbot
Copy link
Collaborator

Hey databus23!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you've already signed the CLA.

@julz
Copy link
Contributor

julz commented Mar 11, 2016

yay it worked! :) I've unblocked the story in our tracker to pull this in - thanks again

@albertoleal
Copy link
Contributor

Thank you, @databus23.

@albertoleal
Copy link
Contributor

Hey @databus23,

Just a quick note to let you know that we've recently reverted this PR as it caused an unexpected failure in our test suite.
It seems that there is a subtle difference in behaviour here when deploying garden-linux on a fresh VM vs when simply updating the garden-linux job. It also seems that it doesn't happen on bosh-lite, only 'real' deployments.

We see the deployment complete successfully but the garden-integration-tests start to report the following error:

Expected error:
    <*errors.errorString | 0xc20802bca0>: {
        s: "container: start: exit status 2",
    }
    container: start: exit status 2
not to have occurred

After some investigation, we realised that the issue only happens on a fresh VM because cgroups are only mounted if they are not already mounted.

Note that we use ubuntu for our tests and CI pipelines.

If you'd like to address the above test failure and resubmit the PR we'd be more than happy to accept. Let us know if you have any questions or thoughts.

Thanks very much,
Alberto and Ed.

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

Successfully merging this pull request may close these issues.

None yet

5 participants