Re: Review Request 69955: Added SLRP unit tests for destroying unpublished persistent volumes.

[Chun-Hung Hsiao]

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

(Updated May 2, 2019, 10:46 p.m.)


Review request for mesos and Benjamin Bannier.


Changes
---

Addressed Benjamin's comments.


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


Repository: mesos


Description
---

This patch adds 3 unit tests: `DestroyUnpublishedPersistentVolume`,
`DestroyUnpublishedPersistentVolumeWithRecovery`, and
`DestroyUnpublishedPersistentVolumeWithReboot` to test that the SLRP is
resilient to misbehaved CSI plugins that fail to publish volumes.


Diffs (updated)
-

  src/tests/storage_local_resource_provider_tests.cpp 
8bf4d2331b59770a7b7f3768499047bec2d67677 


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

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


Testing
---

make check

All tests passed 400 iterations (200 per parameter value) under stress.


Thanks,

Chun-Hung Hsiao

Re: Review Request 69955: Added SLRP unit tests for destroying unpublished persistent volumes.

[Chun-Hung Hsiao]

> On April 16, 2019, 1:18 p.m., Benjamin Bannier wrote:
> > src/tests/storage_local_resource_provider_tests.cpp
> > Lines 3328 (patched)
> > 
> >
> > If we need to await offers, lets also check their contents. It seems 
> > checking the operation status would already be enough.

This would eliminate the nondeterminism of the times of `resourceOffers` being 
called.


- Chun-Hung


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


