You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Qian Zhang <zh...@gmail.com> on 2020/08/04 08:20:11 UTC

Review Request 72733: Implemented the `prepare` method of `volume/csi` isolator.

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

Review request for mesos, Andrei Budnik and Greg Mann.


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


Repository: mesos


Description
-------

Implemented the `prepare` method of `volume/csi` isolator.


Diffs
-----

  src/CMakeLists.txt 4e15e3d99aa2cce2403fe07e762fef2fb4a27dea 
  src/Makefile.am 447db323875e4cad46000977f4a61600baff8f89 
  src/slave/containerizer/mesos/isolators/volume/csi/isolator.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/volume/csi/state.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/volume/csi/state.proto PRE-CREATION 


Diff: https://reviews.apache.org/r/72733/diff/1/


Testing
-------


Thanks,

Qian Zhang


Re: Review Request 72733: Implemented the `prepare` method of `volume/csi` isolator.

Posted by Qian Zhang <zh...@gmail.com>.

> On Aug. 6, 2020, 9:27 a.m., Greg Mann wrote:
> > src/slave/containerizer/mesos/isolators/volume/csi/isolator.hpp
> > Lines 85 (patched)
> > <https://reviews.apache.org/r/72733/diff/1/?file=2236984#file2236984line85>
> >
> >     Does the value of this map really need to be an `Owned`, rather than just a raw `Info`?
> 
> Qian Zhang wrote:
>     I think that's a convention in our isolator implementation, most of the isolators use `Owned` rather than a raw `Info`, like:
>     https://github.com/apache/mesos/blob/1.10.0/src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp#L169
>     https://github.com/apache/mesos/blob/1.10.0/src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp#L117
>     https://github.com/apache/mesos/blob/1.10.0/src/slave/containerizer/mesos/isolators/filesystem/linux.hpp#L99
>     https://github.com/apache/mesos/blob/1.10.0/src/slave/containerizer/mesos/isolators/network/cni/cni.hpp#L223
> 
> Greg Mann wrote:
>     Any idea why? It seems unnecessary to me?

I am not quite sure, maybe it is a best practice to avoid using raw pointer in our code? I see we have some patches which replaced raw pointers with `Owned` in containerizer:
https://reviews.apache.org/r/51388/ 
https://reviews.apache.org/r/32233


> On Aug. 6, 2020, 9:27 a.m., Greg Mann wrote:
> > src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp
> > Lines 146-151 (patched)
> > <https://reviews.apache.org/r/72733/diff/1/?file=2236985#file2236985line146>
> >
> >     Should we check `_volume.mode()` against the access mode contained within the `VolumeCapability`? Maybe we should have some separate validation code for that?
> 
> Qian Zhang wrote:
>     Yeah, I thought this before, but I am not quite sure if we should do that. Actually in CSI spec, there is no any explanation about the interaction between the `NodePublishVolumeRequest.readonly` and the access mode in the `VolumeCapability`, like what if `NodePublishVolumeRequest.readonly == false` but the access mode in the `VolumeCapability` is `SINGLE_NODE_READER_ONLY`? Should we return a failure in this case?
> 
> Greg Mann wrote:
>     I'm not so worried about the `readonly` field, I don't think that we need to validate it against the volume capabilities because mounting a volume as read-only isn't "incompatible" with any of the capabilities (for example, I don't think there's anything wrong with mounting a volume readonly if the volume has only the SINGLE_NODE_WRITER capability, since mounting as readonly does not violate RW permissions).
>     
>     However, I do think we should perform validation for our internal `Volume::Mode` field. If a task attempts to include a CSI volume which has only the SINGLE_NODE_READER_ONLY capability and the volume mode is set as RW, that does not seem valid to me. WDYT?

> I don't think there's anything wrong with mounting a volume readonly if the volume has only the SINGLE_NODE_WRITER capability, since mounting as readonly does not violate RW permissions

But what about the reverse? I mean `NodePublishVolumeRequest.readonly == false` and the volume has only the `SINGLE_NODE_READER_ONLY` capability. Do you think it is wrong?

> However, I do think we should perform validation for our internal Volume::Mode field. If a task attempts to include a CSI volume which has only the SINGLE_NODE_READER_ONLY capability and the volume mode is set as RW, that does not seem valid to me. WDYT?

Yes, I agree. I think I should add more validation here like: if the CSI volume has only the `SINGLE_NODE_READER_ONLY` or `MULTI_NODE_READER_ONLY` capability and our internal `Volume::Mode` field it set as `RW`, then we should return a failure. WDYT?


> On Aug. 6, 2020, 9:27 a.m., Greg Mann wrote:
> > src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp
> > Lines 179-184 (patched)
> > <https://reviews.apache.org/r/72733/diff/1/?file=2236985#file2236985line179>
> >
> >     Can you help me understand why we do not require an absolute container path to already exist in this case, while we require it in other cases (both here and in the Linux filesystem isolator)?
> 
> Qian Zhang wrote:
>     The reason that we do not require an absolute container path to already exist is that the container can be launched with any images, we cannot require all images have the specified absolute container path to already exist, and since the container is launched with an image (i.e. it has its own rootfs), we can just create the absolute container path in its own rootfs which will not affect the agent host rootfs. However if the container is launched without an image (i.e. it has no its own rootfs and just uses the agent host rootfs), we require the absolute container path to already exist in the agent host rootfs and do not want to create the path via `os::mkdir()` because we do not want to change the agent host rootfs.
>     
>     Could you please point me the code block of the Linux filesystem that you mentioned above?
> 
> Greg Mann wrote:
>     > The reason that we do not require an absolute container path to already exist is that the container can be launched with any images, we cannot require all images have the specified absolute container path to already exist, and since the container is launched with an image (i.e. it has its own rootfs)
>     
>     Given the way our comments are written, I think it would be reasonable if a task is launched with an image which specifies a volume with an absolute container path, then they must specify an image in which that absolute path already exists. I think it probably makes sense to have the same policy for CSI volumes that we have for other types of volumes?
>     
>     In the Linux filesystem isolator, we skip volumes with absolute container paths here: https://github.com/apache/mesos/blob/c78dc333fc893a43d40dc33299a61987198a6ea9/src/slave/containerizer/mesos/isolators/filesystem/linux.cpp#L931-L939
>     
>     And I was unable to find another place in that isolator where we create directories in rootfs's for volumes with absolute container paths.
>     
>     However, I see that in other volume isolators we do create the path within the rootfs. Since that is the case, I might suggest that we leave your code as-is and update our comments for the `container_path` field so that they are correct?

Yeah, I think our comments for the `container_path` field is outdated which I guess was writted before we support launching container with an image.
```
message Volume {
  ..

  // Path pointing to a directory or file in the container. If the
  // path is a relative path, it is relative to the container work
  // directory. If the path is an absolute path, that path must
  // already exist.
  required string container_path = 1;
```

If the container is launched with an image, then we should not require an absolute container path to already exist. That's also what Docker does, e.g. when you run `docker run --rm -it -v /opt:/qzhang/data alpine sh`, Docker will automaically create the `/qzhang/data` inside the container which I think is the correct behavior.

I will update that comments for the `container_path` field, thanks for catching this!


> On Aug. 6, 2020, 9:27 a.m., Greg Mann wrote:
> > src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp
> > Lines 287-300 (patched)
> > <https://reviews.apache.org/r/72733/diff/1/?file=2236985#file2236985line287>
> >
> >     Can we eliminate this if you use `collect()` above instead of `await()`?
> 
> Qian Zhang wrote:
>     I think we still need to use `await()` so if any CSI publish volume call fails we can get the actual error message.
> 
> Greg Mann wrote:
>     `collect()` will surface the message of the first failure to occur: https://github.com/apache/mesos/blob/c78dc333fc893a43d40dc33299a61987198a6ea9/3rdparty/libprocess/include/process/collect.hpp#L172-L174
>     
>     If that's sufficient we can use `collect()`, but if you want to aggregate the messages from multiple failures then you're right, `await()` is required.

Yeah, I think we need to aggregate the messages from multiple failures, that is also how we work with CNI plugins.


