Re: Review Request 37684: Added symlink test for /bin, lib, and /lib64.

2015-08-21 Thread Greg Mann 


> On Aug. 21, 2015, 6:14 p.m., Jie Yu wrote:
> > src/tests/containerizer/rootfs.hpp, lines 111-126
> > 
> >
> > Can you combine the this loop with the loop below (like the following). 
> > Also, could you use 'realpath' here to resolve the symlink.
> > 
> > ```
> > foreach (const std::string& directory, directories) {
> >   // Some linux distros are moving all binaries and libraries
> >   // to /usr, in which case /bin, /lib, and /lib64 will be
> >   // symlinks to their equivalent directories in /usr. So we
> >   // call the realpath here first. 
> >   Result realpath = os::realpath(directory);
> >   if (!realpath.isSome()) {
> > return Error("Failed to get realpath for '" + directory + "': " +
> >  (realpath.isError() ? realpath.error() : "No such 
> > directory");
> >   }
> >   
> >   Try result = rootfs->add(realpath.get());
> >   ...
> > }
> > ```

Thanks Jie, that's much better! :-) Comments addressed.


- Greg


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37684/#review96068
---


On Aug. 21, 2015, 7:15 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37684/
> ---
> 
> (Updated Aug. 21, 2015, 7:15 p.m.)
> 
> 
> Review request for mesos, Marco Massenzio and Vinod Kone.
> 
> 
> Bugs: MESOS-3296 and MESOS-3297
> https://issues.apache.org/jira/browse/MESOS-3296
> https://issues.apache.org/jira/browse/MESOS-3297
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added symlink test for /bin, /lib, and /lib64.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/rootfs.hpp 961003fd6768ee0f7effd2804d1c453f8d45e058 
> 
> Diff: https://reviews.apache.org/r/37684/diff/
> 
> 
> Testing
> ---
> 
> On CentOS 7.1 and other distros with symlinked /bin, /lib, & /lib64:
> 
> "sudo make check"
> 
> 
> Some tests will fail due to other root test issues being tracked at 
> MESOS-3292, MESOS-3293, MESOS-3294, MESOS-3295
> 
> This patch solves bugs in MesosContainerizerLaunchTest.ROOT_ChangeRootfs and 
> the LinuxFilesystemIsolatorTest.* tests, so those should all pass.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 37684: Added symlink test for /bin, lib, and /lib64.

2015-08-21 Thread Greg Mann 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37684/
---

(Updated Aug. 21, 2015, 7:15 p.m.)


Review request for mesos, Marco Massenzio and Vinod Kone.


Bugs: MESOS-3296 and MESOS-3297
https://issues.apache.org/jira/browse/MESOS-3296
https://issues.apache.org/jira/browse/MESOS-3297


Repository: mesos


Description
---

Added symlink test for /bin, /lib, and /lib64.


Diffs (updated)
-

  src/tests/containerizer/rootfs.hpp 961003fd6768ee0f7effd2804d1c453f8d45e058 

Diff: https://reviews.apache.org/r/37684/diff/


Testing
---

On CentOS 7.1 and other distros with symlinked /bin, /lib, & /lib64:

"sudo make check"


Some tests will fail due to other root test issues being tracked at MESOS-3292, 
MESOS-3293, MESOS-3294, MESOS-3295

This patch solves bugs in MesosContainerizerLaunchTest.ROOT_ChangeRootfs and 
the LinuxFilesystemIsolatorTest.* tests, so those should all pass.


Thanks,

Greg Mann

Re: Review Request 37684: Added symlink test for /bin, lib, and /lib64.

2015-08-21 Thread Jie Yu 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37684/#review96068
---

Ship it!


Thanks


src/tests/containerizer/rootfs.hpp (lines 111 - 126)


Can you combine the this loop with the loop below (like the following). 
Also, could you use 'realpath' here to resolve the symlink.

```
foreach (const std::string& directory, directories) {
  // Some linux distros are moving all binaries and libraries
  // to /usr, in which case /bin, /lib, and /lib64 will be
  // symlinks to their equivalent directories in /usr. So we
  // call the realpath here first. 
  Result realpath = os::realpath(directory);
  if (!realpath.isSome()) {
return Error("Failed to get realpath for '" + directory + "': " +
 (realpath.isError() ? realpath.error() : "No such 
directory");
  }
  
  Try result = rootfs->add(realpath.get());
  ...
}
```


- Jie Yu


