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

Always launch docker containers in the correct parent cgroup #1118

Merged

Conversation

@PaulFurtado
Copy link
Member

@PaulFurtado PaulFurtado commented Jul 1, 2016

Currently, the docker executor script hardcodes the parent cgroup of a docker container to mesos/$CGROUPS_GUID. This change makes it instead discover the cgroup using /proc/self/cgroup per the reccommendation of: https://www.freedesktop.org/wiki/Software/systemd/PaxControlGroups/

This change also removes the code for setting memory.use_hierarchy=1 because it is ineffective (you can see the warnings in the logs on our mesos slaves). In order to set that property, it must be set on the root cgroup, before any child cgroups are created. My internal changes to cgroups on our mesos slaves takes care of that and all child cgroups inherit the property.

@ssalinas @tpetr Going to try to test this myself tonight, might need your help with the Singularity bits tomorrow. I tested that function pretty thoroughly so I expect this to just work.

PaulFurtado added 2 commits Jul 1, 2016
Currently, the docker executor script hardcodes the parent cgroup of a docker
container to "mesos/$CGROUPS_GUID". This change makes it instead discover the
cgroup using "/proc/self/cgroup" per the reccommendation of:
  https://www.freedesktop.org/wiki/Software/systemd/PaxControlGroups/

This change also removes the code for setting memory.use_hierarchy=1 because
it is ineffective. In order to set that property, it must be set on the root
cgroup, before any child cgroups are created. My internal changes to cgroups
on our mesos slaves takes care of that and all child cgroups inherit the
property.
@ssalinas
Copy link
Member

@ssalinas ssalinas commented Jul 1, 2016

👍 I can test this further. Nitpick, but can we update the examples to be non-hubspot-specific? Other than that looks good, thanks for the cgroup expertise!

@ssalinas ssalinas added the hs_staging label Jul 1, 2016
@PaulFurtado
Copy link
Member Author

@PaulFurtado PaulFurtado commented Jul 1, 2016

@ssalinas 👍 Got rid of the reference to HubSpot in the example

@ssalinas ssalinas added the hs_qa label Jul 6, 2016
@ssalinas

This comment has been minimized.

Copy link
Member

@ssalinas ssalinas commented on 563ab9e Jul 11, 2016

🚢

@ssalinas

This comment has been minimized.

Copy link
Member

@ssalinas ssalinas commented on 5de7618 Jul 11, 2016

🚢

@ssalinas

This comment has been minimized.

Copy link
Member

@ssalinas ssalinas commented on 59b2426 Jul 11, 2016

🚢

@ssalinas ssalinas added the hs_stable label Jul 11, 2016
@ssalinas ssalinas merged commit c43a9c0 into HubSpot:master Jul 15, 2016
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@ssalinas ssalinas removed hs_qa labels Jul 15, 2016
@ssalinas ssalinas added this to the 0.9.0 milestone Jul 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.