You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Greg Mann <gr...@mesosphere.io> on 2020/07/31 19:06:35 UTC
Review Request 72727: Updated the test CSI plugin for CSI server
testing.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72727/
-----------------------------------------------------------
Review request for mesos, Andrei Budnik and Qian Zhang.
Bugs: MESOS-10163
https://issues.apache.org/jira/browse/MESOS-10163
Repository: mesos
Description
-------
This patch adds additional configuration flags to the
test CSI plugin which are necessary in order to test
the agent's CSI server.
Diffs
-----
src/examples/test_csi_plugin.cpp 6202173844a93b8d9642208bd86839b3cf56bce2
Diff: https://reviews.apache.org/r/72727/diff/1/
Testing
-------
These new flags are used in a subsequent test in this chain.
Thanks,
Greg Mann
Re: Review Request 72727: Updated the test CSI plugin for CSI server
testing.
Posted by Greg Mann <gr...@mesosphere.io>.
> On Aug. 12, 2020, 3:17 a.m., Qian Zhang wrote:
> > src/examples/test_csi_plugin.cpp
> > Lines 174-177 (patched)
> > <https://reviews.apache.org/r/72727/diff/3/?file=2237824#file2237824line174>
> >
> > Why do we need this flag? Without this flag, would the unit test in https://reviews.apache.org/r/72728 be affected?
Whoops this is no longer needed, sorry.
- Greg
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72727/#review221554
-----------------------------------------------------------
On Aug. 18, 2020, 1:23 a.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72727/
> -----------------------------------------------------------
>
> (Updated Aug. 18, 2020, 1:23 a.m.)
>
>
> Review request for mesos, Andrei Budnik and Qian Zhang.
>
>
> Bugs: MESOS-10163
> https://issues.apache.org/jira/browse/MESOS-10163
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds additional configuration flags to the
> test CSI plugin which are necessary in order to test
> the agent's CSI server.
>
>
> Diffs
> -----
>
> src/examples/test_csi_plugin.cpp 214a3ee4481512b025ff054db6ffe15d476b0264
>
>
> Diff: https://reviews.apache.org/r/72727/diff/4/
>
>
> Testing
> -------
>
> These new flags are used in a subsequent test in this chain.
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 72727: Updated the test CSI plugin for CSI server
testing.
Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72727/#review221554
-----------------------------------------------------------
src/examples/test_csi_plugin.cpp
Lines 174-177 (patched)
<https://reviews.apache.org/r/72727/#comment310622>
Why do we need this flag? Without this flag, would the unit test in https://reviews.apache.org/r/72728 be affected?
src/examples/test_csi_plugin.cpp
Line 1590 (original), 1614 (patched)
<https://reviews.apache.org/r/72727/#comment310623>
I think this will break the case when `--volume_id_path` is set to true, in which case `volumeInfo.id` is already set to `getVolumePath(capacity, name)` in the constructor and now here we call `getVolumePath` again which will give us a wrong volume path.
- Qian Zhang
On Aug. 12, 2020, 2:55 a.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72727/
> -----------------------------------------------------------
>
> (Updated Aug. 12, 2020, 2:55 a.m.)
>
>
> Review request for mesos, Andrei Budnik and Qian Zhang.
>
>
> Bugs: MESOS-10163
> https://issues.apache.org/jira/browse/MESOS-10163
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds additional configuration flags to the
> test CSI plugin which are necessary in order to test
> the agent's CSI server.
>
>
> Diffs
> -----
>
> src/examples/test_csi_plugin.cpp 214a3ee4481512b025ff054db6ffe15d476b0264
>
>
> Diff: https://reviews.apache.org/r/72727/diff/3/
>
>
> Testing
> -------
>
> These new flags are used in a subsequent test in this chain.
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 72727: Updated the test CSI plugin for CSI server
testing.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72727/#review221673
-----------------------------------------------------------
Patch looks great!
Reviews applied: [72732, 72716, 72690, 72733, 72734, 72753, 72754, 72726, 72727]
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. 18, 2020, 9:23 a.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72727/
> -----------------------------------------------------------
>
> (Updated Aug. 18, 2020, 9:23 a.m.)
>
>
> Review request for mesos, Andrei Budnik and Qian Zhang.
>
>
> Bugs: MESOS-10163
> https://issues.apache.org/jira/browse/MESOS-10163
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds additional configuration flags to the
> test CSI plugin which are necessary in order to test
> the agent's CSI server.
>
>
> Diffs
> -----
>
> src/examples/test_csi_plugin.cpp 214a3ee4481512b025ff054db6ffe15d476b0264
>
>
> Diff: https://reviews.apache.org/r/72727/diff/4/
>
>
> Testing
> -------
>
> These new flags are used in a subsequent test in this chain.
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 72727: Updated the test CSI plugin for CSI server
testing.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72727/
-----------------------------------------------------------
(Updated Aug. 29, 2020, 12:46 a.m.)
Review request for mesos, Andrei Budnik and Qian Zhang.
Bugs: MESOS-10163
https://issues.apache.org/jira/browse/MESOS-10163
Repository: mesos
Description
-------
This patch adds additional configuration flags to the
test CSI plugin which are necessary in order to test
the agent's CSI server.
Diffs (updated)
-----
src/examples/test_csi_plugin.cpp 214a3ee4481512b025ff054db6ffe15d476b0264
Diff: https://reviews.apache.org/r/72727/diff/5/
Changes: https://reviews.apache.org/r/72727/diff/4-5/
Testing
-------
These new flags are used in a subsequent test in this chain.
Thanks,
Greg Mann
Re: Review Request 72727: Updated the test CSI plugin for CSI server
testing.
Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72727/#review221702
-----------------------------------------------------------
Ship it!
Ship It!
- Qian Zhang
On Aug. 18, 2020, 9:23 a.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72727/
> -----------------------------------------------------------
>
> (Updated Aug. 18, 2020, 9:23 a.m.)
>
>
> Review request for mesos, Andrei Budnik and Qian Zhang.
>
>
> Bugs: MESOS-10163
> https://issues.apache.org/jira/browse/MESOS-10163
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds additional configuration flags to the
> test CSI plugin which are necessary in order to test
> the agent's CSI server.
>
>
> Diffs
> -----
>
> src/examples/test_csi_plugin.cpp 214a3ee4481512b025ff054db6ffe15d476b0264
>
>
> Diff: https://reviews.apache.org/r/72727/diff/4/
>
>
> Testing
> -------
>
> These new flags are used in a subsequent test in this chain.
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 72727: Updated the test CSI plugin for CSI server
testing.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72727/
-----------------------------------------------------------
(Updated Aug. 18, 2020, 1:23 a.m.)
Review request for mesos, Andrei Budnik and Qian Zhang.
Bugs: MESOS-10163
https://issues.apache.org/jira/browse/MESOS-10163
Repository: mesos
Description
-------
This patch adds additional configuration flags to the
test CSI plugin which are necessary in order to test
the agent's CSI server.
Diffs (updated)
-----
src/examples/test_csi_plugin.cpp 214a3ee4481512b025ff054db6ffe15d476b0264
Diff: https://reviews.apache.org/r/72727/diff/4/
Changes: https://reviews.apache.org/r/72727/diff/3-4/
Testing
-------
These new flags are used in a subsequent test in this chain.
Thanks,
Greg Mann
Re: Review Request 72727: Updated the test CSI plugin for CSI server
testing.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72727/
-----------------------------------------------------------
(Updated Aug. 11, 2020, 6:55 p.m.)
Review request for mesos, Andrei Budnik and Qian Zhang.
Bugs: MESOS-10163
https://issues.apache.org/jira/browse/MESOS-10163
Repository: mesos
Description
-------
This patch adds additional configuration flags to the
test CSI plugin which are necessary in order to test
the agent's CSI server.
Diffs (updated)
-----
src/examples/test_csi_plugin.cpp 214a3ee4481512b025ff054db6ffe15d476b0264
Diff: https://reviews.apache.org/r/72727/diff/3/
Changes: https://reviews.apache.org/r/72727/diff/2-3/
Testing
-------
These new flags are used in a subsequent test in this chain.
Thanks,
Greg Mann
Re: Review Request 72727: Updated the test CSI plugin for CSI server
testing.
Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72727/#review221544
-----------------------------------------------------------
src/examples/test_csi_plugin.cpp
Lines 1589-1590 (original), 1613-1614 (patched)
<https://reviews.apache.org/r/72727/#comment310599>
Here we assume the volume ID is the volume path, but that is not true when `--volume_id_path` is false. In that case, the volume ID and volume path will be different, so I think the mount operation here will fail.
- Qian Zhang
On Aug. 11, 2020, 11:07 a.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72727/
> -----------------------------------------------------------
>
> (Updated Aug. 11, 2020, 11:07 a.m.)
>
>
> Review request for mesos, Andrei Budnik and Qian Zhang.
>
>
> Bugs: MESOS-10163
> https://issues.apache.org/jira/browse/MESOS-10163
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds additional configuration flags to the
> test CSI plugin which are necessary in order to test
> the agent's CSI server.
>
>
> Diffs
> -----
>
> src/examples/test_csi_plugin.cpp 214a3ee4481512b025ff054db6ffe15d476b0264
>
>
> Diff: https://reviews.apache.org/r/72727/diff/2/
>
>
> Testing
> -------
>
> These new flags are used in a subsequent test in this chain.
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 72727: Updated the test CSI plugin for CSI server
testing.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72727/
-----------------------------------------------------------
(Updated Aug. 11, 2020, 3:07 a.m.)
Review request for mesos, Andrei Budnik and Qian Zhang.
Bugs: MESOS-10163
https://issues.apache.org/jira/browse/MESOS-10163
Repository: mesos
Description
-------
This patch adds additional configuration flags to the
test CSI plugin which are necessary in order to test
the agent's CSI server.
Diffs (updated)
-----
src/examples/test_csi_plugin.cpp 214a3ee4481512b025ff054db6ffe15d476b0264
Diff: https://reviews.apache.org/r/72727/diff/2/
Changes: https://reviews.apache.org/r/72727/diff/1-2/
Testing
-------
These new flags are used in a subsequent test in this chain.
Thanks,
Greg Mann
Re: Review Request 72727: Updated the test CSI plugin for CSI server
testing.
Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72727/#review221524
-----------------------------------------------------------
src/examples/test_csi_plugin.cpp
Lines 232-235 (original), 249-254 (patched)
<https://reviews.apache.org/r/72727/#comment310574>
It seems when `volumeIdPath` is set to `false` we will call `mkdir` to create directory for the volume under the current working directory? But I think we should create it under `TestCSIPlugin::workDir` just like what we did when `volumeIdPath` is set to `true`.
- Qian Zhang
On Aug. 1, 2020, 3:06 a.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72727/
> -----------------------------------------------------------
>
> (Updated Aug. 1, 2020, 3:06 a.m.)
>
>
> Review request for mesos, Andrei Budnik and Qian Zhang.
>
>
> Bugs: MESOS-10163
> https://issues.apache.org/jira/browse/MESOS-10163
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds additional configuration flags to the
> test CSI plugin which are necessary in order to test
> the agent's CSI server.
>
>
> Diffs
> -----
>
> src/examples/test_csi_plugin.cpp 6202173844a93b8d9642208bd86839b3cf56bce2
>
>
> Diff: https://reviews.apache.org/r/72727/diff/1/
>
>
> Testing
> -------
>
> These new flags are used in a subsequent test in this chain.
>
>
> Thanks,
>
> Greg Mann
>
>