> On Aug. 6, 2020, 9:27 a.m., Greg Mann wrote:
> > src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp
> > Lines 316 (patched)
> > <https://reviews.apache.org/r/72733/diff/1/?file=2236985#file2236985line316>
> >
> >     Why do we do a recursive bind mount here, but not for volumes in the Linux filesystem isolator? https://github.com/apache/mesos/blob/c78dc333fc893a43d40dc33299a61987198a6ea9/src/slave/containerizer/mesos/isolators/filesystem/linux.cpp#L1067-L1068
> 
> Qian Zhang wrote:
>     The reason that we do a recursive bind mount here is, if there is any submounts under the `source`, we want all those submounts also bind mounted at the `target` so that the container can access them. We have this logic in all the volume isolators, like:
>     https://github.com/apache/mesos/blob/1.10.0/src/slave/containerizer/mesos/isolators/volume/host_path.cpp#L351
>     https://github.com/apache/mesos/blob/1.10.0/src/slave/containerizer/mesos/isolators/volume/image.cpp#L255
>     
>     The code that you pointed out in the Linux filesystem isolator is to mount the PV, since PV is originally created by Mesos and we are sure there is no submounts under it, so we do not need `MS_REC` in this case.
> 
> Greg Mann wrote:
>     Why do we know that a persistent volume will not have any mounts under it, but we don't know this for CSI volumes? The PV is originally created by Mesos, but applications can do whatever they want in the volume after it's created.

A CSI volume can be a pre-provisioned volume so it can already have some submounts under it before users try to use it via Mesos. But you are right, applicaiton can also create submounts under a PV. Maybe we should fix it for PV in a separate ticket? WDYT?


- Qian


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


