Re: Review Request 72727: Updated the test CSI plugin for CSI server testing.

[Greg Mann]

---
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.

[Qian Zhang]

---
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.

[Mesos Reviewbot]

---
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.

[Greg Mann]

---
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.

[Greg Mann]

> On Aug. 12, 2020, 3:17 a.m., Qian Zhang wrote:
> > src/examples/test_csi_plugin.cpp
> > Lines 174-177 (patched)
> > 
> >
> > 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.

[Qian Zhang]

---
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)


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)


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.

[Greg Mann]

---
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.

[Qian Zhang]

---
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)


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.

[Greg Mann]

---
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.

[Qian Zhang]

---
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)


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
> 
>

Review Request 72727: Updated the test CSI plugin for CSI server testing.

[Greg Mann]

---
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