On April 12, 2019, 10:02 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69955/
> ---
> 
> (Updated April 12, 2019, 10:02 p.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Bugs: MESOS-9565
> https://issues.apache.org/jira/browse/MESOS-9565
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds 3 unit tests: `DestroyUnpublishedPersistentVolume`,
> `DestroyUnpublishedPersistentVolumeWithRecovery`, and
> `DestroyUnpublishedPersistentVolumeWithReboot` to test that the SLRP is
> resilient to misbehaved CSI plugins that fail to publish volumes.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 8bf4d2331b59770a7b7f3768499047bec2d67677 
> 
> 
> Diff: https://reviews.apache.org/r/69955/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> All tests passed 400 iterations (200 per parameter value) under stress.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 69955: Added SLRP unit tests for destroying unpublished persistent volumes.

[Benjamin Bannier]

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


Fix it, then Ship it!





src/tests/storage_local_resource_provider_tests.cpp
Lines 454 (patched)


Let's add a log message here.



src/tests/storage_local_resource_provider_tests.cpp
Lines 3328 (patched)


If we need to await offers, lets also check their contents. It seems 
checking the operation status would already be enough.



src/tests/storage_local_resource_provider_tests.cpp
Lines 3542 (patched)


Ditto.



src/tests/storage_local_resource_provider_tests.cpp
Lines 3792 (patched)


Ditto.


- Benjamin Bannier


On April 13, 2019, 12:02 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69955/
> ---
> 
> (Updated April 13, 2019, 12:02 a.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Bugs: MESOS-9565
> https://issues.apache.org/jira/browse/MESOS-9565
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds 3 unit tests: `DestroyUnpublishedPersistentVolume`,
> `DestroyUnpublishedPersistentVolumeWithRecovery`, and
> `DestroyUnpublishedPersistentVolumeWithReboot` to test that the SLRP is
> resilient to misbehaved CSI plugins that fail to publish volumes.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 8bf4d2331b59770a7b7f3768499047bec2d67677 
> 
> 
> Diff: https://reviews.apache.org/r/69955/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> All tests passed 400 iterations (200 per parameter value) under stress.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 69955: Added SLRP unit tests for destroying unpublished persistent volumes.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 69955 was successfully built and tested.

Reviews applied: `['70431', '69954', '69955']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/3162/mesos-review-69955

- Mesos Reviewbot Windows


On April 12, 2019, 3:02 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69955/
> ---
> 
> (Updated April 12, 2019, 3:02 p.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Bugs: MESOS-9565
> https://issues.apache.org/jira/browse/MESOS-9565
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds 3 unit tests: `DestroyUnpublishedPersistentVolume`,
> `DestroyUnpublishedPersistentVolumeWithRecovery`, and
> `DestroyUnpublishedPersistentVolumeWithReboot` to test that the SLRP is
> resilient to misbehaved CSI plugins that fail to publish volumes.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 8bf4d2331b59770a7b7f3768499047bec2d67677 
> 
> 
> Diff: https://reviews.apache.org/r/69955/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> All tests passed 400 iterations (200 per parameter value) under stress.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 69955: Added SLRP unit tests for destroying unpublished persistent volumes.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [70468, 70431, 69954, 69955]

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

- Mesos Reviewbot


On April 12, 2019, 10:02 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69955/
> ---
> 
> (Updated April 12, 2019, 10:02 p.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Bugs: MESOS-9565
> https://issues.apache.org/jira/browse/MESOS-9565
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds 3 unit tests: `DestroyUnpublishedPersistentVolume`,
> `DestroyUnpublishedPersistentVolumeWithRecovery`, and
> `DestroyUnpublishedPersistentVolumeWithReboot` to test that the SLRP is
> resilient to misbehaved CSI plugins that fail to publish volumes.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 8bf4d2331b59770a7b7f3768499047bec2d67677 
> 
> 
> Diff: https://reviews.apache.org/r/69955/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> All tests passed 400 iterations (200 per parameter value) under stress.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 69955: Added SLRP unit tests for destroying unpublished persistent volumes.

[Chun-Hung Hsiao]

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

(Updated April 12, 2019, 10:02 p.m.)


Review request for mesos and Benjamin Bannier.


Changes
---

Rebased and parameterized the tests.


Summary (updated)
-

Added SLRP unit tests for destroying unpublished persistent volumes.


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


Repository: mesos


Description
---

This patch adds 3 unit tests: `DestroyUnpublishedPersistentVolume`,
`DestroyUnpublishedPersistentVolumeWithRecovery`, and
`DestroyUnpublishedPersistentVolumeWithReboot` to test that the SLRP is
resilient to misbehaved CSI plugins that fail to publish volumes.


Diffs (updated)
-

  src/tests/storage_local_resource_provider_tests.cpp 
8bf4d2331b59770a7b7f3768499047bec2d67677 


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

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


Testing
---

make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 69955: Added SLRP unit tests for destroying unpublished persistent volumes.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 69955 was successfully built and tested.

Reviews applied: `['69954', '69955']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2938/mesos-review-69955

- Mesos Reviewbot Windows


On March 4, 2019, 7:23 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69955/
> ---
> 
> (Updated March 4, 2019, 7:23 p.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Bugs: MESOS-9565
> https://issues.apache.org/jira/browse/MESOS-9565
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds 3 unit tests: `DestroyUnpublishedPersistentVolume`,
> `DestroyUnpublishedPersistentVolumeWithRecovery`, and
> `DestroyUnpublishedPersistentVolumeWithReboot` to test that the SLRP is
> resilient to misbehaved CSI plugins that fail to publish volumes.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> a661951a0a326cc342aa0c45dd0967692ae70941 
> 
> 
> Diff: https://reviews.apache.org/r/69955/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 69955: Added SLRP unit tests for destroying unpublished persistent volumes.

[Chun-Hung Hsiao]

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

(Updated March 4, 2019, 7:23 p.m.)


Review request for mesos and Benjamin Bannier.


Changes
---

Rebased.


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


Repository: mesos


Description
---

This patch adds 3 unit tests: `DestroyUnpublishedPersistentVolume`,
`DestroyUnpublishedPersistentVolumeWithRecovery`, and
`DestroyUnpublishedPersistentVolumeWithReboot` to test that the SLRP is
resilient to misbehaved CSI plugins that fail to publish volumes.


Diffs (updated)
-

  src/tests/storage_local_resource_provider_tests.cpp 
a661951a0a326cc342aa0c45dd0967692ae70941 


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

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


Testing
---

make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 69955: Added SLRP unit tests for destroying unpublished persistent volumes.

[Mesos Reviewbot]

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



Bad review!

Reviews applied: [69955, 69954, 69403, 69905, 69904, 69898, 69897, 69896, 
69895, 69894, 69893, 69892, 69866, 69858]

Error:
2019-03-02 13:44:57 URL:https://reviews.apache.org/r/69954/diff/raw/ 
[7553/7553] -> "69954.patch" [1]
error: patch failed: src/tests/storage_local_resource_provider_tests.cpp:361
error: src/tests/storage_local_resource_provider_tests.cpp: patch does not apply

- Mesos Reviewbot


On Feb. 11, 2019, 9:28 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69955/
> ---
> 
> (Updated Feb. 11, 2019, 9:28 p.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Bugs: MESOS-9565
> https://issues.apache.org/jira/browse/MESOS-9565
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds 3 unit tests: `DestroyUnpublishedPersistentVolume`,
> `DestroyUnpublishedPersistentVolumeWithRecovery`, and
> `DestroyUnpublishedPersistentVolumeWithReboot` to test that the SLRP is
> resilient to misbehaved CSI plugins that fail to publish volumes.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> fb001aa8d32d1a0a03014a35772fe10b65ce8d9a 
> 
> 
> Diff: https://reviews.apache.org/r/69955/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 69955: Added SLRP unit tests for destroying unpublished persistent volumes.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 69955 was successfully built and tested.

Reviews applied: `['69858', '69866', '69892', '69893', '69894', '69895', 
'69896', '69897', '69898', '69904', '69905', '69954', '69955']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2876/mesos-review-69955

- Mesos Reviewbot Windows


On Feb. 12, 2019, 5:28 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69955/
> ---
> 
> (Updated Feb. 12, 2019, 5:28 a.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Bugs: MESOS-9544
> https://issues.apache.org/jira/browse/MESOS-9544
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds 3 unit tests: `DestroyUnpublishedPersistentVolume`,
> `DestroyUnpublishedPersistentVolumeWithRecovery`, and
> `DestroyUnpublishedPersistentVolumeWithReboot` to test that the SLRP is
> resilient to misbehaved CSI plugins that fail to publish volumes.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> fb001aa8d32d1a0a03014a35772fe10b65ce8d9a 
> 
> 
> Diff: https://reviews.apache.org/r/69955/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Review Request 69955: Added SLRP unit tests for destroying unpublished persistent volumes.

[Chun-Hung Hsiao]

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

Review request for mesos and Benjamin Bannier.


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


Repository: mesos


Description
---

This patch adds 3 unit tests: `DestroyUnpublishedPersistentVolume`,
`DestroyUnpublishedPersistentVolumeWithRecovery`, and
`DestroyUnpublishedPersistentVolumeWithReboot` to test that the SLRP is
resilient to misbehaved CSI plugins that fail to publish volumes.


Diffs
-

  src/tests/storage_local_resource_provider_tests.cpp 
fb001aa8d32d1a0a03014a35772fe10b65ce8d9a 


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


Testing
---

make check


Thanks,

Chun-Hung Hsiao