Re: Review Request 48988: Reduced test linux rootfs size.

[haosdent huang]

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




src/tests/containerizer/rootfs.hpp (line 109)


+1 for this, it would reduce a lot of test cases running time.
@zhiwei may worry about running this on PowerPC, maybe we need find the 
corresponding path in PowerPC as well.


- haosdent huang


On June 21, 2016, 2:45 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48988/
> ---
> 
> (Updated June 21, 2016, 2:45 a.m.)
> 
> 
> Review request for mesos, Zhiwei Chen, haosdent huang, Artem Harutyunyan, Jie 
> Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4943
> https://issues.apache.org/jira/browse/MESOS-4943
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Reduced test linux rootfs size.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/rootfs.hpp 19ec7bf50e599f5420453e8cb7d12357770d3fd8 
> 
> Diff: https://reviews.apache.org/r/48988/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 48988: Reduced test linux rootfs size.

[Jie Yu]

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




src/tests/containerizer/rootfs.hpp (line 104)


This should be called 'files'?



src/tests/containerizer/rootfs.hpp (line 137)


I am not sure if this logic still applies. For instance, if `/lib64 -> 
/xxx/lib64`.

The realpath of `/lib64/librt.so.1` is `/xxx/lib64/librt.so.1`. However, 
`islink('/lib64/librt.so.1') == false`.

I think you should test if `realpath.get() == file`?


- Jie Yu


On June 21, 2016, 2:45 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48988/
> ---
> 
> (Updated June 21, 2016, 2:45 a.m.)
> 
> 
> Review request for mesos, Zhiwei Chen, haosdent huang, Artem Harutyunyan, Jie 
> Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4943
> https://issues.apache.org/jira/browse/MESOS-4943
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Reduced test linux rootfs size.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/rootfs.hpp 19ec7bf50e599f5420453e8cb7d12357770d3fd8 
> 
> Diff: https://reviews.apache.org/r/48988/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 48988: Reduced test linux rootfs size.

[Jie Yu]

> On June 21, 2016, 3:09 a.m., haosdent huang wrote:
> > src/tests/containerizer/rootfs.hpp, line 109
> > 
> >
> > +1 for this, it would reduce a lot of test cases running time.
> > @zhiwei may worry about running this on PowerPC, maybe we need find the 
> > corresponding path in PowerPC as well.

@zhiwei, we'll ship this for now. Let us konw if it breaks root tests on 
powerpc or not.


- Jie


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


On June 21, 2016, 2:45 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48988/
> ---
> 
> (Updated June 21, 2016, 2:45 a.m.)
> 
> 
> Review request for mesos, Zhiwei Chen, haosdent huang, Artem Harutyunyan, Jie 
> Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4943
> https://issues.apache.org/jira/browse/MESOS-4943
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Reduced test linux rootfs size.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/rootfs.hpp 19ec7bf50e599f5420453e8cb7d12357770d3fd8 
> 
> Diff: https://reviews.apache.org/r/48988/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 48988: Reduced test linux rootfs size.

[Gilbert Song]

> On June 22, 2016, 10:23 a.m., Jie Yu wrote:
> > src/tests/containerizer/rootfs.hpp, line 142
> > 
> >
> > I am not sure if this logic still applies. For instance, if `/lib64 -> 
> > /xxx/lib64`.
> > 
> > The realpath of `/lib64/librt.so.1` is `/xxx/lib64/librt.so.1`. 
> > However, `islink('/lib64/librt.so.1') == false`.
> > 
> > I think you should test if `realpath.get() == file`?

The logic should not be changed here. It should be identical to the old logic:

1. get realpath and copy the file.
2. check islink() and copy the link.

For the files listed, some of them are symlink, depending on diff OS. So for 
example, the realpath of `/lib/x86_64-linux-gnu` is 
`/lib/x86_64-linux-gnu/ld-2.19.so`, and in this case 
`islink('/lib/x86_64-linux-gnu')` is not false.

I dont understand why you want to do the `realpath.get() == file` check here. 
If it is a file, it was already copied sbove. We only want to handle all the 
symlinks here altogether. 

PS: In CentOS and Fedora, `/usr/bin/sh` is a symlink as well, pointing to 
`/usr/bin/bash`. They are pretty common cases that some executable/library 
files are symlink.


- Gilbert


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


On June 20, 2016, 7:45 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48988/
> ---
> 
> (Updated June 20, 2016, 7:45 p.m.)
> 
> 
> Review request for mesos, Zhiwei Chen, haosdent huang, Artem Harutyunyan, Jie 
> Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4943
> https://issues.apache.org/jira/browse/MESOS-4943
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Reduced test linux rootfs size.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/rootfs.hpp 19ec7bf50e599f5420453e8cb7d12357770d3fd8 
> 
> Diff: https://reviews.apache.org/r/48988/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 48988: Reduced test linux rootfs size.

[Gilbert Song]

> On June 20, 2016, 8:09 p.m., haosdent huang wrote:
> > src/tests/containerizer/rootfs.hpp, line 109
> > 
> >
> > +1 for this, it would reduce a lot of test cases running time.
> > @zhiwei may worry about running this on PowerPC, maybe we need find the 
> > corresponding path in PowerPC as well.
> 
> Jie Yu wrote:
> @zhiwei, we'll ship this for now. Let us konw if it breaks root tests on 
> powerpc or not.

Thanks haosdent, I added Zhiwei for this reason:)


- Gilbert


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


