You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Chun-Hung Hsiao <ch...@apache.org> on 2018/03/29 23:58:58 UTC

Review Request 66371: Temporarily disabled CSI proto compilation in CMake.

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

Review request for mesos, Benjamin Bannier, David Forsythe, and Jan Schlicht.


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


Repository: mesos


Description
-------

The `major` and `minor` macros defined on some systems conflict with
field names in the CSI v0.1.0 spec proto. Temporarily disable it in
CMake until CSI is bumped to 0.2.


Diffs
-----

  src/CMakeLists.txt fb9e9d4cfc4a62830fe3065a139ae14401c0e52e 


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


Testing
-------

`make check` with CMake


Thanks,

Chun-Hung Hsiao


Re: Review Request 66371: Temporarily disabled CSI proto compilation in CMake.

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



Patch looks great!

Reviews applied: [66371]

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

- Mesos Reviewbot


On March 29, 2018, 4:58 p.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66371/
> -----------------------------------------------------------
> 
> (Updated March 29, 2018, 4:58 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, David Forsythe, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8749
>     https://issues.apache.org/jira/browse/MESOS-8749
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The `major` and `minor` macros defined on some systems conflict with
> field names in the CSI v0.1.0 spec proto. Temporarily disable it in
> CMake until CSI is bumped to 0.2.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt fb9e9d4cfc4a62830fe3065a139ae14401c0e52e 
> 
> 
> Diff: https://reviews.apache.org/r/66371/diff/1/
> 
> 
> Testing
> -------
> 
> `make check` with CMake
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>


Re: Review Request 66371: Temporarily disabled CSI proto compilation in CMake.

Posted by Chun-Hung Hsiao <ch...@apache.org>.

> On March 30, 2018, 10:02 a.m., David Forsythe wrote:
> > This works for me on FreeBSD now, though I'm not sure if it would be better to gate this rather than removing it completely?

I remove it instead of gating it for the following reasons:

1. Current CMake doesn't include any build rules for the components (storage local resource provider) that depends on CSI, so it doesn't matter if we compile the proto file. The rules will be introduced in https://reviews.apache.org/r/66163/.
2. The compilation problem happens not only on FreeBSD, but also on platforms using GCC 7, so there is no simple gate for this.
3. This is just a temporary removal. https://reviews.apache.org/r/65594/ will introduce a internal proto message that uses a CSI message, so eventually we need CSI proto to be compiled on all supported platforms, after the CSI bundle is bumped to 0.2 (where there will be no name conflicts).

Therefore, it doesn't worth the time to design a gate that will be removed soon.


- Chun-Hung


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


On March 29, 2018, 11:58 p.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66371/
> -----------------------------------------------------------
> 
> (Updated March 29, 2018, 11:58 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, David Forsythe, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8749
>     https://issues.apache.org/jira/browse/MESOS-8749
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The `major` and `minor` macros defined on some systems conflict with
> field names in the CSI v0.1.0 spec proto. Temporarily disable it in
> CMake until CSI is bumped to 0.2.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt fb9e9d4cfc4a62830fe3065a139ae14401c0e52e 
> 
> 
> Diff: https://reviews.apache.org/r/66371/diff/1/
> 
> 
> Testing
> -------
> 
> `make check` with CMake
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>


Re: Review Request 66371: Temporarily disabled CSI proto compilation in CMake.

Posted by David Forsythe <df...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66371/#review200232
-----------------------------------------------------------


Ship it!




This works for me on FreeBSD now, though I'm not sure if it would be better to gate this rather than removing it completely?

- David Forsythe


On March 29, 2018, 11:58 p.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66371/
> -----------------------------------------------------------
> 
> (Updated March 29, 2018, 11:58 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, David Forsythe, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8749
>     https://issues.apache.org/jira/browse/MESOS-8749
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The `major` and `minor` macros defined on some systems conflict with
> field names in the CSI v0.1.0 spec proto. Temporarily disable it in
> CMake until CSI is bumped to 0.2.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt fb9e9d4cfc4a62830fe3065a139ae14401c0e52e 
> 
> 
> Diff: https://reviews.apache.org/r/66371/diff/1/
> 
> 
> Testing
> -------
> 
> `make check` with CMake
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>


Re: Review Request 66371: Temporarily disabled CSI proto compilation in CMake.

Posted by Chun-Hung Hsiao <ch...@apache.org>.

> On March 30, 2018, 8:34 a.m., David Forsythe wrote:
> > src/CMakeLists.txt
> > Line 35 (original)
> > <https://reviews.apache.org/r/66371/diff/1/?file=1990431#file1990431line35>
> >
> >     Should this line be removed?

This line is the same as Line 31. The review board is not clever enough to mark the intended one ;)


- Chun-Hung


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


On March 29, 2018, 11:58 p.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66371/
> -----------------------------------------------------------
> 
> (Updated March 29, 2018, 11:58 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, David Forsythe, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8749
>     https://issues.apache.org/jira/browse/MESOS-8749
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The `major` and `minor` macros defined on some systems conflict with
> field names in the CSI v0.1.0 spec proto. Temporarily disable it in
> CMake until CSI is bumped to 0.2.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt fb9e9d4cfc4a62830fe3065a139ae14401c0e52e 
> 
> 
> Diff: https://reviews.apache.org/r/66371/diff/1/
> 
> 
> Testing
> -------
> 
> `make check` with CMake
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>


Re: Review Request 66371: Temporarily disabled CSI proto compilation in CMake.

Posted by David Forsythe <df...@gmail.com>.

> On March 30, 2018, 8:34 a.m., David Forsythe wrote:
> > src/CMakeLists.txt
> > Line 35 (original)
> > <https://reviews.apache.org/r/66371/diff/1/?file=1990431#file1990431line35>
> >
> >     Should this line be removed?
> 
> Chun-Hung Hsiao wrote:
>     This line is the same as Line 31. The review board is not clever enough to mark the intended one ;)

Ah, of course.


- David


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


On March 29, 2018, 11:58 p.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66371/
> -----------------------------------------------------------
> 
> (Updated March 29, 2018, 11:58 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, David Forsythe, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8749
>     https://issues.apache.org/jira/browse/MESOS-8749
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The `major` and `minor` macros defined on some systems conflict with
> field names in the CSI v0.1.0 spec proto. Temporarily disable it in
> CMake until CSI is bumped to 0.2.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt fb9e9d4cfc4a62830fe3065a139ae14401c0e52e 
> 
> 
> Diff: https://reviews.apache.org/r/66371/diff/1/
> 
> 
> Testing
> -------
> 
> `make check` with CMake
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>


Re: Review Request 66371: Temporarily disabled CSI proto compilation in CMake.

Posted by David Forsythe <df...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66371/#review200228
-----------------------------------------------------------




src/CMakeLists.txt
Line 35 (original)
<https://reviews.apache.org/r/66371/#comment280886>

    Should this line be removed?


- David Forsythe


On March 29, 2018, 11:58 p.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66371/
> -----------------------------------------------------------
> 
> (Updated March 29, 2018, 11:58 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, David Forsythe, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8749
>     https://issues.apache.org/jira/browse/MESOS-8749
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The `major` and `minor` macros defined on some systems conflict with
> field names in the CSI v0.1.0 spec proto. Temporarily disable it in
> CMake until CSI is bumped to 0.2.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt fb9e9d4cfc4a62830fe3065a139ae14401c0e52e 
> 
> 
> Diff: https://reviews.apache.org/r/66371/diff/1/
> 
> 
> Testing
> -------
> 
> `make check` with CMake
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>


Re: Review Request 66371: Temporarily disabled CSI proto compilation in CMake.

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



FAIL: Some of the unit tests failed. Please check the relevant logs.

Reviews applied: `['66371']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66371

Relevant logs:

- [mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66371/logs/mesos-tests-stdout.log):

```
[       OK ] Endpoint/SlaveEndpointTest.NoAuthorizer/2 (110 ms)
[----------] 9 tests from Endpoint/SlaveEndpointTest (1075 ms total)

[----------] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest
[ RUN      ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0
[       OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0 (35 ms)
[ RUN      ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1
[       OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1 (46 ms)
[----------] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (83 ms total)

[----------] 1 test from IsolationFlag/CpuIsolatorTest
[ RUN      ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0
[       OK ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0 (906 ms)
[----------] 1 test from IsolationFlag/CpuIsolatorTest (934 ms total)

[----------] 1 test from IsolationFlag/MemoryIsolatorTest
[ RUN      ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0
[       OK ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (840 ms)
[----------] 1 test from IsolationFlag/MemoryIsolatorTest (873 ms total)

[----------] Global test environment tear-down
[==========] 949 tests from 94 test cases ran. (438255 ms total)
[  PASSED  ] 948 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] CommandExecutorCheckTest.CommandCheckTimeout

 1 FAILED TEST
  YOU HAVE 214 DISABLED TESTS

```

- [mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66371/logs/mesos-tests-stderr.log):

```
I0330 00:52:50.774760 10840 slave.cpp:3877] Shutting down framework 5237f813-6103-443f-9461-d2c6c4027152-0000
I0330 00:52:50.774760  7572 master.cpp:10446] UpdI0330 00:52:50.589793  2996 exec.cpp:162] Version: 1.6.0
I0330 00:52:50.617796  6444 exec.cpp:236] Executor registered on agent 5237f813-6103-443f-9461-d2c6c4027152-S0
I0330 00:52:50.621753 10452 executor.cpp:176] Received SUBSCRIBED event
I0330 00:52:50.627758 10452 executor.cpp:180] Subscribed executor on winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net
I0330 00:52:50.627758 10452 executor.cpp:176] Received LAUNCH event
I0330 00:52:50.632789 10452 executor.cpp:648] Starting task a1564c79-2acb-4157-9add-2e44a68909e3
I0330 00:52:50.714795 10452 executor.cpp:483] Running 'D:\DCOS\mesos\src\mesos-containerizer.exe launch <POSSIBLY-SENSITIVE-DATA>'
I0330 00:52:50.744762 10452 executor.cpp:661] Forked command at 7772
I0330 00:52:50.777762  7712 exec.cpp:445] Executor asked to shutdown
I0330 00:52:50.778776  2432 executor.cpp:176] Received SHUTDOWN event
I0330 00:52:50.778776  2432 executor.cpp:758] Shutting down
I0330 00:52:50.778776  2432 executor.cpp:868] Sending SIGTERM to process tree at pid 7ating the state of task a1564c79-2acb-4157-9add-2e44a68909e3 of framework 5237f813-6103-443f-9461-d2c6c4027152-0000 (latest state: TASK_KILLED, status update state: TASK_KILLED)
I0330 00:52:50.774760 10840 slave.cpp:6570] Shutting down executor 'a1564c79-2acb-4157-9add-2e44a68909e3' of framework 5237f813-6103-443f-9461-d2c6c4027152-0000 at executor(1)@10.3.1.8:54402
I0330 00:52:50.776940 10840 slave.cpp:923] Agent terminating
W0330 00:52:50.777762 10840 slave.cpp:3873] Ignoring shutdown framework 5237f813-6103-443f-9461-d2c6c4027152-0000 because it is terminating
I0330 00:52:50.779772  7572 master.cpp:10545] Removing task a1564c79-2acb-4157-9add-2e44a68909e3 with resources cpus(allocated: *):4; mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: *):[31000-32000] of framework 5237f813-6103-443f-9461-d2c6c4027152-0000 on agent 5237f813-6103-443f-9461-d2c6c4027152-S0 at slave(418)@10.3.1.8:54381 (winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0330 00:52:50.780773  7956 containerizer.cpp:2338] Destroying container 3cef3400-1646-4410-9f72-bb6f9e484d69 in RUNNING state
I0330 00:52:50.781764  7956 containerizer.cpp:2952] Transitioning the state of container 3cef3400-1646-4410-9f72-bb6f9e484d69 from RUNNING to DESTROYING
I0330 00:52:50.782796  7956 launcher.cpp:156] Asked to destroy container 3cef3400-1646-4410-9f72-bb6f9e484d69
I0330 00:52:50.783773  7572 master.cpp:1295] Agent 5237f813-6103-443f-9461-d2c6c4027152-S0 at slave(418)@10.3.1.8:54381 (winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net) disconnected
I0330 00:52:50.783773  7572 master.cpp:3283] Disconnecting agent 5237f813-6103-443f-9461-d2c6c4027152-S0 at slave(418)@10.3.1.8:54381 (winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0330 00:52:50.783773  7572 master.cpp:3302] Deactivating agent 5237f813-6103-443f-9461-d2c6c4027152-S0 at slave(418)@10.3.1.8:54381 (winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0330 00:52:50.783773  8880 hierarchical.cpp:344] Removed framework 5237f813-6103-443f-9461-d2c6c4027152-0000
I0330 00:52:50.784775  8880 hierarchical.cpp:766] Agent 5237f813-6103-443f-9461-d2c6c4027152-S0 deactivated
I0330 00:52:50.869778  7276 containerizer.cpp:2791] Container 3cef3400-1646-4410-9f72-bb6f9e484d69 has exited
I0330 00:52:50.906800  3332 master.cpp:1137] Master terminating
I0330 00:52:50.908800  9292 hierarchical.cpp:609] Removed agent 5237f813-6103-443f-9461-d2c6c4027152-S0
I0330 00:52:51.435806  9480 process.cpp:929] Stopped the socket accept loop
```

- Mesos Reviewbot Windows


On March 29, 2018, 11:58 p.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66371/
> -----------------------------------------------------------
> 
> (Updated March 29, 2018, 11:58 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, David Forsythe, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8749
>     https://issues.apache.org/jira/browse/MESOS-8749
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The `major` and `minor` macros defined on some systems conflict with
> field names in the CSI v0.1.0 spec proto. Temporarily disable it in
> CMake until CSI is bumped to 0.2.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt fb9e9d4cfc4a62830fe3065a139ae14401c0e52e 
> 
> 
> Diff: https://reviews.apache.org/r/66371/diff/1/
> 
> 
> Testing
> -------
> 
> `make check` with CMake
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>