On Aug. 21, 2015, 4:51 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37684/
> ---
> 
> (Updated Aug. 21, 2015, 4:51 p.m.)
> 
> 
> Review request for mesos, Marco Massenzio and Vinod Kone.
> 
> 
> Bugs: MESOS-3296 and MESOS-3297
> https://issues.apache.org/jira/browse/MESOS-3296
> https://issues.apache.org/jira/browse/MESOS-3297
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added symlink test for /bin, /lib, and /lib64.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/rootfs.hpp 961003fd6768ee0f7effd2804d1c453f8d45e058 
> 
> Diff: https://reviews.apache.org/r/37684/diff/
> 
> 
> Testing
> ---
> 
> On CentOS 7.1 and other distros with symlinked /bin, /lib, & /lib64:
> 
> "sudo make check"
> 
> 
> Some tests will fail due to other root test issues being tracked at 
> MESOS-3292, MESOS-3293, MESOS-3294, MESOS-3295
> 
> This patch solves bugs in MesosContainerizerLaunchTest.ROOT_ChangeRootfs and 
> the LinuxFilesystemIsolatorTest.* tests, so those should all pass.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 37684: Added symlink test for /bin, lib, and /lib64.

2015-08-21 Thread Marco Massenzio 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37684/#review96054
---

Ship it!


Ship It!

- Marco Massenzio


On Aug. 21, 2015, 4:51 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37684/
> ---
> 
> (Updated Aug. 21, 2015, 4:51 p.m.)
> 
> 
> Review request for mesos, Marco Massenzio and Vinod Kone.
> 
> 
> Bugs: MESOS-3296 and MESOS-3297
> https://issues.apache.org/jira/browse/MESOS-3296
> https://issues.apache.org/jira/browse/MESOS-3297
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added symlink test for /bin, /lib, and /lib64.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/rootfs.hpp 961003fd6768ee0f7effd2804d1c453f8d45e058 
> 
> Diff: https://reviews.apache.org/r/37684/diff/
> 
> 
> Testing
> ---
> 
> On CentOS 7.1 and other distros with symlinked /bin, /lib, & /lib64:
> 
> "sudo make check"
> 
> 
> Some tests will fail due to other root test issues being tracked at 
> MESOS-3292, MESOS-3293, MESOS-3294, MESOS-3295
> 
> This patch solves bugs in MesosContainerizerLaunchTest.ROOT_ChangeRootfs and 
> the LinuxFilesystemIsolatorTest.* tests, so those should all pass.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 37684: Added symlink test for /bin, lib, and /lib64.

2015-08-21 Thread Greg Mann 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37684/
---

(Updated Aug. 21, 2015, 4:51 p.m.)


Review request for mesos, Marco Massenzio and Vinod Kone.


Bugs: MESOS-3296 and MESOS-3297
https://issues.apache.org/jira/browse/MESOS-3296
https://issues.apache.org/jira/browse/MESOS-3297


Repository: mesos


Description
---

Added symlink test for /bin, /lib, and /lib64.


Diffs
-

  src/tests/containerizer/rootfs.hpp 961003fd6768ee0f7effd2804d1c453f8d45e058 

Diff: https://reviews.apache.org/r/37684/diff/


Testing (updated)
---

On CentOS 7.1 and other distros with symlinked /bin, /lib, & /lib64:

"sudo make check"


Some tests will fail due to other root test issues being tracked at MESOS-3292, 
MESOS-3293, MESOS-3294, MESOS-3295

This patch solves bugs in MesosContainerizerLaunchTest.ROOT_ChangeRootfs and 
the LinuxFilesystemIsolatorTest.* tests, so those should all pass.


Thanks,

Greg Mann

Review Request 37684: Added symlink test for /bin, lib, and /lib64.

2015-08-21 Thread Greg Mann 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37684/
---

Review request for mesos, Marco Massenzio and Vinod Kone.


Bugs: MESOS-3296 and MESOS-3297
https://issues.apache.org/jira/browse/MESOS-3296
https://issues.apache.org/jira/browse/MESOS-3297


Repository: mesos


Description
---

Added symlink test for /bin, /lib, and /lib64.


Diffs
-

  src/tests/containerizer/rootfs.hpp 961003fd6768ee0f7effd2804d1c453f8d45e058 

Diff: https://reviews.apache.org/r/37684/diff/


Testing
---

On CentOS 7.1 and other distros with symlinked /bin, /lib, & /lib64:

sudo make check

Some tests will fail due to other root test issues being tracked at MESOS-3292, 
MESOS-3293, MESOS-3294, MESOS-3295

This patch solves bugs in MesosContainerizerLaunchTest.ROOT_ChangeRootfs and 
the LinuxFilesystemIsolatorTest.* tests, so those should all pass.


Thanks,

Greg Mann