On June 20, 2016, 7:45 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48988/
> ---
> 
> (Updated June 20, 2016, 7:45 p.m.)
> 
> 
> Review request for mesos, Zhiwei Chen, haosdent huang, Artem Harutyunyan, Jie 
> Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4943
> https://issues.apache.org/jira/browse/MESOS-4943
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Reduced test linux rootfs size.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/rootfs.hpp 19ec7bf50e599f5420453e8cb7d12357770d3fd8 
> 
> Diff: https://reviews.apache.org/r/48988/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 48988: Reduced test linux rootfs size.

[Jie Yu]

> On June 22, 2016, 5:23 p.m., Jie Yu wrote:
> > src/tests/containerizer/rootfs.hpp, line 142
> > 
> >
> > I am not sure if this logic still applies. For instance, if `/lib64 -> 
> > /xxx/lib64`.
> > 
> > The realpath of `/lib64/librt.so.1` is `/xxx/lib64/librt.so.1`. 
> > However, `islink('/lib64/librt.so.1') == false`.
> > 
> > I think you should test if `realpath.get() == file`?
> 
> Gilbert Song wrote:
> The logic should not be changed here. It should be identical to the old 
> logic:
> 
> 1. get realpath and copy the file.
> 2. check islink() and copy the link.
> 
> For the files listed, some of them are symlink, depending on diff OS. So 
> for example, the realpath of `/lib/x86_64-linux-gnu` is 
> `/lib/x86_64-linux-gnu/ld-2.19.so`, and in this case 
> `islink('/lib/x86_64-linux-gnu')` is not false.
> 
> I dont understand why you want to do the `realpath.get() == file` check 
> here. If it is a file, it was already copied sbove. We only want to handle 
> all the symlinks here altogether. 
> 
> PS: In CentOS and Fedora, `/usr/bin/sh` is a symlink as well, pointing to 
> `/usr/bin/bash`. They are pretty common cases that some executable/library 
> files are symlink.

what if the file itself is not a link, but the directory that contains it is a 
symlink?


- Jie


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


On June 21, 2016, 2:45 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48988/
> ---
> 
> (Updated June 21, 2016, 2:45 a.m.)
> 
> 
> Review request for mesos, Zhiwei Chen, haosdent huang, Artem Harutyunyan, Jie 
> Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4943
> https://issues.apache.org/jira/browse/MESOS-4943
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Reduced test linux rootfs size.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/rootfs.hpp 19ec7bf50e599f5420453e8cb7d12357770d3fd8 
> 
> Diff: https://reviews.apache.org/r/48988/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 48988: Reduced test linux rootfs size.

[Gilbert Song]

> On June 22, 2016, 10:23 a.m., Jie Yu wrote:
> > src/tests/containerizer/rootfs.hpp, line 142
> > 
> >
> > I am not sure if this logic still applies. For instance, if `/lib64 -> 
> > /xxx/lib64`.
> > 
> > The realpath of `/lib64/librt.so.1` is `/xxx/lib64/librt.so.1`. 
> > However, `islink('/lib64/librt.so.1') == false`.
> > 
> > I think you should test if `realpath.get() == file`?
> 
> Gilbert Song wrote:
> The logic should not be changed here. It should be identical to the old 
> logic:
> 
> 1. get realpath and copy the file.
> 2. check islink() and copy the link.
> 
> For the files listed, some of them are symlink, depending on diff OS. So 
> for example, the realpath of `/lib/x86_64-linux-gnu` is 
> `/lib/x86_64-linux-gnu/ld-2.19.so`, and in this case 
> `islink('/lib/x86_64-linux-gnu')` is not false.
> 
> I dont understand why you want to do the `realpath.get() == file` check 
> here. If it is a file, it was already copied sbove. We only want to handle 
> all the symlinks here altogether. 
> 
> PS: In CentOS and Fedora, `/usr/bin/sh` is a symlink as well, pointing to 
> `/usr/bin/bash`. They are pretty common cases that some executable/library 
> files are symlink.
> 
> Jie Yu wrote:
> what if the file itself is not a link, but the directory that contains it 
> is a symlink?

Gotcha.


- Gilbert


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


On June 20, 2016, 7:45 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48988/
> ---
> 
> (Updated June 20, 2016, 7:45 p.m.)
> 
> 
> Review request for mesos, Zhiwei Chen, haosdent huang, Artem Harutyunyan, Jie 
> Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4943
> https://issues.apache.org/jira/browse/MESOS-4943
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Reduced test linux rootfs size.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/rootfs.hpp 19ec7bf50e599f5420453e8cb7d12357770d3fd8 
> 
> Diff: https://reviews.apache.org/r/48988/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 48988: Reduced test linux rootfs size.

[Gilbert Song]

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

(Updated June 22, 2016, 3:53 p.m.)


Review request for mesos, Zhiwei Chen, haosdent huang, Artem Harutyunyan, Jie 
Yu, and Timothy Chen.


Bugs: MESOS-4943
https://issues.apache.org/jira/browse/MESOS-4943


Repository: mesos


Description
---

Reduced test linux rootfs size.


Diffs (updated)
-

  src/tests/containerizer/rootfs.hpp 19ec7bf50e599f5420453e8cb7d12357770d3fd8 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 48988: Reduced test linux rootfs size.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On June 22, 2016, 10:53 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48988/
> ---
> 
> (Updated June 22, 2016, 10:53 p.m.)
> 
> 
> Review request for mesos, Zhiwei Chen, haosdent huang, Artem Harutyunyan, Jie 
> Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4943
> https://issues.apache.org/jira/browse/MESOS-4943
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Reduced test linux rootfs size.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/rootfs.hpp 19ec7bf50e599f5420453e8cb7d12357770d3fd8 
> 
> Diff: https://reviews.apache.org/r/48988/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>