You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by James Peach <jp...@apache.org> on 2018/05/11 18:32:44 UTC
Review Request 67097: Added `linux/devices` isolator whitelist
support.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67097/
-----------------------------------------------------------
Review request for mesos, Gilbert Song, Jason Lai, Jie Yu, and Zhitao Li.
Bugs: MESOS-8792
https://issues.apache.org/jira/browse/MESOS-8792
Repository: mesos
Description
-------
Added `linux/devices` isolator support for populating the container
devices. This introduces a general mechanism for populating devices
into a specific container but currently only implements devices for all
containers based on the devices specified by the `--allowed_devices`
agent flag.
Diffs
-----
src/slave/containerizer/mesos/isolators/linux/devices.hpp PRE-CREATION
src/slave/containerizer/mesos/isolators/linux/devices.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/67097/diff/1/
Testing
-------
manual
Thanks,
James Peach
Re: Review Request 67097: Added `linux/devices` isolator whitelist
support.
Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67097/
-----------------------------------------------------------
(Updated May 23, 2018, 3:44 p.m.)
Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
Bugs: MESOS-8792
https://issues.apache.org/jira/browse/MESOS-8792
Repository: mesos
Description
-------
Added `linux/devices` isolator support for populating the container
devices. This introduces a general mechanism for populating devices
into a specific container but currently only implements devices for all
containers based on the devices specified by the `--allowed_devices`
agent flag.
Diffs (updated)
-----
src/slave/containerizer/mesos/isolators/linux/devices.hpp PRE-CREATION
src/slave/containerizer/mesos/isolators/linux/devices.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/67097/diff/5/
Changes: https://reviews.apache.org/r/67097/diff/4-5/
Testing
-------
make check (Fedora 27)
Thanks,
James Peach
Re: Review Request 67097: Added `linux/devices` isolator whitelist
support.
Posted by James Peach <jp...@apache.org>.
> On May 21, 2018, 7:24 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/linux/devices.cpp
> > Lines 170-175 (patched)
> > <https://reviews.apache.org/r/67097/diff/4/?file=2024122#file2024122line171>
> >
> > Any reason we only do that for the directory, not the actual device file?
>
> James Peach wrote:
> The actual device file is supposed to be accessed by the task user, so it has to be owned by that user. This chown is ensuring that we don't also grant access to other host users as a side-effect of the container.
>
> Jie Yu wrote:
> What do you mean by "side-effect"?
>
> James Peach wrote:
> We only want to grant access to the containerized task, not to other processes that might be running on the host. For example, when we grant read access, we set `S_IRGRP | S_IROTH` which makes the device readable by everyone on the host. Restricting the permissions on the container devices directory prevents this, and setting the owner ensures we will still have access even if we are later running as the task user.
>
> Jie Yu wrote:
> What I don't follow is that why we need to set the ownership and permission on the devicesDir. What if we just change the actual device files to have proper ownership (task's user) and permission (700), will that be sufficient?
> What if we just change the actual device files to have proper ownership (task's user) and permission (700), will that be sufficient?
That will work, but I think this approach is better semantically. It presents devices as owned by root, which is what you would get on a VM or a host, and it gives the expected access throughout the container, which is consistent with the cgroups devices policy and avoids any potential issues with multiple credentials inside the container.
- James
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67097/#review203515
-----------------------------------------------------------
On May 15, 2018, 5:53 p.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67097/
> -----------------------------------------------------------
>
> (Updated May 15, 2018, 5:53 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
>
>
> Bugs: MESOS-8792
> https://issues.apache.org/jira/browse/MESOS-8792
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added `linux/devices` isolator support for populating the container
> devices. This introduces a general mechanism for populating devices
> into a specific container but currently only implements devices for all
> containers based on the devices specified by the `--allowed_devices`
> agent flag.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/linux/devices.hpp PRE-CREATION
> src/slave/containerizer/mesos/isolators/linux/devices.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/67097/diff/4/
>
>
> Testing
> -------
>
> make check (Fedora 27)
>
>
> Thanks,
>
> James Peach
>
>
Re: Review Request 67097: Added `linux/devices` isolator whitelist
support.
Posted by Jie Yu <yu...@gmail.com>.
> On May 21, 2018, 7:24 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/linux/devices.cpp
> > Lines 170-175 (patched)
> > <https://reviews.apache.org/r/67097/diff/4/?file=2024122#file2024122line171>
> >
> > Any reason we only do that for the directory, not the actual device file?
>
> James Peach wrote:
> The actual device file is supposed to be accessed by the task user, so it has to be owned by that user. This chown is ensuring that we don't also grant access to other host users as a side-effect of the container.
>
> Jie Yu wrote:
> What do you mean by "side-effect"?
>
> James Peach wrote:
> We only want to grant access to the containerized task, not to other processes that might be running on the host. For example, when we grant read access, we set `S_IRGRP | S_IROTH` which makes the device readable by everyone on the host. Restricting the permissions on the container devices directory prevents this, and setting the owner ensures we will still have access even if we are later running as the task user.
>
> Jie Yu wrote:
> What I don't follow is that why we need to set the ownership and permission on the devicesDir. What if we just change the actual device files to have proper ownership (task's user) and permission (700), will that be sufficient?
>
> James Peach wrote:
> > What if we just change the actual device files to have proper ownership (task's user) and permission (700), will that be sufficient?
>
> That will work, but I think this approach is better semantically. It presents devices as owned by root, which is what you would get on a VM or a host, and it gives the expected access throughout the container, which is consistent with the cgroups devices policy and avoids any potential issues with multiple credentials inside the container.
I feel that it's a bit un-intuitive to me that the access control of the device depends on the parent directory's permission (not the device itself). For instance, a host process (user `foo`) having `CAP_DAC_READ_SEARCH` will have write access to the device if the device is allowed to be written by the container user `bar`.
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67097/#review203515
-----------------------------------------------------------
On May 15, 2018, 5:53 p.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67097/
> -----------------------------------------------------------
>
> (Updated May 15, 2018, 5:53 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
>
>
> Bugs: MESOS-8792
> https://issues.apache.org/jira/browse/MESOS-8792
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added `linux/devices` isolator support for populating the container
> devices. This introduces a general mechanism for populating devices
> into a specific container but currently only implements devices for all
> containers based on the devices specified by the `--allowed_devices`
> agent flag.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/linux/devices.hpp PRE-CREATION
> src/slave/containerizer/mesos/isolators/linux/devices.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/67097/diff/4/
>
>
> Testing
> -------
>
> make check (Fedora 27)
>
>
> Thanks,
>
> James Peach
>
>
Re: Review Request 67097: Added `linux/devices` isolator whitelist
support.
Posted by James Peach <jp...@apache.org>.
> On May 21, 2018, 7:24 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/linux/devices.cpp
> > Lines 170-175 (patched)
> > <https://reviews.apache.org/r/67097/diff/4/?file=2024122#file2024122line171>
> >
> > Any reason we only do that for the directory, not the actual device file?
>
> James Peach wrote:
> The actual device file is supposed to be accessed by the task user, so it has to be owned by that user. This chown is ensuring that we don't also grant access to other host users as a side-effect of the container.
>
> Jie Yu wrote:
> What do you mean by "side-effect"?
>
> James Peach wrote:
> We only want to grant access to the containerized task, not to other processes that might be running on the host. For example, when we grant read access, we set `S_IRGRP | S_IROTH` which makes the device readable by everyone on the host. Restricting the permissions on the container devices directory prevents this, and setting the owner ensures we will still have access even if we are later running as the task user.
>
> Jie Yu wrote:
> What I don't follow is that why we need to set the ownership and permission on the devicesDir. What if we just change the actual device files to have proper ownership (task's user) and permission (700), will that be sufficient?
>
> James Peach wrote:
> > What if we just change the actual device files to have proper ownership (task's user) and permission (700), will that be sufficient?
>
> That will work, but I think this approach is better semantically. It presents devices as owned by root, which is what you would get on a VM or a host, and it gives the expected access throughout the container, which is consistent with the cgroups devices policy and avoids any potential issues with multiple credentials inside the container.
>
> Jie Yu wrote:
> I feel that it's a bit un-intuitive to me that the access control of the device depends on the parent directory's permission (not the device itself). For instance, a host process (user `foo`) having `CAP_DAC_READ_SEARCH` will have write access to the device if the device is allowed to be written by the container user `bar`.
As per our conversation on slack, I added a comment documenting that the access control for devices has container granularity.
- James
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67097/#review203515
-----------------------------------------------------------
On May 15, 2018, 5:53 p.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67097/
> -----------------------------------------------------------
>
> (Updated May 15, 2018, 5:53 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
>
>
> Bugs: MESOS-8792
> https://issues.apache.org/jira/browse/MESOS-8792
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added `linux/devices` isolator support for populating the container
> devices. This introduces a general mechanism for populating devices
> into a specific container but currently only implements devices for all
> containers based on the devices specified by the `--allowed_devices`
> agent flag.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/linux/devices.hpp PRE-CREATION
> src/slave/containerizer/mesos/isolators/linux/devices.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/67097/diff/4/
>
>
> Testing
> -------
>
> make check (Fedora 27)
>
>
> Thanks,
>
> James Peach
>
>
Re: Review Request 67097: Added `linux/devices` isolator whitelist
support.
Posted by James Peach <jp...@apache.org>.
> On May 21, 2018, 7:24 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/linux/devices.cpp
> > Lines 170-175 (patched)
> > <https://reviews.apache.org/r/67097/diff/4/?file=2024122#file2024122line171>
> >
> > Any reason we only do that for the directory, not the actual device file?
The actual device file is supposed to be accessed by the task user, so it has to be owned by that user. This chown is ensuring that we don't also grant access to other host users as a side-effect of the container.
- James
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67097/#review203515
-----------------------------------------------------------
On May 15, 2018, 5:53 p.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67097/
> -----------------------------------------------------------
>
> (Updated May 15, 2018, 5:53 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
>
>
> Bugs: MESOS-8792
> https://issues.apache.org/jira/browse/MESOS-8792
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added `linux/devices` isolator support for populating the container
> devices. This introduces a general mechanism for populating devices
> into a specific container but currently only implements devices for all
> containers based on the devices specified by the `--allowed_devices`
> agent flag.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/linux/devices.hpp PRE-CREATION
> src/slave/containerizer/mesos/isolators/linux/devices.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/67097/diff/4/
>
>
> Testing
> -------
>
> make check (Fedora 27)
>
>
> Thanks,
>
> James Peach
>
>
Re: Review Request 67097: Added `linux/devices` isolator whitelist
support.
Posted by James Peach <jp...@apache.org>.
> On May 21, 2018, 7:24 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/linux/devices.cpp
> > Lines 170-175 (patched)
> > <https://reviews.apache.org/r/67097/diff/4/?file=2024122#file2024122line171>
> >
> > Any reason we only do that for the directory, not the actual device file?
>
> James Peach wrote:
> The actual device file is supposed to be accessed by the task user, so it has to be owned by that user. This chown is ensuring that we don't also grant access to other host users as a side-effect of the container.
>
> Jie Yu wrote:
> What do you mean by "side-effect"?
We only want to grant access to the containerized task, not to other processes that might be running on the host. For example, when we grant read access, we set `S_IRGRP | S_IROTH` which makes the device readable by everyone on the host. Restricting the permissions on the container devices directory prevents this, and setting the owner ensures we will still have access even if we are later running as the task user.
- James
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67097/#review203515
-----------------------------------------------------------
On May 15, 2018, 5:53 p.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67097/
> -----------------------------------------------------------
>
> (Updated May 15, 2018, 5:53 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
>
>
> Bugs: MESOS-8792
> https://issues.apache.org/jira/browse/MESOS-8792
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added `linux/devices` isolator support for populating the container
> devices. This introduces a general mechanism for populating devices
> into a specific container but currently only implements devices for all
> containers based on the devices specified by the `--allowed_devices`
> agent flag.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/linux/devices.hpp PRE-CREATION
> src/slave/containerizer/mesos/isolators/linux/devices.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/67097/diff/4/
>
>
> Testing
> -------
>
> make check (Fedora 27)
>
>
> Thanks,
>
> James Peach
>
>
Re: Review Request 67097: Added `linux/devices` isolator whitelist
support.
Posted by Jie Yu <yu...@gmail.com>.
> On May 21, 2018, 7:24 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/linux/devices.cpp
> > Lines 170-175 (patched)
> > <https://reviews.apache.org/r/67097/diff/4/?file=2024122#file2024122line171>
> >
> > Any reason we only do that for the directory, not the actual device file?
>
> James Peach wrote:
> The actual device file is supposed to be accessed by the task user, so it has to be owned by that user. This chown is ensuring that we don't also grant access to other host users as a side-effect of the container.
What do you mean by "side-effect"?
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67097/#review203515
-----------------------------------------------------------
On May 15, 2018, 5:53 p.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67097/
> -----------------------------------------------------------
>
> (Updated May 15, 2018, 5:53 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
>
>
> Bugs: MESOS-8792
> https://issues.apache.org/jira/browse/MESOS-8792
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added `linux/devices` isolator support for populating the container
> devices. This introduces a general mechanism for populating devices
> into a specific container but currently only implements devices for all
> containers based on the devices specified by the `--allowed_devices`
> agent flag.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/linux/devices.hpp PRE-CREATION
> src/slave/containerizer/mesos/isolators/linux/devices.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/67097/diff/4/
>
>
> Testing
> -------
>
> make check (Fedora 27)
>
>
> Thanks,
>
> James Peach
>
>
Re: Review Request 67097: Added `linux/devices` isolator whitelist
support.
Posted by Jie Yu <yu...@gmail.com>.
> On May 21, 2018, 7:24 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/linux/devices.cpp
> > Lines 170-175 (patched)
> > <https://reviews.apache.org/r/67097/diff/4/?file=2024122#file2024122line171>
> >
> > Any reason we only do that for the directory, not the actual device file?
>
> James Peach wrote:
> The actual device file is supposed to be accessed by the task user, so it has to be owned by that user. This chown is ensuring that we don't also grant access to other host users as a side-effect of the container.
>
> Jie Yu wrote:
> What do you mean by "side-effect"?
>
> James Peach wrote:
> We only want to grant access to the containerized task, not to other processes that might be running on the host. For example, when we grant read access, we set `S_IRGRP | S_IROTH` which makes the device readable by everyone on the host. Restricting the permissions on the container devices directory prevents this, and setting the owner ensures we will still have access even if we are later running as the task user.
What I don't follow is that why we need to set the ownership and permission on the devicesDir. What if we just change the actual device files to have proper ownership (task's user) and permission (700), will that be sufficient?
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67097/#review203515
-----------------------------------------------------------
On May 15, 2018, 5:53 p.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67097/
> -----------------------------------------------------------
>
> (Updated May 15, 2018, 5:53 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
>
>
> Bugs: MESOS-8792
> https://issues.apache.org/jira/browse/MESOS-8792
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added `linux/devices` isolator support for populating the container
> devices. This introduces a general mechanism for populating devices
> into a specific container but currently only implements devices for all
> containers based on the devices specified by the `--allowed_devices`
> agent flag.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/linux/devices.hpp PRE-CREATION
> src/slave/containerizer/mesos/isolators/linux/devices.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/67097/diff/4/
>
>
> Testing
> -------
>
> make check (Fedora 27)
>
>
> Thanks,
>
> James Peach
>
>
Re: Review Request 67097: Added `linux/devices` isolator whitelist
support.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67097/#review203515
-----------------------------------------------------------
Fix it, then Ship it!
src/slave/containerizer/mesos/isolators/linux/devices.cpp
Lines 170-175 (patched)
<https://reviews.apache.org/r/67097/#comment285805>
Any reason we only do that for the directory, not the actual device file?
- Jie Yu
On May 15, 2018, 5:53 p.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67097/
> -----------------------------------------------------------
>
> (Updated May 15, 2018, 5:53 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
>
>
> Bugs: MESOS-8792
> https://issues.apache.org/jira/browse/MESOS-8792
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added `linux/devices` isolator support for populating the container
> devices. This introduces a general mechanism for populating devices
> into a specific container but currently only implements devices for all
> containers based on the devices specified by the `--allowed_devices`
> agent flag.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/linux/devices.hpp PRE-CREATION
> src/slave/containerizer/mesos/isolators/linux/devices.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/67097/diff/4/
>
>
> Testing
> -------
>
> make check (Fedora 27)
>
>
> Thanks,
>
> James Peach
>
>
Re: Review Request 67097: Added `linux/devices` isolator whitelist
support.
Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67097/
-----------------------------------------------------------
(Updated May 15, 2018, 5:53 p.m.)
Review request for mesos, Gilbert Song, Jason Lai, Jie Yu, and Zhitao Li.
Bugs: MESOS-8792
https://issues.apache.org/jira/browse/MESOS-8792
Repository: mesos
Description
-------
Added `linux/devices` isolator support for populating the container
devices. This introduces a general mechanism for populating devices
into a specific container but currently only implements devices for all
containers based on the devices specified by the `--allowed_devices`
agent flag.
Diffs (updated)
-----
src/slave/containerizer/mesos/isolators/linux/devices.hpp PRE-CREATION
src/slave/containerizer/mesos/isolators/linux/devices.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/67097/diff/2/
Changes: https://reviews.apache.org/r/67097/diff/1-2/
Testing
-------
manual
Thanks,
James Peach
Re: Review Request 67097: Added `linux/devices` isolator whitelist
support.
Posted by James Peach <jp...@apache.org>.
> On May 11, 2018, 11:34 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/linux/devices.cpp
> > Lines 73 (patched)
> > <https://reviews.apache.org/r/67097/diff/1/?file=2020541#file2020541line74>
> >
> > I'd suggest we just skip this whitelist entry, instead of fail the agent launch.
> >
> > If the operator does not specify the device path, it indicate that he does not want the device path to show up in the container. He just want the device cgroup to allow the device to be whitelisted. Thoughts?
>
> James Peach wrote:
> I thought about that, and I'm happy to change to that behaviour. However, even in the devices cgroup isolator, the path is actually required .. it just silently doesn't work if you omit the path. Maybe we should require the path in both cases?
We discussed this on chat and agreed that returning an error for a missing path is the right behavior for both the linux/devices and cgroups/devices isolators.
- James
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67097/#review202975
-----------------------------------------------------------
On May 15, 2018, 5:53 p.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67097/
> -----------------------------------------------------------
>
> (Updated May 15, 2018, 5:53 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
>
>
> Bugs: MESOS-8792
> https://issues.apache.org/jira/browse/MESOS-8792
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added `linux/devices` isolator support for populating the container
> devices. This introduces a general mechanism for populating devices
> into a specific container but currently only implements devices for all
> containers based on the devices specified by the `--allowed_devices`
> agent flag.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/linux/devices.hpp PRE-CREATION
> src/slave/containerizer/mesos/isolators/linux/devices.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/67097/diff/4/
>
>
> Testing
> -------
>
> make check (Fedora 27)
>
>
> Thanks,
>
> James Peach
>
>
Re: Review Request 67097: Added `linux/devices` isolator whitelist
support.
Posted by James Peach <jp...@apache.org>.
> On May 11, 2018, 11:34 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/linux/devices.cpp
> > Lines 73 (patched)
> > <https://reviews.apache.org/r/67097/diff/1/?file=2020541#file2020541line74>
> >
> > I'd suggest we just skip this whitelist entry, instead of fail the agent launch.
> >
> > If the operator does not specify the device path, it indicate that he does not want the device path to show up in the container. He just want the device cgroup to allow the device to be whitelisted. Thoughts?
I thought about that, and I'm happy to change to that behaviour. However, even in the devices cgroup isolator, the path is actually required .. it just silently doesn't work if you omit the path. Maybe we should require the path in both cases?
- James
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67097/#review202975
-----------------------------------------------------------
On May 11, 2018, 6:32 p.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67097/
> -----------------------------------------------------------
>
> (Updated May 11, 2018, 6:32 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jason Lai, Jie Yu, and Zhitao Li.
>
>
> Bugs: MESOS-8792
> https://issues.apache.org/jira/browse/MESOS-8792
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added `linux/devices` isolator support for populating the container
> devices. This introduces a general mechanism for populating devices
> into a specific container but currently only implements devices for all
> containers based on the devices specified by the `--allowed_devices`
> agent flag.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/linux/devices.hpp PRE-CREATION
> src/slave/containerizer/mesos/isolators/linux/devices.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/67097/diff/1/
>
>
> Testing
> -------
>
> manual
>
>
> Thanks,
>
> James Peach
>
>
Re: Review Request 67097: Added `linux/devices` isolator whitelist
support.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67097/#review202975
-----------------------------------------------------------
src/slave/containerizer/mesos/isolators/linux/devices.cpp
Lines 71 (patched)
<https://reviews.apache.org/r/67097/#comment285036>
use `deviceAccess` please :)
src/slave/containerizer/mesos/isolators/linux/devices.cpp
Lines 73 (patched)
<https://reviews.apache.org/r/67097/#comment285037>
I'd suggest we just skip this whitelist entry, instead of fail the agent launch.
If the operator does not specify the device path, it indicate that he does not want the device path to show up in the container. He just want the device cgroup to allow the device to be whitelisted. Thoughts?
- Jie Yu
On May 11, 2018, 6:32 p.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67097/
> -----------------------------------------------------------
>
> (Updated May 11, 2018, 6:32 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jason Lai, Jie Yu, and Zhitao Li.
>
>
> Bugs: MESOS-8792
> https://issues.apache.org/jira/browse/MESOS-8792
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added `linux/devices` isolator support for populating the container
> devices. This introduces a general mechanism for populating devices
> into a specific container but currently only implements devices for all
> containers based on the devices specified by the `--allowed_devices`
> agent flag.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/linux/devices.hpp PRE-CREATION
> src/slave/containerizer/mesos/isolators/linux/devices.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/67097/diff/1/
>
>
> Testing
> -------
>
> manual
>
>
> Thanks,
>
> James Peach
>
>