date:20200813

Re: Review Request 72754: Enabled the `volume/csi` isolator in `MesosContainerizer`.

2020-08-13 Thread Greg Mann


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


Fix it, then Ship it!





src/slave/containerizer/containerizer.cpp
Lines 224 (patched)


Need include for 'slave/csi_server.hpp'.



src/slave/containerizer/mesos/containerizer.cpp
Lines 185 (patched)


Need include for 'slave/csi_server.hpp'.


- Greg Mann


On Aug. 13, 2020, 6:14 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72754/
> ---
> 
> (Updated Aug. 13, 2020, 6:14 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10156
> https://issues.apache.org/jira/browse/MESOS-10156
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled the `volume/csi` isolator in `MesosContainerizer`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/containerizer.hpp 
> 2b3c4c0363a6362c62695f087ea25248657d99df 
>   src/slave/containerizer/containerizer.cpp 
> 9e44e5e0a41e48121f147778295a07b10b03bcf2 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 56e4c49733ac4e236b81ebf9e3238bf8a189c9f2 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 3c1840cae4befe09581ea4efae855d552bd19b05 
> 
> 
> Diff: https://reviews.apache.org/r/72754/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 72753: Implemented the `recover` method of `volume/csi` isolator.

2020-08-13 Thread Greg Mann


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


Ship it!




- Greg Mann


On Aug. 11, 2020, 8:31 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72753/
> ---
> 
> (Updated Aug. 11, 2020, 8:31 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10155
> https://issues.apache.org/jira/browse/MESOS-10155
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented the `recover` method of `volume/csi` isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/volume/csi/isolator.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp 
> PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/72753/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 72759: Improved CSI service manager to set node ID for managed CSI plugins.

2020-08-13 Thread Qian Zhang



> On Aug. 14, 2020, 6:21 a.m., Greg Mann wrote:
> > src/csi/service_manager.cpp
> > Lines 740 (patched)
> > 
> >
> > Where does this env var name come from, 'MESOS_NODE_ID'?

Orginially I wanted to just name it `NODE_ID`, however I think it is too 
generic and may be conflict with other env var used by the CSI plugin. So I 
changed to add a `MESOS_` prefix just like other env vars that Mesos sets for 
containers, e.g. `MESOS_FRAMEWORK_ID`, `MESOS_SLAVE_ID`, 
`MESOS_AGENT_ENDPOINT`. Or maybe we should name it `MESOS_AGENT_IP`?


- Qian


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


On Aug. 12, 2020, 7:47 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72759/
> ---
> 
> (Updated Aug. 12, 2020, 7:47 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10175
> https://issues.apache.org/jira/browse/MESOS-10175
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improved CSI service manager to set node ID for managed CSI plugins.
> 
> 
> Diffs
> -
> 
>   src/csi/service_manager.cpp 7a8d8e5dc3c3bae5251284652d73b33ca554f783 
> 
> 
> Diff: https://reviews.apache.org/r/72759/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 72734: Implemented the `cleanup` method of `volume/csi` isolator.

2020-08-13 Thread Greg Mann


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


Ship it!




Ship It!

- Greg Mann


On Aug. 8, 2020, 3:18 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72734/
> ---
> 
> (Updated Aug. 8, 2020, 3:18 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10154
> https://issues.apache.org/jira/browse/MESOS-10154
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented the `cleanup` method of `volume/csi` isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/volume/csi/isolator.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp 
> PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/72734/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 72734: Implemented the `cleanup` method of `volume/csi` isolator.

2020-08-13 Thread Greg Mann



> On Aug. 6, 2020, 10:04 p.m., Greg Mann wrote:
> > src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp
> > Lines 389-391 (patched)
> > 
> >
> > I'm wondering if there's an issue that if we return a failure here, the 
> > state on disk (the `volumes` file for this container) and the state in 
> > memory (the state stored in `infos`) will be inconsistent.
> > 
> > If the agent were to restart, the CSI volume isolator could clean up 
> > that file if it refers to an unknown container. In that case, we will have 
> > left a volume published on this node without a record of it, but maybe 
> > that's OK?
> 
> Qian Zhang wrote:
> In that case, when agent is restarted, containerizer will try to destroy 
> the container again which in turn will call `cleanup` to try to unpublish the 
> volume again.
> 
> Before the agent is restarted, yes we will have left a volume published 
> on the node without a record of it in memory (since we have already done 
> `infos.erase(containerId)` before making the unpublish call), but I think it 
> should OK. Do you see any problems with it?

Yea I think this case will be OK without extra handling for the MVP.


- Greg


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


On Aug. 8, 2020, 3:18 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72734/
> ---
> 
> (Updated Aug. 8, 2020, 3:18 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10154
> https://issues.apache.org/jira/browse/MESOS-10154
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented the `cleanup` method of `volume/csi` isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/volume/csi/isolator.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp 
> PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/72734/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 72759: Improved CSI service manager to set node ID for managed CSI plugins.

2020-08-13 Thread Greg Mann


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




src/csi/service_manager.cpp
Lines 740 (patched)


Where does this env var name come from, 'MESOS_NODE_ID'?


- Greg Mann


On Aug. 12, 2020, 11:47 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72759/
> ---
> 
> (Updated Aug. 12, 2020, 11:47 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10175
> https://issues.apache.org/jira/browse/MESOS-10175
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improved CSI service manager to set node ID for managed CSI plugins.
> 
> 
> Diffs
> -
> 
>   src/csi/service_manager.cpp 7a8d8e5dc3c3bae5251284652d73b33ca554f783 
> 
> 
> Diff: https://reviews.apache.org/r/72759/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 72770: Updated the help message of the agent flag `--csi_plugin_config_dir`.

2020-08-13 Thread Greg Mann


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


Ship it!




Ship It!

- Greg Mann


On Aug. 13, 2020, 8:46 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72770/
> ---
> 
> (Updated Aug. 13, 2020, 8:46 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10151
> https://issues.apache.org/jira/browse/MESOS-10151
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is to make the help message of the agent flag `--csi_plugin_config_dir`
> aligned with the latest protobuf message `CSIPluginInfo`.
> 
> 
> Diffs
> -
> 
>   docs/configuration/agent.md e8608d977652baf02b4474bb48d3f4598cfa5f3f 
>   src/slave/flags.cpp 02a5568f29927e767029a53f05f3b3e97ed0d1f8 
> 
> 
> Diff: https://reviews.apache.org/r/72770/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 72741: Implemented offer constraints filter with Exists/NotExists predicates.

2020-08-13 Thread Benjamin Mahler

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

src/master/offer_constraints_filter.cpp
Lines 43-56 (patched)

We should check that they aren't both set?

src/master/offer_constraints_filter.cpp
Lines 44-51 (patched)

Can we use a switch here per our approach to protobuf enum handling? That 
gives us the benefit of a compiler error when we miss a case (now or later)

src/master/offer_constraints_filter.cpp
Lines 59-60 (patched)

Use single quotes here, and ideally a clear field path:

Exactly one of 'AttributeConstraint::Selector::name' of 
'AttributeConstraint::Selector::pseudoattribute_type' must be set

src/master/offer_constraints_filter.cpp
Lines 64 (patched)

This is a little confusing to me since we already have a predicate type 
from the protobuf and this is essentially the same thing but in a more usable 
format for this code?

I wonder if there's a way to accomplish what we want without the extra 
predicate type. Since the evaluator holds one of these predicates, maybe the 
evaluator can be the one to hold any extra predicate logic / information needed 
rather than having a duplicate predicate type.

src/master/offer_constraints_filter.cpp
Lines 121 (patched)

braces on the next line (looks like some other classes functions need to be 
updated in this file)

src/master/offer_constraints_filter.cpp
Lines 134-143 (patched)

I think we allow multiple entries for the same attribute key, on the agent 
side? Not sure if anyone uses that.

src/master/offer_constraints_filter.cpp
Lines 147 (patched)

switch (

src/master/offer_constraints_filter.cpp
Lines 181 (patched)

Ditto, don't think we need to bother with the std move here?

src/master/offer_constraints_filter.cpp
Lines 192-195 (patched)

I think we generally put the public interface first, followed by an 
explicit private section?

Ditto in the other case(s) above.

src/master/offer_constraints_filter.cpp
Lines 237 (patched)

hm.. maybe do the role insertion above this loop and have a reference to 
the vector? Seems cleaner and cheaper?

```
foreachpair (
const string& role,
OfferConstraints::RoleConstrints& roleConstraints,
*constraints.mutable_role_constraints()) {
  vector& groups = expressions[role];

  for (OfferConstraints::RoleConstraints::Group& group :
   *roleConstraints.mutable_groups()) {
...

groups.emplace_back(std::move(*evaluator));
  }
}
```

src/master/offer_constraints_filter.cpp
Lines 244 (patched)

Don't think we need to worry about the cost benefit of std moving the error 
message to bother with the extra code? It will only be done once, and only in 
the error case.

- Benjamin Mahler

On Aug. 6, 2020, 4:24 p.m., Andrei Sekretenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72741/
> ---
> 
> (Updated Aug. 6, 2020, 4:24 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-10171
> https://issues.apache.org/jira/browse/MESOS-10171
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch implements a subclass of AgentResourcesFilter that suppoorts
> the Exists/NotExists offer constraints.
> 
> More constraints will be added to this filter in further patches.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 4e15e3d99aa2cce2403fe07e762fef2fb4a27dea 
>   src/Makefile.am 447db323875e4cad46000977f4a61600baff8f89 
>   src/master/offer_constraints_filter.hpp PRE-CREATION 
>   src/master/offer_constraints_filter.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/72741/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>

Re: Review Request 72742: Added basic tests for the offer constraints filter.

2020-08-13 Thread Mesos Reviewbot


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



Bad patch!

Reviews applied: [72771, 72738, 72739, 72740, 72741, 72742]

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_72742"]

Error:
..
tobuf-3.5.0/src -I../3rdparty/rapidjson-1.1.0/include 
-I../../../3rdparty/stout/include -I../3rdparty/zookeeper-3.4.8/src/c/include 
-I../3rdparty/zookeeper-3.4.8/src/c/generated -I/usr/include/subversion-1 
-I/usr/include/apr-1 -I/usr/include/apr-1.0  -pthread -Wall 
-Wsign-compare -Wformat-security -fstack-protector-strong -fPIC -fPIE -g1 -O0 
-Wno-unused-local-typedefs -std=c++11 -c -o 
master/contender/libmesos_no_3rdparty_la-standalone.lo `test -f 
'master/contender/standalone.cpp' || echo 
'../../../src/'`master/contender/standalone.cpp
libtool: compile:  g++ -DPACKAGE_NAME=\"mesos\" -DPACKAGE_TARNAME=\"mesos\" 
-DPACKAGE_VERSION=\"1.11.0\" "-DPACKAGE_STRING=\"mesos 1.11.0\"" 
-DPACKAGE_BUGREPORT=\"\" -DPACKAGE_URL=\"\" -DPACKAGE=\"mesos\" 
-DVERSION=\"1.11.0\" -DSTDC_HEADERS=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_H=1 
-DHAVE_STDLIB_H=1 -DHAVE_STRING_H=1 -DHAVE_MEMORY_H=1 -DHAVE_STRINGS_H=1 
-DHAVE_INTTYPES_H=1 -DHAVE_STDINT_H=1 -DHAVE_UNISTD_H=1 -DHAVE_DLFCN_H=1 
-DLT_OBJDIR=\".libs/\" -DHAVE_CXX11=1 -DHAVE_PTHREAD_PRIO_INHERIT=1 
-DHAVE_PTHREAD=1 -DHAVE_OPENSSL_SSL_H=1 -DHAVE_FTS_H=1 -DHAVE_APR_POOLS_H=1 
-DHAVE_LIBAPR_1=1 -DHAVE_LIBCURL=1 -DMESOS_HAS_JAVA=1 -DENABLE_NVML=1 
-DHAVE_LIBSASL2=1 -DHAVE_SVN_VERSION_H=1 -DHAVE_LIBSVN_SUBR_1=1 
-DHAVE_SVN_DELTA_H=1 -DHAVE_LIBSVN_DELTA_1=1 -DHAVE_ZLIB_H=1 -DHAVE_LIBZ=1 
-DHAVE_PYTHON=\"2.7\" -DMESOS_HAS_PYTHON=1 -I. -I../../../src -Werror 
-DLIBDIR=\"/tmp/SRC/build/mesos-1.11.0/_inst/lib\" 
-DPKGLIBEXECDIR=\"/tmp/SRC/build/mesos-1.11.0/_inst/libexec/mesos\" 
-DPKGDATADIR=\"/tmp/SRC/bu
 ild/mesos-1.11.0/_inst/share/mesos\" 
-DPKGMODULEDIR=\"/tmp/SRC/build/mesos-1.11.0/_inst/lib/mesos/modules\" 
-I../../../include -I../include -I../include/mesos -D__STDC_FORMAT_MACROS 
-I../3rdparty/boost-1.65.0 -I../3rdparty/concurrentqueue-7b69a8f 
-I../3rdparty/elfio-3.2 -I../3rdparty/glog-0.4.0/src 
-I../3rdparty/grpc-1.10.0/include -I../3rdparty/leveldb-1.19/include 
-I../3rdparty/libarchive-3.3.2/libarchive/ 
-I../../../3rdparty/libprocess/include -I../3rdparty/nvml-352.79 
-I../3rdparty/picojson-1.3.0 -I../3rdparty/protobuf-3.5.0/src 
-I../3rdparty/rapidjson-1.1.0/include -I../../../3rdparty/stout/include 
-I../3rdparty/zookeeper-3.4.8/src/c/include 
-I../3rdparty/zookeeper-3.4.8/src/c/generated -I/usr/include/subversion-1 
-I/usr/include/apr-1 -I/usr/include/apr-1.0 -pthread -Wall -Wsign-compare 
-Wformat-security -fstack-protector-strong -fPIC -g1 -O0 
-Wno-unused-local-typedefs -std=c++11 -c ../../../src/master/constants.cpp  
-fPIC -DPIC -o master/.libs/libmesos_no_3rdparty_la-constants
 .o
libtool: compile:  g++ -DPACKAGE_NAME=\"mesos\" -DPACKAGE_TARNAME=\"mesos\" 
-DPACKAGE_VERSION=\"1.11.0\" "-DPACKAGE_STRING=\"mesos 1.11.0\"" 
-DPACKAGE_BUGREPORT=\"\" -DPACKAGE_URL=\"\" -DPACKAGE=\"mesos\" 
-DVERSION=\"1.11.0\" -DSTDC_HEADERS=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_H=1 
-DHAVE_STDLIB_H=1 -DHAVE_STRING_H=1 -DHAVE_MEMORY_H=1 -DHAVE_STRINGS_H=1 
-DHAVE_INTTYPES_H=1 -DHAVE_STDINT_H=1 -DHAVE_UNISTD_H=1 -DHAVE_DLFCN_H=1 
-DLT_OBJDIR=\".libs/\" -DHAVE_CXX11=1 -DHAVE_PTHREAD_PRIO_INHERIT=1 
-DHAVE_PTHREAD=1 -DHAVE_OPENSSL_SSL_H=1 -DHAVE_FTS_H=1 -DHAVE_APR_POOLS_H=1 
-DHAVE_LIBAPR_1=1 -DHAVE_LIBCURL=1 -DMESOS_HAS_JAVA=1 -DENABLE_NVML=1 
-DHAVE_LIBSASL2=1 -DHAVE_SVN_VERSION_H=1 -DHAVE_LIBSVN_SUBR_1=1 
-DHAVE_SVN_DELTA_H=1 -DHAVE_LIBSVN_DELTA_1=1 -DHAVE_ZLIB_H=1 -DHAVE_LIBZ=1 
-DHAVE_PYTHON=\"2.7\" -DMESOS_HAS_PYTHON=1 -I. -I../../../src -Werror 
-DLIBDIR=\"/tmp/SRC/build/mesos-1.11.0/_inst/lib\" 
-DPKGLIBEXECDIR=\"/tmp/SRC/build/mesos-1.11.0/_inst/libexec/mesos\" 
-DPKGDATADIR=\"/tmp/SRC/bu
 ild/mesos-1.11.0/_inst/share/mesos\" 
-DPKGMODULEDIR=\"/tmp/SRC/build/mesos-1.11.0/_inst/lib/mesos/modules\" 
-I../../../include -I../include -I../include/mesos -D__STDC_FORMAT_MACROS 
-I../3rdparty/boost-1.65.0 -I../3rdparty/concurrentqueue-7b69a8f 
-I../3rdparty/elfio-3.2 -I../3rdparty/glog-0.4.0/src 
-I../3rdparty/grpc-1.10.0/include -I../3rdparty/leveldb-1.19/include 
-I../3rdparty/libarchive-3.3.2/libarchive/ 
-I../../../3rdparty/libprocess/include -I../3rdparty/nvml-352.79 
-I../3rdparty/picojson-1.3.0 -I../3rdparty/protobuf-3.5.0/src 
-I../3rdparty/rapidjson-1.1.0/include -I../../../3rdparty/stout/include 
-I../3rdparty/zookeeper-3.4.8/src/c/include 
-I../3rdparty/zookeeper-3.4.8/src/c/generated -I/usr/include/subversion-1 
-I/usr/include/apr-1

Re: Review Request 72738: Added protobuf messages for constraints-based offer filtering.

2020-08-13 Thread Andrei Sekretenko



> On Aug. 11, 2020, 7:16 p.m., Benjamin Mahler wrote:
> > src/internal/devolve.cpp
> > Lines 248-249 (original), 248-249 (patched)
> > 
> >
> > Just to double check, I assume you found this by searching for 
> > suppressed_roles or some other piece of the message?

No, actually it was a failure of the new test of UpdateFramework with 
constraints.

Double-checked by serached once again and realized that we also have unused 
`evolve` with no test coverage and a `TODO` for considering removal.
Removing that in the preceding patch.


- Andrei


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


On Aug. 13, 2020, 6:54 p.m., Andrei Sekretenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72738/
> ---
> 
> (Updated Aug. 13, 2020, 6:54 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-10171
> https://issues.apache.org/jira/browse/MESOS-10171
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds framework's offer constraints  into `Subscribe` and
> `UpdateFramework` calls, which is a prerequisite to implementing
> constraints-based offer filtering (see MESOS-10161).
> 
> 
> Diffs
> -
> 
>   include/mesos/scheduler/scheduler.proto 
> 6e1639a9baf017fa87b274daeedc821389216ddc 
>   include/mesos/v1/scheduler/scheduler.proto 
> eb5fdeb984b28403bd8281742bd0a5f2053863e3 
>   src/internal/devolve.cpp 4527c522473b74622055e1765740e3706b95afdb 
> 
> 
> Diff: https://reviews.apache.org/r/72738/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>

Re: Review Request 72738: Added protobuf messages for constraints-based offer filtering.

2020-08-13 Thread Andrei Sekretenko


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

(Updated Aug. 13, 2020, 6:54 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Accommodated the suggested explanation if the constraints structure (with 
slight adjustments).

Checked for suppressed_roles once again and found `evolve()`; inserted a patch 
before this one to remove that.


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


Repository: mesos


Description
---

This patch adds framework's offer constraints  into `Subscribe` and
`UpdateFramework` calls, which is a prerequisite to implementing
constraints-based offer filtering (see MESOS-10161).


Diffs (updated)
-

  include/mesos/scheduler/scheduler.proto 
6e1639a9baf017fa87b274daeedc821389216ddc 
  include/mesos/v1/scheduler/scheduler.proto 
eb5fdeb984b28403bd8281742bd0a5f2053863e3 
  src/internal/devolve.cpp 4527c522473b74622055e1765740e3706b95afdb 


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

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


Testing
---


Thanks,

Andrei Sekretenko

Review Request 72771: Removed `evolve(const scheduler::Call&)`.

2020-08-13 Thread Andrei Sekretenko


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

Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

This patch removes a non-trivial `evolve()` overload that is not used
anywhere in Mesos code and is not covered by tests.


Diffs
-

  src/internal/evolve.hpp e4e3ab41a90c420842103b27a8180cfc5835986e 
  src/internal/evolve.cpp c5e41516e5b9b08acf0641c89c3386e050783bf0 


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


Testing
---

`support/mesos-gtest-runner.py src/mesos-tests`


Thanks,

Andrei Sekretenko

Re: Review Request 72770: Updated the help message of the agent flag `--csi_plugin_config_dir`.

2020-08-13 Thread Mesos Reviewbot


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



Patch looks great!

Reviews applied: [72770]

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. 13, 2020, 8:46 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72770/
> ---
> 
> (Updated Aug. 13, 2020, 8:46 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10151
> https://issues.apache.org/jira/browse/MESOS-10151
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is to make the help message of the agent flag `--csi_plugin_config_dir`
> aligned with the latest protobuf message `CSIPluginInfo`.
> 
> 
> Diffs
> -
> 
>   docs/configuration/agent.md e8608d977652baf02b4474bb48d3f4598cfa5f3f 
>   src/slave/flags.cpp 02a5568f29927e767029a53f05f3b3e97ed0d1f8 
> 
> 
> Diff: https://reviews.apache.org/r/72770/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 72728: Added a unit test for the CSI server.

2020-08-13 Thread Mesos Reviewbot


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



Patch looks great!

Reviews applied: [72732, 72716, 72690, 72733, 72734, 72753, 72754, 72726, 
72727, 72761, 72728]

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. 13, 2020, 2:38 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72728/
> ---
> 
> (Updated Aug. 13, 2020, 2:38 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
> ---
> 
> The new test 'CSIServerTest.ROOT_LoadPlugin' verifies that
> the CSI server can load a plugin and execute publish and
> unpublish calls against it.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 49dab4b6488b75d32e6ee9d54d68afe36549c353 
>   src/tests/csi_server_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/72728/diff/4/
> 
> 
> Testing
> ---
> 
> `sudo make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Review Request 72770: Updated the help message of the agent flag `--csi_plugin_config_dir`.

2020-08-13 Thread Qian Zhang


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

Review request for mesos, Andrei Budnik and Greg Mann.


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


Repository: mesos


Description
---

This is to make the help message of the agent flag `--csi_plugin_config_dir`
aligned with the latest protobuf message `CSIPluginInfo`.


Diffs
-

  docs/configuration/agent.md e8608d977652baf02b4474bb48d3f4598cfa5f3f 
  src/slave/flags.cpp 02a5568f29927e767029a53f05f3b3e97ed0d1f8 


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


Testing
---


Thanks,

Qian Zhang

Re: Review Request 72761: Added the CSI server to the Mesos agent.

2020-08-13 Thread Qian Zhang


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




src/slave/main.cpp
Lines 570 (patched)


This line cannot compile:
```
../../src/slave/main.cpp: In function ‘int main(int, char**)’:
../../src/slave/main.cpp:570:9: error: ‘flags’ is not a member of 
‘process::network::openssl’
 if (process::network::openssl::flags().enabled) {
```

I used the following configuration parameters, anything missed?
```
../configure --disable-libtool-wrappers --disable-python --disable-java 
--enable-ssl --enable-libevent --enable-debug
```


- Qian Zhang


On Aug. 13, 2020, 3:19 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72761/
> ---
> 
> (Updated Aug. 13, 2020, 3:19 a.m.)
> 
> 
> Review request for mesos and Qian Zhang.
> 
> 
> Bugs: MESOS-10163
> https://issues.apache.org/jira/browse/MESOS-10163
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a CSI server to the Mesos agent in both
> the agent binary and in tests.
> 
> 
> Diffs
> -
> 
>   src/local/local.cpp 895057027165609c792089cd336988b65a3b305d 
>   src/slave/main.cpp 0aa2cc9d254d0b74485a0584b9f55c2266f29367 
>   src/slave/slave.hpp 2cf45c616d5c50508bc5cd77096b0fef86542b1d 
>   src/slave/slave.cpp c828d99ca9909b904767cc9ea5671b462714f547 
>   src/tests/cluster.hpp 415a60f5fd4881bca329ffd718f58d676653a17d 
>   src/tests/cluster.cpp ab4cea49eb3e8cf18d320fbb30beaa3c454a08b0 
>   src/tests/mesos.hpp 0ad0999fba185539c90b053bda43be001da26253 
>   src/tests/mesos.cpp d6933a60c55802e6f9e89242608fbfad7631faed 
>   src/tests/mock_slave.hpp 58daefa4459eaf15c2394f14db9f94527984b2b8 
>   src/tests/mock_slave.cpp fa2a0f5c8dadeb8aea87815741bc4fe071d5fbe0 
> 
> 
> Diff: https://reviews.apache.org/r/72761/diff/1/
> 
> 
> Testing
> ---
> 
> Testing details at the end of this chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 72760: Passed the CSI server into the Mesos containerizer.

2020-08-13 Thread Greg Mann


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

(Updated Aug. 13, 2020, 2:38 p.m.)


Review request for mesos and Qian Zhang.


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


Repository: mesos


Description
---

Passed the CSI server into the Mesos containerizer.


Diffs
-

  src/slave/containerizer/containerizer.cpp 
9e44e5e0a41e48121f147778295a07b10b03bcf2 


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


Testing
---


Thanks,

Greg Mann

Re: Review Request 72728: Added a unit test for the CSI server.

2020-08-13 Thread Greg Mann


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

(Updated Aug. 13, 2020, 2:38 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
---

The new test 'CSIServerTest.ROOT_LoadPlugin' verifies that
the CSI server can load a plugin and execute publish and
unpublish calls against it.


Diffs
-

  src/Makefile.am 49dab4b6488b75d32e6ee9d54d68afe36549c353 
  src/tests/csi_server_tests.cpp PRE-CREATION 


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


Testing
---

`sudo make check`


Thanks,

Greg Mann

Re: Review Request 72728: Added a unit test for the CSI server.

2020-08-13 Thread Greg Mann


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

(Updated Aug. 13, 2020, 2:36 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
---

The new test 'CSIServerTest.ROOT_LoadPlugin' verifies that
the CSI server can load a plugin and execute publish and
unpublish calls against it.


Diffs
-

  src/Makefile.am 49dab4b6488b75d32e6ee9d54d68afe36549c353 
  src/tests/csi_server_tests.cpp PRE-CREATION 


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


Testing
---

`sudo make check`


Thanks,

Greg Mann

Re: Review Request 72754: Enabled the `volume/csi` isolator in `MesosContainerizer`.

2020-08-13 Thread Qian Zhang


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

(Updated Aug. 13, 2020, 2:14 p.m.)


Review request for mesos, Andrei Budnik and Greg Mann.


Changes
---

Minor fix.


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


Repository: mesos


Description
---

Enabled the `volume/csi` isolator in `MesosContainerizer`.


Diffs (updated)
-

  src/slave/containerizer/containerizer.hpp 
2b3c4c0363a6362c62695f087ea25248657d99df 
  src/slave/containerizer/containerizer.cpp 
9e44e5e0a41e48121f147778295a07b10b03bcf2 
  src/slave/containerizer/mesos/containerizer.hpp 
56e4c49733ac4e236b81ebf9e3238bf8a189c9f2 
  src/slave/containerizer/mesos/containerizer.cpp 
3c1840cae4befe09581ea4efae855d552bd19b05 


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

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


Testing
---


Thanks,

Qian Zhang

Re: Review Request 72754: Enabled the `volume/csi` isolator in `MesosContainerizer`.

Re: Review Request 72753: Implemented the `recover` method of `volume/csi` isolator.

Re: Review Request 72759: Improved CSI service manager to set node ID for managed CSI plugins.

Re: Review Request 72734: Implemented the `cleanup` method of `volume/csi` isolator.

Re: Review Request 72734: Implemented the `cleanup` method of `volume/csi` isolator.

Re: Review Request 72759: Improved CSI service manager to set node ID for managed CSI plugins.

Re: Review Request 72770: Updated the help message of the agent flag `--csi_plugin_config_dir`.

Re: Review Request 72741: Implemented offer constraints filter with Exists/NotExists predicates.

Re: Review Request 72742: Added basic tests for the offer constraints filter.

Re: Review Request 72738: Added protobuf messages for constraints-based offer filtering.

Re: Review Request 72738: Added protobuf messages for constraints-based offer filtering.

Review Request 72771: Removed `evolve(const scheduler::Call&)`.

Re: Review Request 72770: Updated the help message of the agent flag `--csi_plugin_config_dir`.

Re: Review Request 72728: Added a unit test for the CSI server.

Review Request 72770: Updated the help message of the agent flag `--csi_plugin_config_dir`.

Re: Review Request 72761: Added the CSI server to the Mesos agent.

Re: Review Request 72760: Passed the CSI server into the Mesos containerizer.

Re: Review Request 72728: Added a unit test for the CSI server.

Re: Review Request 72728: Added a unit test for the CSI server.

Re: Review Request 72754: Enabled the `volume/csi` isolator in `MesosContainerizer`.

20 matches

Site Navigation

Mail list logo

Footer information