On Aug. 6, 2020, 5:23 p.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72733/
> -----------------------------------------------------------
> 
> (Updated Aug. 6, 2020, 5:23 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10153
>     https://issues.apache.org/jira/browse/MESOS-10153
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Implemented the `prepare` method of `volume/csi` isolator.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt 4e15e3d99aa2cce2403fe07e762fef2fb4a27dea 
>   src/Makefile.am 447db323875e4cad46000977f4a61600baff8f89 
>   src/common/validation.cpp 14a8c7b9d70c303b527e5e43012999471ced2d78 
>   src/slave/containerizer/mesos/isolators/volume/csi/isolator.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/volume/csi/state.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/volume/csi/state.proto PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/72733/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


Re: Review Request 72733: Implemented the `prepare` method of `volume/csi` isolator.

Posted by Qian Zhang <zh...@gmail.com>.

> On Aug. 6, 2020, 9:27 a.m., Greg Mann wrote:
> > src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp
> > Lines 115-117 (patched)
> > <https://reviews.apache.org/r/72733/diff/1/?file=2236985#file2236985line115>
> >
> >     This seems pretty simple, could we do it now?
> 
> Qian Zhang wrote:
>     Actually we have this issue in the other two isolators:
>     https://github.com/apache/mesos/blob/1.10.0/src/slave/containerizer/mesos/isolators/volume/image.cpp#L118:L123
>     https://github.com/apache/mesos/blob/1.10.0/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp#L364:L367
>     
>     I think I can create a separate ticket for this issue and refactor all of them later, HDYT?
> 
> Greg Mann wrote:
>     Personally I think it's OK to write this isolator the "correct" way now, we can always update the others later. I don't feel a need to keep our isolator implementations super consistent across isolators.

Sure, let me do it for this isolator now.


- Qian


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


On Aug. 6, 2020, 5:23 p.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72733/
> -----------------------------------------------------------
> 
> (Updated Aug. 6, 2020, 5:23 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10153
>     https://issues.apache.org/jira/browse/MESOS-10153
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Implemented the `prepare` method of `volume/csi` isolator.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt 4e15e3d99aa2cce2403fe07e762fef2fb4a27dea 
>   src/Makefile.am 447db323875e4cad46000977f4a61600baff8f89 
>   src/common/validation.cpp 14a8c7b9d70c303b527e5e43012999471ced2d78 
>   src/slave/containerizer/mesos/isolators/volume/csi/isolator.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/volume/csi/state.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/volume/csi/state.proto PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/72733/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


Re: Review Request 72733: Implemented the `prepare` method of `volume/csi` isolator.

Posted by Greg Mann <gr...@mesosphere.io>.

> On Aug. 6, 2020, 1:27 a.m., Greg Mann wrote:
> > src/slave/containerizer/mesos/isolators/volume/csi/isolator.hpp
> > Lines 85 (patched)
> > <https://reviews.apache.org/r/72733/diff/1/?file=2236984#file2236984line85>
> >
> >     Does the value of this map really need to be an `Owned`, rather than just a raw `Info`?
> 
> Qian Zhang wrote:
>     I think that's a convention in our isolator implementation, most of the isolators use `Owned` rather than a raw `Info`, like:
>     https://github.com/apache/mesos/blob/1.10.0/src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp#L169
>     https://github.com/apache/mesos/blob/1.10.0/src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp#L117
>     https://github.com/apache/mesos/blob/1.10.0/src/slave/containerizer/mesos/isolators/filesystem/linux.hpp#L99
>     https://github.com/apache/mesos/blob/1.10.0/src/slave/containerizer/mesos/isolators/network/cni/cni.hpp#L223

Any idea why? It seems unnecessary to me?


> On Aug. 6, 2020, 1:27 a.m., Greg Mann wrote:
> > src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp
> > Lines 115-117 (patched)
> > <https://reviews.apache.org/r/72733/diff/1/?file=2236985#file2236985line115>
> >
> >     This seems pretty simple, could we do it now?
> 
> Qian Zhang wrote:
>     Actually we have this issue in the other two isolators:
>     https://github.com/apache/mesos/blob/1.10.0/src/slave/containerizer/mesos/isolators/volume/image.cpp#L118:L123
>     https://github.com/apache/mesos/blob/1.10.0/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp#L364:L367
>     
>     I think I can create a separate ticket for this issue and refactor all of them later, HDYT?

Personally I think it's OK to write this isolator the "correct" way now, we can always update the others later. I don't feel a need to keep our isolator implementations super consistent across isolators.


> On Aug. 6, 2020, 1:27 a.m., Greg Mann wrote:
> > src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp
> > Lines 146-151 (patched)
> > <https://reviews.apache.org/r/72733/diff/1/?file=2236985#file2236985line146>
> >
> >     Should we check `_volume.mode()` against the access mode contained within the `VolumeCapability`? Maybe we should have some separate validation code for that?
> 
> Qian Zhang wrote:
>     Yeah, I thought this before, but I am not quite sure if we should do that. Actually in CSI spec, there is no any explanation about the interaction between the `NodePublishVolumeRequest.readonly` and the access mode in the `VolumeCapability`, like what if `NodePublishVolumeRequest.readonly == false` but the access mode in the `VolumeCapability` is `SINGLE_NODE_READER_ONLY`? Should we return a failure in this case?

I'm not so worried about the `readonly` field, I don't think that we need to validate it against the volume capabilities because mounting a volume as read-only isn't "incompatible" with any of the capabilities (for example, I don't think there's anything wrong with mounting a volume readonly if the volume has only the SINGLE_NODE_WRITER capability, since mounting as readonly does not violate RW permissions).

However, I do think we should perform validation for our internal `Volume::Mode` field. If a task attempts to include a CSI volume which has only the SINGLE_NODE_READER_ONLY capability and the volume mode is set as RW, that does not seem valid to me. WDYT?


> On Aug. 6, 2020, 1:27 a.m., Greg Mann wrote:
> > src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp
> > Lines 179-184 (patched)
> > <https://reviews.apache.org/r/72733/diff/1/?file=2236985#file2236985line179>
> >
> >     Can you help me understand why we do not require an absolute container path to already exist in this case, while we require it in other cases (both here and in the Linux filesystem isolator)?
> 
> Qian Zhang wrote:
>     The reason that we do not require an absolute container path to already exist is that the container can be launched with any images, we cannot require all images have the specified absolute container path to already exist, and since the container is launched with an image (i.e. it has its own rootfs), we can just create the absolute container path in its own rootfs which will not affect the agent host rootfs. However if the container is launched without an image (i.e. it has no its own rootfs and just uses the agent host rootfs), we require the absolute container path to already exist in the agent host rootfs and do not want to create the path via `os::mkdir()` because we do not want to change the agent host rootfs.
>     
>     Could you please point me the code block of the Linux filesystem that you mentioned above?

> The reason that we do not require an absolute container path to already exist is that the container can be launched with any images, we cannot require all images have the specified absolute container path to already exist, and since the container is launched with an image (i.e. it has its own rootfs)

Given the way our comments are written, I think it would be reasonable if a task is launched with an image which specifies a volume with an absolute container path, then they must specify an image in which that absolute path already exists. I think it probably makes sense to have the same policy for CSI volumes that we have for other types of volumes?

In the Linux filesystem isolator, we skip volumes with absolute container paths here: https://github.com/apache/mesos/blob/c78dc333fc893a43d40dc33299a61987198a6ea9/src/slave/containerizer/mesos/isolators/filesystem/linux.cpp#L931-L939

And I was unable to find another place in that isolator where we create directories in rootfs's for volumes with absolute container paths.

However, I see that in other volume isolators we do create the path within the rootfs. Since that is the case, I might suggest that we leave your code as-is and update our comments for the `container_path` field so that they are correct?


> On Aug. 6, 2020, 1:27 a.m., Greg Mann wrote:
> > src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp
> > Lines 287-300 (patched)
> > <https://reviews.apache.org/r/72733/diff/1/?file=2236985#file2236985line287>
> >
> >     Can we eliminate this if you use `collect()` above instead of `await()`?
> 
> Qian Zhang wrote:
>     I think we still need to use `await()` so if any CSI publish volume call fails we can get the actual error message.

`collect()` will surface the message of the first failure to occur: https://github.com/apache/mesos/blob/c78dc333fc893a43d40dc33299a61987198a6ea9/3rdparty/libprocess/include/process/collect.hpp#L172-L174

If that's sufficient we can use `collect()`, but if you want to aggregate the messages from multiple failures then you're right, `await()` is required.


> On Aug. 6, 2020, 1:27 a.m., Greg Mann wrote:
> > src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp
> > Lines 316 (patched)
> > <https://reviews.apache.org/r/72733/diff/1/?file=2236985#file2236985line316>
> >
> >     Why do we do a recursive bind mount here, but not for volumes in the Linux filesystem isolator? https://github.com/apache/mesos/blob/c78dc333fc893a43d40dc33299a61987198a6ea9/src/slave/containerizer/mesos/isolators/filesystem/linux.cpp#L1067-L1068
> 
> Qian Zhang wrote:
>     The reason that we do a recursive bind mount here is, if there is any submounts under the `source`, we want all those submounts also bind mounted at the `target` so that the container can access them. We have this logic in all the volume isolators, like:
>     https://github.com/apache/mesos/blob/1.10.0/src/slave/containerizer/mesos/isolators/volume/host_path.cpp#L351
>     https://github.com/apache/mesos/blob/1.10.0/src/slave/containerizer/mesos/isolators/volume/image.cpp#L255
>     
>     The code that you pointed out in the Linux filesystem isolator is to mount the PV, since PV is originally created by Mesos and we are sure there is no submounts under it, so we do not need `MS_REC` in this case.

Why do we know that a persistent volume will not have any mounts under it, but we don't know this for CSI volumes? The PV is originally created by Mesos, but applications can do whatever they want in the volume after it's created.


- Greg


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


On Aug. 6, 2020, 9:23 a.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72733/
> -----------------------------------------------------------
> 
> (Updated Aug. 6, 2020, 9:23 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10153
>     https://issues.apache.org/jira/browse/MESOS-10153
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Implemented the `prepare` method of `volume/csi` isolator.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt 4e15e3d99aa2cce2403fe07e762fef2fb4a27dea 
>   src/Makefile.am 447db323875e4cad46000977f4a61600baff8f89 
>   src/common/validation.cpp 14a8c7b9d70c303b527e5e43012999471ced2d78 
>   src/slave/containerizer/mesos/isolators/volume/csi/isolator.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/volume/csi/state.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/volume/csi/state.proto PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/72733/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


Re: Review Request 72733: Implemented the `prepare` method of `volume/csi` isolator.

Posted by Qian Zhang <zh...@gmail.com>.

> On Aug. 6, 2020, 9:27 a.m., Greg Mann wrote:
> > src/slave/containerizer/mesos/isolators/volume/csi/isolator.hpp
> > Lines 85 (patched)
> > <https://reviews.apache.org/r/72733/diff/1/?file=2236984#file2236984line85>
> >
> >     Does the value of this map really need to be an `Owned`, rather than just a raw `Info`?

I think that's a convention in our isolator implementation, most of the isolators use `Owned` rather than a raw `Info`, like:
https://github.com/apache/mesos/blob/1.10.0/src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp#L169
https://github.com/apache/mesos/blob/1.10.0/src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp#L117
https://github.com/apache/mesos/blob/1.10.0/src/slave/containerizer/mesos/isolators/filesystem/linux.hpp#L99
https://github.com/apache/mesos/blob/1.10.0/src/slave/containerizer/mesos/isolators/network/cni/cni.hpp#L223


> On Aug. 6, 2020, 9:27 a.m., Greg Mann wrote:
> > src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp
> > Lines 115-117 (patched)
> > <https://reviews.apache.org/r/72733/diff/1/?file=2236985#file2236985line115>
> >
> >     This seems pretty simple, could we do it now?

Actually we have this issue in the other two isolators:
https://github.com/apache/mesos/blob/1.10.0/src/slave/containerizer/mesos/isolators/volume/image.cpp#L118:L123
https://github.com/apache/mesos/blob/1.10.0/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp#L364:L367

I think I can create a separate ticket for this issue and refactor all of them later, HDYT?


> On Aug. 6, 2020, 9:27 a.m., Greg Mann wrote:
> > src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp
> > Lines 132-134 (patched)
> > <https://reviews.apache.org/r/72733/diff/1/?file=2236985#file2236985line132>
> >
> >     Do we have any validation code which could catch this earlier? Ditto for the `static_provisioning` check below.

Good catch! I think I can do the validation earlier here: https://github.com/apache/mesos/blob/1.10.0/src/common/validation.cpp#L213


> On Aug. 6, 2020, 9:27 a.m., Greg Mann wrote:
> > src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp
> > Lines 146-151 (patched)
> > <https://reviews.apache.org/r/72733/diff/1/?file=2236985#file2236985line146>
> >
> >     Should we check `_volume.mode()` against the access mode contained within the `VolumeCapability`? Maybe we should have some separate validation code for that?

Yeah, I thought this before, but I am not quite sure if we should do that. Actually in CSI spec, there is no any explanation about the interaction between the `NodePublishVolumeRequest.readonly` and the access mode in the `VolumeCapability`, like what if `NodePublishVolumeRequest.readonly == false` but the access mode in the `VolumeCapability` is `SINGLE_NODE_READER_ONLY`? Should we return a failure in this case?


> On Aug. 6, 2020, 9:27 a.m., Greg Mann wrote:
> > src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp
> > Lines 179-184 (patched)
> > <https://reviews.apache.org/r/72733/diff/1/?file=2236985#file2236985line179>
> >
> >     Can you help me understand why we do not require an absolute container path to already exist in this case, while we require it in other cases (both here and in the Linux filesystem isolator)?

The reason that we do not require an absolute container path to already exist is that the container can be launched with any images, we cannot require all images have the specified absolute container path to already exist, and since the container is launched with an image (i.e. it has its own rootfs), we can just create the absolute container path in its own rootfs which will not affect the agent host rootfs. However if the container is launched without an image (i.e. it has no its own rootfs and just uses the agent host rootfs), we require the absolute container path to already exist in the agent host rootfs and do not want to create the path via `os::mkdir()` because we do not want to change the agent host rootfs.

Could you please point me the code block of the Linux filesystem that you mentioned above?


> On Aug. 6, 2020, 9:27 a.m., Greg Mann wrote:
> > src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp
> > Lines 287-300 (patched)
> > <https://reviews.apache.org/r/72733/diff/1/?file=2236985#file2236985line287>
> >
> >     Can we eliminate this if you use `collect()` above instead of `await()`?

I think we still need to use `await()` so if any CSI publish volume call fails we can get the actual error message.


> On Aug. 6, 2020, 9:27 a.m., Greg Mann wrote:
> > src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp
> > Lines 316 (patched)
> > <https://reviews.apache.org/r/72733/diff/1/?file=2236985#file2236985line316>
> >
> >     Why do we do a recursive bind mount here, but not for volumes in the Linux filesystem isolator? https://github.com/apache/mesos/blob/c78dc333fc893a43d40dc33299a61987198a6ea9/src/slave/containerizer/mesos/isolators/filesystem/linux.cpp#L1067-L1068

The reason that we do a recursive bind mount here is, if there is any submounts under the `source`, we want all those submounts also bind mounted at the `target` so that the container can access them. We have this logic in all the volume isolators, like:
https://github.com/apache/mesos/blob/1.10.0/src/slave/containerizer/mesos/isolators/volume/host_path.cpp#L351
https://github.com/apache/mesos/blob/1.10.0/src/slave/containerizer/mesos/isolators/volume/image.cpp#L255

The code that you pointed out in the Linux filesystem isolator is to mount the PV, since PV is originally created by Mesos and we are sure there is no submounts under it, so we do not need `MS_REC` in this case.


- Qian


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


On Aug. 4, 2020, 4:20 p.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72733/
> -----------------------------------------------------------
> 
> (Updated Aug. 4, 2020, 4:20 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10153
>     https://issues.apache.org/jira/browse/MESOS-10153
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Implemented the `prepare` method of `volume/csi` isolator.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt 4e15e3d99aa2cce2403fe07e762fef2fb4a27dea 
>   src/Makefile.am 447db323875e4cad46000977f4a61600baff8f89 
>   src/slave/containerizer/mesos/isolators/volume/csi/isolator.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/volume/csi/state.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/volume/csi/state.proto PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/72733/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


Re: Review Request 72733: Implemented the `prepare` method of `volume/csi` isolator.

Posted by Greg Mann <gr...@mesosphere.io>.

> On Aug. 6, 2020, 1:27 a.m., Greg Mann wrote:
> > src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp
> > Lines 115-117 (patched)
> > <https://reviews.apache.org/r/72733/diff/1/?file=2236985#file2236985line115>
> >
> >     This seems pretty simple, could we do it now?
> 
> Qian Zhang wrote:
>     Actually we have this issue in the other two isolators:
>     https://github.com/apache/mesos/blob/1.10.0/src/slave/containerizer/mesos/isolators/volume/image.cpp#L118:L123
>     https://github.com/apache/mesos/blob/1.10.0/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp#L364:L367
>     
>     I think I can create a separate ticket for this issue and refactor all of them later, HDYT?
> 
> Greg Mann wrote:
>     Personally I think it's OK to write this isolator the "correct" way now, we can always update the others later. I don't feel a need to keep our isolator implementations super consistent across isolators.
> 
> Qian Zhang wrote:
>     Sure, let me do it for this isolator now.

Thanks!!


> On Aug. 6, 2020, 1:27 a.m., Greg Mann wrote:
> > src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp
> > Lines 146-151 (patched)
> > <https://reviews.apache.org/r/72733/diff/1/?file=2236985#file2236985line146>
> >
> >     Should we check `_volume.mode()` against the access mode contained within the `VolumeCapability`? Maybe we should have some separate validation code for that?
> 
> Qian Zhang wrote:
>     Yeah, I thought this before, but I am not quite sure if we should do that. Actually in CSI spec, there is no any explanation about the interaction between the `NodePublishVolumeRequest.readonly` and the access mode in the `VolumeCapability`, like what if `NodePublishVolumeRequest.readonly == false` but the access mode in the `VolumeCapability` is `SINGLE_NODE_READER_ONLY`? Should we return a failure in this case?
> 
> Greg Mann wrote:
>     I'm not so worried about the `readonly` field, I don't think that we need to validate it against the volume capabilities because mounting a volume as read-only isn't "incompatible" with any of the capabilities (for example, I don't think there's anything wrong with mounting a volume readonly if the volume has only the SINGLE_NODE_WRITER capability, since mounting as readonly does not violate RW permissions).
>     
>     However, I do think we should perform validation for our internal `Volume::Mode` field. If a task attempts to include a CSI volume which has only the SINGLE_NODE_READER_ONLY capability and the volume mode is set as RW, that does not seem valid to me. WDYT?
> 
> Qian Zhang wrote:
>     > I don't think there's anything wrong with mounting a volume readonly if the volume has only the SINGLE_NODE_WRITER capability, since mounting as readonly does not violate RW permissions
>     
>     But what about the reverse? I mean `NodePublishVolumeRequest.readonly == false` and the volume has only the `SINGLE_NODE_READER_ONLY` capability. Do you think it is wrong?
>     
>     > However, I do think we should perform validation for our internal Volume::Mode field. If a task attempts to include a CSI volume which has only the SINGLE_NODE_READER_ONLY capability and the volume mode is set as RW, that does not seem valid to me. WDYT?
>     
>     Yes, I agree. I think I should add more validation here like: if the CSI volume has only the `SINGLE_NODE_READER_ONLY` or `MULTI_NODE_READER_ONLY` capability and our internal `Volume::Mode` field it set as `RW`, then we should return a failure. WDYT?

> But what about the reverse? I mean NodePublishVolumeRequest.readonly == false and the volume has only the SINGLE_NODE_READER_ONLY capability. Do you think it is wrong?

The 'readonly' field, when true, requires the volume to be mounted readonly. When 'readonly' is false, it doesn't require anything, and the volume can still be mounted readonly. So, I don't think the case you mentioned is wrong.

> I think I should add more validation here like: if the CSI volume has only the SINGLE_NODE_READER_ONLY or MULTI_NODE_READER_ONLY capability and our internal Volume::Mode field it set as RW, then we should return a failure. WDYT?

Yep, sounds good!


> On Aug. 6, 2020, 1:27 a.m., Greg Mann wrote:
> > src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp
> > Lines 316 (patched)
> > <https://reviews.apache.org/r/72733/diff/1/?file=2236985#file2236985line316>
> >
> >     Why do we do a recursive bind mount here, but not for volumes in the Linux filesystem isolator? https://github.com/apache/mesos/blob/c78dc333fc893a43d40dc33299a61987198a6ea9/src/slave/containerizer/mesos/isolators/filesystem/linux.cpp#L1067-L1068
> 
> Qian Zhang wrote:
>     The reason that we do a recursive bind mount here is, if there is any submounts under the `source`, we want all those submounts also bind mounted at the `target` so that the container can access them. We have this logic in all the volume isolators, like:
>     https://github.com/apache/mesos/blob/1.10.0/src/slave/containerizer/mesos/isolators/volume/host_path.cpp#L351
>     https://github.com/apache/mesos/blob/1.10.0/src/slave/containerizer/mesos/isolators/volume/image.cpp#L255
>     
>     The code that you pointed out in the Linux filesystem isolator is to mount the PV, since PV is originally created by Mesos and we are sure there is no submounts under it, so we do not need `MS_REC` in this case.
> 
> Greg Mann wrote:
>     Why do we know that a persistent volume will not have any mounts under it, but we don't know this for CSI volumes? The PV is originally created by Mesos, but applications can do whatever they want in the volume after it's created.
> 
> Qian Zhang wrote:
>     A CSI volume can be a pre-provisioned volume so it can already have some submounts under it before users try to use it via Mesos. But you are right, applicaiton can also create submounts under a PV. Maybe we should fix it for PV in a separate ticket? WDYT?

Yea I think you're right, we want to do a recursive mount in both cases. Let's fix the PV case in another patch.


- Greg


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


On Aug. 10, 2020, 2:37 a.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72733/
> -----------------------------------------------------------
> 
> (Updated Aug. 10, 2020, 2:37 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10153
>     https://issues.apache.org/jira/browse/MESOS-10153
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Implemented the `prepare` method of `volume/csi` isolator.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 0f91d88abba4bd49b46676e956211f1cf4e7219b 
>   include/mesos/v1/mesos.proto f25db8aeefd721653b2027282abbae8f2e6a40f3 
>   src/CMakeLists.txt 4e15e3d99aa2cce2403fe07e762fef2fb4a27dea 
>   src/Makefile.am 447db323875e4cad46000977f4a61600baff8f89 
>   src/common/validation.cpp 14a8c7b9d70c303b527e5e43012999471ced2d78 
>   src/slave/containerizer/mesos/isolators/volume/csi/isolator.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/volume/csi/state.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/volume/csi/state.proto PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/72733/diff/4/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


Re: Review Request 72733: Implemented the `prepare` method of `volume/csi` isolator.

Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72733/#review221469
-----------------------------------------------------------




src/slave/containerizer/mesos/isolators/volume/csi/isolator.hpp
Lines 85 (patched)
<https://reviews.apache.org/r/72733/#comment310485>

    Does the value of this map really need to be an `Owned`, rather than just a raw `Info`?



src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp
Lines 115-117 (patched)
<https://reviews.apache.org/r/72733/#comment310493>

    This seems pretty simple, could we do it now?



src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp
Lines 132-134 (patched)
<https://reviews.apache.org/r/72733/#comment310494>

    Do we have any validation code which could catch this earlier? Ditto for the `static_provisioning` check below.



src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp
Lines 146-151 (patched)
<https://reviews.apache.org/r/72733/#comment310495>

    Should we check `_volume.mode()` against the access mode contained within the `VolumeCapability`? Maybe we should have some separate validation code for that?



src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp
Lines 179-184 (patched)
<https://reviews.apache.org/r/72733/#comment310500>

    Can you help me understand why we do not require an absolute container path to already exist in this case, while we require it in other cases (both here and in the Linux filesystem isolator)?



src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp
Lines 229 (patched)
<https://reviews.apache.org/r/72733/#comment310496>

    How about "Create the directory to store this container's CSI volume state." ?



src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp
Lines 247 (patched)
<https://reviews.apache.org/r/72733/#comment310497>

    I think you can eliminate the `stringify` here, since we have a templated function which can take protobuf messages: https://github.com/apache/mesos/blob/877b5886804a17ea4aee4f251a43a3f5dbc84ca1/src/slave/state.hpp#L213
    
    Also, it looks like the checkpoint() function will create the target directory, so you could eliminate the call to `os::mkdir(containerDir)` above.



src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp
Lines 287-300 (patched)
<https://reviews.apache.org/r/72733/#comment310498>

    Can we eliminate this if you use `collect()` above instead of `await()`?



src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp
Lines 316 (patched)
<https://reviews.apache.org/r/72733/#comment310499>

    Why do we do a recursive bind mount here, but not for volumes in the Linux filesystem isolator? https://github.com/apache/mesos/blob/c78dc333fc893a43d40dc33299a61987198a6ea9/src/slave/containerizer/mesos/isolators/filesystem/linux.cpp#L1067-L1068



src/slave/containerizer/mesos/isolators/volume/csi/state.hpp
Lines 20 (patched)
<https://reviews.apache.org/r/72733/#comment310486>

    Also need includes for `std::string` and `std::hash`.


- Greg Mann


On Aug. 4, 2020, 8:20 a.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72733/
> -----------------------------------------------------------
> 
> (Updated Aug. 4, 2020, 8:20 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10153
>     https://issues.apache.org/jira/browse/MESOS-10153
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Implemented the `prepare` method of `volume/csi` isolator.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt 4e15e3d99aa2cce2403fe07e762fef2fb4a27dea 
>   src/Makefile.am 447db323875e4cad46000977f4a61600baff8f89 
>   src/slave/containerizer/mesos/isolators/volume/csi/isolator.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/volume/csi/state.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/volume/csi/state.proto PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/72733/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


Re: Review Request 72733: Implemented the `prepare` method of `volume/csi` isolator.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72733/#review221476
-----------------------------------------------------------



Bad patch!

Reviews applied: [72660, 72661, 72672, 72681, 72715, 72707, 72716, 72690, 72733]

Failed command: ['bash', '-c', "set -o pipefail; export OS='ubuntu:16.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers --disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/jenkins/buildbot.sh 2>&1 | tee build_72733"]

Error:
ubuntu-16.04: Pulling from mesos/mesos-build
18d680d61657: Pulling fs layer
0addb6fece63: Pulling fs layer
78e58219b215: Pulling fs layer
eb6959a66df2: Pulling fs layer
1105027a5560: Pulling fs layer
c36946130a38: Pulling fs layer
6a6a5e68faab: Pulling fs layer
80e07249924c: Pulling fs layer
c4c63e2501db: Pulling fs layer
668b207c2829: Pulling fs layer
ed76dddad568: Pulling fs layer
6a6a5e68faab: Waiting
1105027a5560: Waiting
668b207c2829: Waiting
c36946130a38: Waiting
80e07249924c: Waiting
eb6959a66df2: Waiting
ed76dddad568: Waiting
c4c63e2501db: Waiting
78e58219b215: Verifying Checksum
78e58219b215: Download complete
0addb6fece63: Download complete
eb6959a66df2: Download complete
18d680d61657: Verifying Checksum
18d680d61657: Download complete
c36946130a38: Verifying Checksum
c36946130a38: Download complete
6a6a5e68faab: Verifying Checksum
6a6a5e68faab: Download complete
80e07249924c: Download complete
668b207c2829: Verifying Checksum
668b207c2829: Download complete
c4c63e2501db: Verifying Checksum
c4c63e2501db: Download complete
ed76dddad568: Verifying Checksum
ed76dddad568: Download complete
18d680d61657: Pull complete
0addb6fece63: Pull complete
78e58219b215: Pull complete
eb6959a66df2: Pull complete
1105027a5560: Verifying Checksum
1105027a5560: Download complete
1105027a5560: Pull complete
c36946130a38: Pull complete
6a6a5e68faab: Pull complete
80e07249924c: Pull complete
c4c63e2501db: Pull complete
668b207c2829: Pull complete
ed76dddad568: Pull complete
Digest: sha256:fa967cbcfb44f55708a3cbc87f245c6d29dd891464db558af56a03ee321526bb
Status: Downloaded newer image for mesos/mesos-build:ubuntu-16.04
docker.io/mesos/mesos-build:ubuntu-16.04
Cloning into '/tmp/SRC'...
Note: checking out 'fe1af8932251b8a6755a9945af6d372e5b2f130c'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by performing another checkout.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -b with the checkout command again. Example:

  git checkout -b <new-branch-name>

autoreconf: Entering directory `.'
autoreconf: configure.ac: not using Gettext
autoreconf: running: aclocal --warnings=all -I m4
autoreconf: configure.ac: tracing
configure.ac:1873: warning: cannot check for file existence when cross compiling
../../lib/autoconf/general.m4:2788: AC_CHECK_FILE is expanded from...
configure.ac:1873: the top level
configure.ac:1878: warning: cannot check for file existence when cross compiling
../../lib/autoconf/general.m4:2788: AC_CHECK_FILE is expanded from...
configure.ac:1878: the top level
configure.ac:2399: warning: AC_RUN_IFELSE called without default to allow cross compiling
../../lib/autoconf/general.m4:2759: AC_RUN_IFELSE is expanded from...
configure.ac:2399: the top level
autoreconf: running: libtoolize --copy
libtoolize: putting auxiliary files in '.'.
libtoolize: copying file './ltmain.sh'
libtoolize: putting macros in AC_CONFIG_MACRO_DIRS, 'm4'.
libtoolize: copying file 'm4/libtool.m4'
libtoolize: copying file 'm4/ltoptions.m4'
libtoolize: copying file 'm4/ltsugar.m4'
libtoolize: copying file 'm4/ltversion.m4'
libtoolize: copying file 'm4/lt~obsolete.m4'
autoreconf: running: /usr/bin/autoconf --warnings=all
configure.ac:1873: warning: cannot check for file existence when cross compiling
../../lib/autoconf/general.m4:2788: AC_CHECK_FILE is expanded from...
configure.ac:1873: the top level
configure.ac:1878: warning: cannot check for file existence when cross compiling
../../lib/autoconf/general.m4:2788: AC_CHECK_FILE is expanded from...
configure.ac:1878: the top level
configure.ac:2399: warning: AC_RUN_IFELSE called without default to allow cross compiling
../../lib/autoconf/general.m4:2759: AC_RUN_IFELSE is expanded from...
configure.ac:2399: the top level
autoreconf: configure.ac: not using Autoheader
autoreconf: running: automake --add-missing --copy --no-force --warnings=all
configure.ac:50: installing './ar-lib'
configure.ac:34: installing './compile'
configure.ac:24: installing './config.guess'
configure.ac:24: installing './config.sub'
configure.ac:46: installing './install-sh'
configure.ac:46: installing './missing'
3rdparty/Makefile.am:307: warning: source file '$(HTTP_PARSER)/http_parser.c' is in a subdirectory,
3rdparty/Makefile.am:307: but option 'subdir-objects' is disabled
automake: warning: possible forward-incompatibility.
automake: At least a source file is in a subdirectory, but the 'subdir-objects'
automake: automake option hasn't been enabled.  For now, the corresponding output
automake: object file(s) will be placed in the top-level directory.  However,
automake: this behaviour will change in future Automake versions: they will
automake: unconditionally cause object files to be placed in the same subdirectory
automake: of the corresponding sources.
automake: You are advised to start using 'subdir-objects' option throughout your
automake: project, to avoid future incompatibilities.
3rdparty/Makefile.am: installing './depcomp'
3rdparty/Makefile.am:262: warning: variable 'GLOG_LDFLAGS' is defined but no program or
3rdparty/Makefile.am:262: library has 'GLOG' as canonical name (possible typo)
3rdparty/stout/Makefile.am:138: warning: source file 'tests/adaptor_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/archiver_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/base64_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/bits_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/boundedhashmap_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/bytes_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/cache_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/cpp17_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/duration_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/dynamiclibrary_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/error_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/flags_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/gzip_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/hashmap_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/hashset_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/interval_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/ip_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/json_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/jsonify_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/lambda_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/linkedhashmap_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/mac_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/main.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/multimap_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/none_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/numify_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/option_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/os_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/os/copyfile_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/os/env_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/os/filesystem_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/os/process_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/os/rmdir_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/os/sendfile_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/os/signals_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/os/socket_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/os/strerror_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/os/systems_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/path_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/protobuf_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/recordio_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/result_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/some_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/strings_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/subcommand_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/svn_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/try_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/uri_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/uuid_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/variant_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:138: warning: source file 'tests/version_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:138: but option 'subdir-objects' is disabled
3rdparty/stout/Makefile.am:193: warning: source file 'tests/proc_tests.cpp' is in a subdirectory,
3rdparty/stout/Makefile.am:193: but option 'subdir-objects' is disabled
autoreconf: Leaving directory `.'
checking build system type... x86_64-pc-linux-gnu
checking host system type... x86_64-pc-linux-gnu
checking target system type... x86_64-pc-linux-gnu
checking whether the C++ compiler works... yes
checking for C++ compiler default output file name... a.out
checking for suffix of executables... 
checking whether we are cross compiling... no
checking for suffix of object files... o
checking whether we are using the GNU C++ compiler... yes
checking whether g++ accepts -g... yes
checking whether we are using the GNU C compiler... yes
checking whether gcc accepts -g... yes
checking for gcc option to accept ISO C89... none needed
checking whether gcc understands -c and -o together... yes
checking whether ln -s works... yes
checking for C++ compiler vendor... gnu
checking for a sed that does not truncate output... /bin/sed
checking for C++ compiler version... 5.4.0
checking for C++ compiler vendor... (cached) gnu
checking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... yes
checking for a thread-safe mkdir -p... /bin/mkdir -p
checking for gawk... no
checking for mawk... mawk
checking whether make sets $(MAKE)... yes
checking for style of include used by make... GNU
checking whether make supports nested variables... yes
checking how to create a pax tar archive... gnutar
checking dependency style of gcc... gcc3
checking dependency style of g++... gcc3
checking whether to enable maintainer-specific portions of Makefiles... yes
checking for ar... ar
checking the archiver (ar) interface... ar
checking how to print strings... printf
checking for a sed that does not truncate output... (cached) /bin/sed
checking for grep that handles long lines and -e... /bin/grep
checking for egrep... /bin/grep -E
checking for fgrep... /bin/grep -F
checking for ld used by gcc... /usr/bin/ld
checking if the linker (/usr/bin/ld) is GNU ld... yes
checking for BSD- or MS-compatible name lister (nm)... /usr/bin/nm -B
checking the name lister (/usr/bin/nm -B) interface... BSD nm
checking the maximum length of command line arguments... 1572864
checking how to convert x86_64-pc-linux-gnu file names to x86_64-pc-linux-gnu format... func_convert_file_noop
checking how to convert x86_64-pc-linux-gnu file names to toolchain format... func_convert_file_noop
checking for /usr/bin/ld option to reload object files... -r
checking for objdump... objdump
checking how to recognize dependent libraries... pass_all
checking for dlltool... no
checking how to associate runtime and link libraries... printf %s\n
checking for archiver @FILE support... @
checking for strip... strip
checking for ranlib... ranlib
checking command to parse /usr/bin/nm -B output from gcc object... ok
checking for sysroot... no
checking for a working dd... /bin/dd
checking how to truncate binary pipes... /bin/dd bs=4096 count=1
checking for mt... no
checking if : is a manifest tool... no
checking how to run the C preprocessor... gcc -E
checking for ANSI C header files... yes
checking for sys/types.h... yes
checking for sys/stat.h... yes
checking for stdlib.h... yes
checking for string.h... yes
checking for memory.h... yes
checking for strings.h... yes
checking for inttypes.h... yes
checking for stdint.h... yes
checking for unistd.h... yes
checking for dlfcn.h... yes
checking for objdir... .libs
checking if gcc supports -fno-rtti -fno-exceptions... no
checking for gcc option to produce PIC... -fPIC -DPIC
checking if gcc PIC flag -fPIC -DPIC works... yes
checking if gcc static flag -static works... yes
checking if gcc supports -c -o file.o... yes
checking if gcc supports -c -o file.o... (cached) yes
checking whether the gcc linker (/usr/bin/ld -m elf_x86_64) supports shared libraries... yes
checking whether -lc should be explicitly linked in... no
checking dynamic linker characteristics... GNU/Linux ld.so
checking how to hardcode library paths into programs... immediate
checking whether stripping libraries is possible... yes
checking if libtool supports shared libraries... yes
checking whether to build shared libraries... yes
checking whether to build static libraries... no
checking how to run the C++ preprocessor... g++ -E
checking for ld used by g++... /usr/bin/ld -m elf_x86_64
checking if the linker (/usr/bin/ld -m elf_x86_64) is GNU ld... yes
checking whether the g++ linker (/usr/bin/ld -m elf_x86_64) supports shared libraries... yes
checking for g++ option to produce PIC... -fPIC -DPIC
checking if g++ PIC flag -fPIC -DPIC works... yes
checking if g++ static flag -static works... yes
checking if g++ supports -c -o file.o... yes
checking if g++ supports -c -o file.o... (cached) yes
checking whether the g++ linker (/usr/bin/ld -m elf_x86_64) supports shared libraries... yes
checking dynamic linker characteristics... (cached) GNU/Linux ld.so
checking how to hardcode library paths into programs... immediate
configure: creating ./config.lt
config.lt: creating libtool
checking whether to enable GC of unused sections... no
configure: Setting up CXXFLAGS for g++ version >= 4.8
checking whether C++ compiler accepts -fstack-protector-strong... yes
checking whether g++ supports C++11 features by default... no
checking whether g++ supports C++11 features with -std=c++11... yes
checking if compiler needs -Werror to reject unknown flags... no
checking for the pthreads library -lpthreads... no
checking whether pthreads work without any flags... no
checking whether pthreads work with -Kthread... no
checking whether pthreads work with -kthread... no
checking for the pthreads library -llthread... no
checking whether pthreads work with -pthread... yes
checking for joinable pthread attribute... PTHREAD_CREATE_JOINABLE
checking if more special flags are required for pthreads... no
checking for PTHREAD_PRIO_INHERIT... yes
configure: Setting up build environment for x86_64 linux-gnu
checking for backtrace in -lunwind... no
checking for RAND_poll in -lcrypto... yes
checking openssl/ssl.h usability... yes
checking openssl/ssl.h presence... yes
checking for openssl/ssl.h... yes
checking for SSL_CTX_new in -lssl... yes
checking for main in -lgflags... no
checking for patch... patch
checking fts.h usability... yes
checking fts.h presence... yes
checking for fts.h... yes
checking for library containing fts_close... none required
checking apr_pools.h usability... yes
checking apr_pools.h presence... yes
checking for apr_pools.h... yes
checking for apr_initialize in -lapr-1... yes
checking for curl_global_init in -lcurl... yes
checking for javac... /usr/bin/javac
checking for java... /usr/bin/java
checking value of Java system property 'java.home'... /usr/lib/jvm/java-8-openjdk-amd64/jre
configure: using JAVA_HOME=/usr/lib/jvm/java-8-openjdk-amd64
checking whether or not we can build with JNI... yes
checking for mvn... /usr/bin/mvn
checking for javah... /usr/lib/jvm/java-8-openjdk-amd64/bin/javah
checking whether to enable launcher sealing... no
checking whether to enable the Seccomp isolator... no
checking for sasl_done in -lsasl2... yes
checking SASL CRAM-MD5 support... yes
checking svn_version.h usability... yes
checking svn_version.h presence... yes
checking for svn_version.h... yes
checking for svn_stringbuf_create_ensure in -lsvn_subr-1... yes
checking svn_delta.h usability... yes
checking svn_delta.h presence... yes
checking for svn_delta.h... yes
checking for svn_txdelta in -lsvn_delta-1... yes
checking whether to enable the XFS disk isolator... no
checking zlib.h usability... yes
checking zlib.h presence... yes
checking for zlib.h... yes
checking for deflate, gzread, gzwrite, inflate in -lz... yes
checking C++ standard library for undefined behaviour with selected optimization level... no
checking for python... /usr/bin/python
checking for python version... 2.7
checking for python platform... linux2
checking for python script directory... ${prefix}/lib/python2.7/dist-packages
checking for python extension module directory... ${exec_prefix}/lib/python2.7/dist-packages
checking for python2.7... (cached) /usr/bin/python
checking for a version of Python >= '2.1.0'... yes
checking for a version of Python >= '2.6'... yes
checking for the distutils Python package... yes
checking for Python include path... -I/usr/include/python2.7
checking for Python library path... -L/usr/lib -lpython2.7
checking for Python site-packages path... /usr/lib/python2.7/dist-packages
checking python extra libraries... -lpthread -ldl  -lutil -lm
checking python extra linking flags... -Xlinker -export-dynamic -Wl,-O1 -Wl,-Bsymbolic-functions
checking consistency of all components of python development environment... yes
checking whether we can build usable Python eggs... cc1plus: warning: command line option '-Wstrict-prototypes' is valid for C/ObjC but not for C++
yes
checking for an old installation of the Mesos egg (before 0.20.0)... no
checking whether to enable new CLI... no
checking src/common/git_version.hpp presence... no
configure: generating src/common/git_version.hpp
checking that generated files are newer than configure... done
configure: creating ./config.status
config.status: creating Makefile
config.status: creating mesos.pc
config.status: creating src/Makefile
config.status: creating 3rdparty/Makefile
config.status: creating 3rdparty/libprocess/Makefile
config.status: creating 3rdparty/libprocess/include/Makefile
config.status: creating 3rdparty/stout/Makefile
config.status: creating 3rdparty/stout/include/Makefile
config.status: creating 3rdparty/gmock_sources.cc
config.status: creating bin/mesos.sh
config.status: creating bin/mesos-agent.sh
config.status: creating bin/mesos-local.sh
config.status: creating bin/mesos-master.sh
config.status: creating bin/mesos-slave.sh
config.status: creating bin/mesos-tests.sh
config.status: creating bin/mesos-agent-flags.sh
config.status: creating bin/mesos-local-flags.sh
config.status: creating bin/mesos-master-flags.sh
config.status: creating bin/mesos-slave-flags.sh
config.status: creating bin/mesos-tests-flags.sh
config.status: creating bin/gdb-mesos-agent.sh
config.status: creating bin/gdb-mesos-local.sh
config.status: creating bin/gdb-mesos-master.sh
config.status: creating bin/gdb-mesos-slave.sh
config.status: creating bin/gdb-mesos-tests.sh
config.status: creating bin/lldb-mesos-agent.sh
config.status: creating bin/lldb-mesos-local.sh
config.status: creating bin/lldb-mesos-master.sh
config.status: creating bin/lldb-mesos-slave.sh
config.status: creating bin/lldb-mesos-tests.sh
config.status: creating bin/valgrind-mesos-agent.sh
config.status: creating bin/valgrind-mesos-local.sh
config.status: creating bin/valgrind-mesos-master.sh
config.status: creating bin/valgrind-mesos-slave.sh
config.status: creating bin/valgrind-mesos-tests.sh
config.status: creating src/deploy/mesos-daemon.sh
config.status: creating src/deploy/mesos-start-agents.sh
config.status: creating src/deploy/mesos-start-cluster.sh
config.status: creating src/deploy/mesos-start-masters.sh
config.status: creating src/deploy/mesos-start-slaves.sh
config.status: creating src/deploy/mesos-stop-agents.sh
config.status: creating src/deploy/mesos-stop-cluster.sh
config.status: creating src/deploy/mesos-stop-masters.sh
config.status: creating src/deploy/mesos-stop-slaves.sh
config.status: creating include/mesos/version.hpp
config.status: creating src/java/generated/org/apache/mesos/MesosNativeLibrary.java
config.status: creating src/examples/java/test-exception-framework
config.status: creating src/examples/java/test-executor
config.status: creating src/examples/java/test-framework
config.status: creating src/examples/java/test-multiple-executors-framework
config.status: creating src/examples/java/test-log
config.status: creating src/examples/java/v1-test-framework
config.status: creating src/java/mesos.pom
config.status: creating src/examples/python/test-executor
config.status: creating src/examples/python/test-framework
config.status: creating src/python/setup.py
config.status: creating src/python/cli/setup.py
config.status: creating src/python/interface/setup.py
config.status: creating src/python/native_common/ext_modules.py
config.status: creating src/python/executor/setup.py
config.status: creating src/python/native/setup.py
config.status: creating src/python/scheduler/setup.py
config.status: creating src/common/git_version.hpp
config.status: linking src/python/native_common/ext_modules.py to src/python/executor/ext_modules.py
config.status: linking src/python/native_common/ext_modules.py to src/python/scheduler/ext_modules.py
config.status: executing depfiles commands
config.status: executing libtool commands
configure: Build option summary:
    CXX:        g++
    CXXFLAGS:   -g1 -O0 -Wno-unused-local-typedefs -std=c++11
    CPPFLAGS:   -I/usr/include/subversion-1 -I/usr/include/apr-1 -I/usr/include/apr-1.0	      
    LDFLAGS:    
    LIBS:       -lz -lsvn_delta-1 -lsvn_subr-1 -lsasl2 -lcurl -lapr-1  -lrt
    JAVA_TEST_LDFLAGS: -L/usr/lib/jvm/java-8-openjdk-amd64/jre/lib/amd64/server -R/usr/lib/jvm/java-8-openjdk-amd64/jre/lib/amd64/server -Wl,-ljvm
    JAVA_JVM_LIBRARY:  /usr/lib/jvm/java-8-openjdk-amd64/jre/lib/amd64/server/libjvm.so

make  dist-gzip am__post_remove_distdir='@:'
make[1]: Entering directory '/tmp/SRC/build'
if test -d "mesos-1.11.0"; then find "mesos-1.11.0" -type d ! -perm -200 -exec chmod u+w {} ';' && rm -rf "mesos-1.11.0" || { sleep 5 && rm -rf "mesos-1.11.0"; }; else :; fi
test -d "mesos-1.11.0" || mkdir "mesos-1.11.0"
 (cd 3rdparty && make  top_distdir=../mesos-1.11.0 distdir=../mesos-1.11.0/3rdparty \
     am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[2]: Entering directory '/tmp/SRC/build/3rdparty'
 (cd stout && make  top_distdir=../../mesos-1.11.0 distdir=../../mesos-1.11.0/3rdparty/stout \
     am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[3]: Entering directory '/tmp/SRC/build/3rdparty/stout'
 (cd include && make  top_distdir=../../../mesos-1.11.0 distdir=../../../mesos-1.11.0/3rdparty/stout/include \
     am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[4]: Entering directory '/tmp/SRC/build/3rdparty/stout/include'
make[4]: Leaving directory '/tmp/SRC/build/3rdparty/stout/include'
make  \
  top_distdir="../../mesos-1.11.0" distdir="../../mesos-1.11.0/3rdparty/stout" \
  dist-hook
make[4]: Entering directory '/tmp/SRC/build/3rdparty/stout'
cp -r ../../../3rdparty/stout/3rdparty ../../mesos-1.11.0/3rdparty/stout/
make[4]: Leaving directory '/tmp/SRC/build/3rdparty/stout'
make[3]: Leaving directory '/tmp/SRC/build/3rdparty/stout'
 (cd libprocess && make  top_distdir=../../mesos-1.11.0 distdir=../../mesos-1.11.0/3rdparty/libprocess \
     am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[3]: Entering directory '/tmp/SRC/build/3rdparty/libprocess'
 (cd include && make  top_distdir=../../../mesos-1.11.0 distdir=../../../mesos-1.11.0/3rdparty/libprocess/include \
     am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[4]: Entering directory '/tmp/SRC/build/3rdparty/libprocess/include'
make[4]: Leaving directory '/tmp/SRC/build/3rdparty/libprocess/include'
make  \
  top_distdir="../../mesos-1.11.0" distdir="../../mesos-1.11.0/3rdparty/libprocess" \
  dist-hook
make[4]: Entering directory '/tmp/SRC/build/3rdparty/libprocess'
cp -r ../../../3rdparty/libprocess/3rdparty ../../mesos-1.11.0/3rdparty/libprocess/
make[4]: Leaving directory '/tmp/SRC/build/3rdparty/libprocess'
make[3]: Leaving directory '/tmp/SRC/build/3rdparty/libprocess'
make[2]: Leaving directory '/tmp/SRC/build/3rdparty'
 (cd src && make  top_distdir=../mesos-1.11.0 distdir=../mesos-1.11.0/src \
     am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[2]: Entering directory '/tmp/SRC/build/src'
../3rdparty/protobuf-3.5.0/src/protoc -I../../include -I../../src -I../3rdparty/csi-0.2.0 -I../3rdparty/csi-1.1.0 -I../3rdparty/protobuf-3.5.0/src  --cpp_out=. ../../src/tests/common/http_tests.proto
/bin/bash: ../3rdparty/protobuf-3.5.0/src/protoc: No such file or directory
Makefile:15659: recipe for target 'tests/common/http_tests.pb.cc' failed
make[2]: *** [tests/common/http_tests.pb.cc] Error 127
make[2]: Leaving directory '/tmp/SRC/build/src'
Makefile:885: recipe for target 'distdir' failed
make[1]: *** [distdir] Error 1
make[1]: Leaving directory '/tmp/SRC/build'
Makefile:984: recipe for target 'dist' failed
make: *** [dist] Error 2
Untagged: mesos/mesos-build:ubuntu-16.04
Untagged: mesos/mesos-build@sha256:fa967cbcfb44f55708a3cbc87f245c6d29dd891464db558af56a03ee321526bb
Deleted: sha256:e895c0531b9a9a288fabe479a49f7059aed83645351ac99ec2ea2616822c9f97
Deleted: sha256:09119b0b314a69ba6ec6251f2e89f4199fe1e874c84f9abf07dcbd23dbc3f1c1
Deleted: sha256:562fe6af5a3883058be9c784a839392215ed4185a21d21f1d99df0d17f3ae6e7
Deleted: sha256:0dcdfbe322a4f332f73ac70905d557300ec6dae3bd19586758772c750c7b4a19
Deleted: sha256:c66652d605f01094e2db53d62505dbd524e076d5aa69b89d5e620003803eb149
Deleted: sha256:a2cf79bfb9593c05fd7142ddb49afef77ea4ad5d2464e841f09fe62ffee396e0
Deleted: sha256:f7a904214b390f39d98573882f631dc908df8b2b540cf04e1062f8182c1efffd
Deleted: sha256:ce1f6fcaa83dfce189d76e08f184085732eab4eeb2562d2399953958405c5bec
Deleted: sha256:cce92fda689ab9033f0b8db214bc63edd1ae3e05831a0f3a9418976d7dc7ccdd
Deleted: sha256:d22094bbd65447c59a42c580eaa3a44cee9cd855f00905f59409be21bcefc745
Deleted: sha256:b8976847450013f3eb5e9a81a5778f73ed7bef67e6393049712ef17102b4b7b7
Deleted: sha256:b8c891f0ffec910a12757d733b178e3f62d81dbbde2b31d3b754071c416108ed

Full log: https://builds.apache.org/job/Mesos-Reviewbot-Linux/11685/console

- Mesos Reviewbot


On Aug. 4, 2020, 8:20 a.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72733/
> -----------------------------------------------------------
> 
> (Updated Aug. 4, 2020, 8:20 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10153
>     https://issues.apache.org/jira/browse/MESOS-10153
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Implemented the `prepare` method of `volume/csi` isolator.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt 4e15e3d99aa2cce2403fe07e762fef2fb4a27dea 
>   src/Makefile.am 447db323875e4cad46000977f4a61600baff8f89 
>   src/slave/containerizer/mesos/isolators/volume/csi/isolator.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/volume/csi/state.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/volume/csi/state.proto PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/72733/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


Re: Review Request 72733: Implemented the `prepare` method of `volume/csi` isolator.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72733/#review221464
-----------------------------------------------------------



Patch looks great!

Reviews applied: [72660, 72661, 72672, 72681, 72715, 72707, 72716, 72690, 72733]

Passed command: export OS='ubuntu:16.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers --disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/jenkins/buildbot.sh

- Mesos Reviewbot


On Aug. 4, 2020, 4:20 p.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72733/
> -----------------------------------------------------------
> 
> (Updated Aug. 4, 2020, 4:20 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10153
>     https://issues.apache.org/jira/browse/MESOS-10153
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Implemented the `prepare` method of `volume/csi` isolator.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt 4e15e3d99aa2cce2403fe07e762fef2fb4a27dea 
>   src/Makefile.am 447db323875e4cad46000977f4a61600baff8f89 
>   src/slave/containerizer/mesos/isolators/volume/csi/isolator.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/volume/csi/state.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/volume/csi/state.proto PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/72733/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


Re: Review Request 72733: Implemented the `prepare` method of `volume/csi` isolator.

Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72733/#review221613
-----------------------------------------------------------


Ship it!




Ship It!

- Greg Mann


On Aug. 11, 2020, 8:32 a.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72733/
> -----------------------------------------------------------
> 
> (Updated Aug. 11, 2020, 8:32 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10153
>     https://issues.apache.org/jira/browse/MESOS-10153
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Implemented the `prepare` method of `volume/csi` isolator.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 0f91d88abba4bd49b46676e956211f1cf4e7219b 
>   include/mesos/v1/mesos.proto f25db8aeefd721653b2027282abbae8f2e6a40f3 
>   src/CMakeLists.txt c60d98a21bbd422a5ee1c22f7281bc701e5ade14 
>   src/Makefile.am 49dab4b6488b75d32e6ee9d54d68afe36549c353 
>   src/common/validation.cpp 14a8c7b9d70c303b527e5e43012999471ced2d78 
>   src/slave/containerizer/mesos/isolators/volume/csi/isolator.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/volume/csi/state.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/volume/csi/state.proto PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/72733/diff/5/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


Re: Review Request 72733: Implemented the `prepare` method of `volume/csi` isolator.

Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72733/
-----------------------------------------------------------

(Updated Aug. 11, 2020, 4:32 p.m.)


Review request for mesos, Andrei Budnik and Greg Mann.


Changes
-------

Addressed review comments.


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


Repository: mesos


Description
-------

Implemented the `prepare` method of `volume/csi` isolator.


Diffs (updated)
-----

  include/mesos/mesos.proto 0f91d88abba4bd49b46676e956211f1cf4e7219b 
  include/mesos/v1/mesos.proto f25db8aeefd721653b2027282abbae8f2e6a40f3 
  src/CMakeLists.txt c60d98a21bbd422a5ee1c22f7281bc701e5ade14 
  src/Makefile.am 49dab4b6488b75d32e6ee9d54d68afe36549c353 
  src/common/validation.cpp 14a8c7b9d70c303b527e5e43012999471ced2d78 
  src/slave/containerizer/mesos/isolators/volume/csi/isolator.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/volume/csi/state.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/volume/csi/state.proto PRE-CREATION 


Diff: https://reviews.apache.org/r/72733/diff/5/

Changes: https://reviews.apache.org/r/72733/diff/4-5/


Testing
-------


Thanks,

Qian Zhang


Re: Review Request 72733: Implemented the `prepare` method of `volume/csi` isolator.

Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72733/
-----------------------------------------------------------

(Updated Aug. 10, 2020, 10:37 a.m.)


Review request for mesos, Andrei Budnik and Greg Mann.


Changes
-------

Rebased.


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


Repository: mesos


Description
-------

Implemented the `prepare` method of `volume/csi` isolator.


Diffs (updated)
-----

  include/mesos/mesos.proto 0f91d88abba4bd49b46676e956211f1cf4e7219b 
  include/mesos/v1/mesos.proto f25db8aeefd721653b2027282abbae8f2e6a40f3 
  src/CMakeLists.txt 4e15e3d99aa2cce2403fe07e762fef2fb4a27dea 
  src/Makefile.am 447db323875e4cad46000977f4a61600baff8f89 
  src/common/validation.cpp 14a8c7b9d70c303b527e5e43012999471ced2d78 
  src/slave/containerizer/mesos/isolators/volume/csi/isolator.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/volume/csi/state.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/volume/csi/state.proto PRE-CREATION 


Diff: https://reviews.apache.org/r/72733/diff/4/

Changes: https://reviews.apache.org/r/72733/diff/3-4/


Testing
-------


Thanks,

Qian Zhang


Re: Review Request 72733: Implemented the `prepare` method of `volume/csi` isolator.

Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72733/
-----------------------------------------------------------

(Updated Aug. 7, 2020, 11:20 a.m.)


Review request for mesos, Andrei Budnik and Greg Mann.


Changes
-------

Addressed review comments.


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


Repository: mesos


Description
-------

Implemented the `prepare` method of `volume/csi` isolator.


Diffs (updated)
-----

  include/mesos/mesos.proto 0f91d88abba4bd49b46676e956211f1cf4e7219b 
  include/mesos/v1/mesos.proto f25db8aeefd721653b2027282abbae8f2e6a40f3 
  src/CMakeLists.txt 4e15e3d99aa2cce2403fe07e762fef2fb4a27dea 
  src/Makefile.am 447db323875e4cad46000977f4a61600baff8f89 
  src/common/validation.cpp 14a8c7b9d70c303b527e5e43012999471ced2d78 
  src/slave/containerizer/mesos/isolators/volume/csi/isolator.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/volume/csi/state.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/volume/csi/state.proto PRE-CREATION 


Diff: https://reviews.apache.org/r/72733/diff/3/

Changes: https://reviews.apache.org/r/72733/diff/2-3/


Testing
-------


Thanks,

Qian Zhang


Re: Review Request 72733: Implemented the `prepare` method of `volume/csi` isolator.

Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72733/
-----------------------------------------------------------

(Updated Aug. 6, 2020, 5:23 p.m.)


Review request for mesos, Andrei Budnik and Greg Mann.


Changes
-------

Addressed review comments.


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


Repository: mesos


Description
-------

Implemented the `prepare` method of `volume/csi` isolator.


Diffs (updated)
-----

  src/CMakeLists.txt 4e15e3d99aa2cce2403fe07e762fef2fb4a27dea 
  src/Makefile.am 447db323875e4cad46000977f4a61600baff8f89 
  src/common/validation.cpp 14a8c7b9d70c303b527e5e43012999471ced2d78 
  src/slave/containerizer/mesos/isolators/volume/csi/isolator.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/volume/csi/state.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/volume/csi/state.proto PRE-CREATION 


Diff: https://reviews.apache.org/r/72733/diff/2/

Changes: https://reviews.apache.org/r/72733/diff/1-2/


Testing
-------


Thanks,

Qian Zhang