Re: Review Request 72759: Exposed Mesos agent ID to managed CSI plugins.

2020-08-18 Thread Qian Zhang 

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

(Updated Aug. 19, 2020, 10:52 a.m.)


Review request for mesos, Andrei Budnik and Greg Mann.


Changes
---

Minor changes.


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


Repository: mesos


Description
---

Exposed Mesos agent ID to managed CSI plugins.


Diffs (updated)
-

  src/csi/service_manager.hpp 76a80fb793d2293e56dec221da23290df6a4acc6 
  src/csi/service_manager.cpp 7a8d8e5dc3c3bae5251284652d73b33ca554f783 
  src/resource_provider/storage/provider.cpp 
0a8dc26e66db0242474bcbbd0b2ff9cec81c58f5 
  src/slave/csi_server.cpp b435d5db194ffad736838a5b98494944d3264045 


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

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


Testing
---


Thanks,

Qian Zhang

Re: Review Request 72759: Exposed Mesos agent ID to managed CSI plugins.

2020-08-18 Thread Mesos Reviewbot 

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



Bad patch!

Reviews applied: [72759]

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

Error:
ubuntu-16.04: Pulling from mesos/mesos-build
18d680d61657: Pulling fs layer
0addb6fece63: Pulling fs layer
78e58219b215: Pulling fs layer
eb6959a66df2: Pulling fs layer
1105027a5560: Pulling fs layer
c36946130a38: Pulling fs layer
6a6a5e68faab: Pulling fs layer
eb6959a66df2: Waiting
80e07249924c: Pulling fs layer
1105027a5560: Waiting
c36946130a38: Waiting
c4c63e2501db: Pulling fs layer
668b207c2829: Pulling fs layer
80e07249924c: Waiting
ed76dddad568: Pulling fs layer
668b207c2829: Waiting
c4c63e2501db: Waiting
0addb6fece63: Verifying Checksum
0addb6fece63: Download complete
78e58219b215: Verifying Checksum
78e58219b215: Download complete
eb6959a66df2: Verifying Checksum
eb6959a66df2: Download complete
18d680d61657: Verifying Checksum
18d680d61657: Download complete
c36946130a38: Verifying Checksum
c36946130a38: Download complete
80e07249924c: Verifying Checksum
80e07249924c: Download complete
6a6a5e68faab: Verifying Checksum
6a6a5e68faab: Download complete
668b207c2829: Download complete
c4c63e2501db: Verifying Checksum
c4c63e2501db: Download complete
ed76dddad568: Verifying Checksum
18d680d61657: Pull complete
0addb6fece63: Pull complete
78e58219b215: Pull complete
eb6959a66df2: Pull complete
1105027a5560: Verifying Checksum
1105027a5560: Download complete
1105027a5560: Pull complete
c36946130a38: Pull complete
6a6a5e68faab: Pull complete
80e07249924c: Pull complete
c4c63e2501db: Pull complete
668b207c2829: Pull complete
ed76dddad568: Pull complete
Digest: sha256:fa967cbcfb44f55708a3cbc87f245c6d29dd891464db558af56a03ee321526bb
Status: Downloaded newer image for mesos/mesos-build:ubuntu-16.04
docker.io/mesos/mesos-build:ubuntu-16.04
Cloning into '/tmp/SRC'...
Note: checking out '29d5b8121a6e1e91cda5fb2a4566a7c6d7b43c93'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by performing another checkout.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -b with the checkout command again. Example:

  git checkout -b 

autoreconf: Entering directory `.'
autoreconf: configure.ac: not using Gettext
autoreconf: running: aclocal --warnings=all -I m4
autoreconf: configure.ac: tracing
configure.ac:1873: warning: cannot check for file existence when cross compiling
../../lib/autoconf/general.m4:2788: AC_CHECK_FILE is expanded from...
configure.ac:1873: the top level
configure.ac:1878: warning: cannot check for file existence when cross compiling
../../lib/autoconf/general.m4:2788: AC_CHECK_FILE is expanded from...
configure.ac:1878: the top level
configure.ac:2399: warning: AC_RUN_IFELSE called without default to allow cross 
compiling
../../lib/autoconf/general.m4:2759: AC_RUN_IFELSE is expanded from...
configure.ac:2399: the top level
autoreconf: running: libtoolize --copy
libtoolize: putting auxiliary files in '.'.
libtoolize: copying file './ltmain.sh'
libtoolize: putting macros in AC_CONFIG_MACRO_DIRS, 'm4'.
libtoolize: copying file 'm4/libtool.m4'
libtoolize: copying file 'm4/ltoptions.m4'
libtoolize: copying file 'm4/ltsugar.m4'
libtoolize: copying file 'm4/ltversion.m4'
libtoolize: copying file 'm4/lt~obsolete.m4'
autoreconf: running: /usr/bin/autoconf --warnings=all
configure.ac:1873: warning: cannot check for file existence when cross compiling
../../lib/autoconf/general.m4:2788: AC_CHECK_FILE is expanded from...
configure.ac:1873: the top level
configure.ac:1878: warning: cannot check for file existence when cross compiling
../../lib/autoconf/general.m4:2788: AC_CHECK_FILE is expanded from...
configure.ac:1878: the top level
configure.ac:2399: warning: AC_RUN_IFELSE called without default to allow cross 
compiling
../../lib/autoconf/general.m4:2759: AC_RUN_IFELSE is expanded from...
configure.ac:2399: the top level
autoreconf: configure.ac: not using Autoheader
autoreconf: running: automake --add-missing --copy --no-force --warnings=all
configure.ac:50: installing './ar-lib'
configure.ac:34: installing './compile'
configure.ac:24: installing './config.guess'
configure.ac:24: installing './config.sub'
configure.ac:46: installing './install-sh'
configure.ac:46: installing './missing'
3rdparty/Makefile.am:307: warning: source file '$(HTTP_PARSER)/http_parser.c' 
is in a subdirectory,
3rdparty/Makefile.am:307: but option 'subdir-objects' is disabled

Re: Review Request 72759: Exposed Mesos agent ID to managed CSI plugins.

2020-08-18 Thread Greg Mann 

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


Ship it!




Ship It!

- Greg Mann


On Aug. 18, 2020, 4:22 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72759/
> ---
> 
> (Updated Aug. 18, 2020, 4:22 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
> ---
> 
> Exposed Mesos agent ID to managed CSI plugins.
> 
> 
> Diffs
> -
> 
>   src/csi/service_manager.hpp 76a80fb793d2293e56dec221da23290df6a4acc6 
>   src/csi/service_manager.cpp 7a8d8e5dc3c3bae5251284652d73b33ca554f783 
>   src/resource_provider/storage/provider.cpp 
> 0a8dc26e66db0242474bcbbd0b2ff9cec81c58f5 
>   src/slave/csi_server.cpp b435d5db194ffad736838a5b98494944d3264045 
> 
> 
> Diff: https://reviews.apache.org/r/72759/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 72759: Exposed Mesos agent ID to managed CSI plugins.

2020-08-18 Thread Qian Zhang 

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

(Updated Aug. 19, 2020, 12:22 a.m.)


Review request for mesos, Andrei Budnik and Greg Mann.


Changes
---

Addressed review comment.


Summary (updated)
-

Exposed Mesos agent ID to managed CSI plugins.


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


Repository: mesos


Description (updated)
---

Exposed Mesos agent ID to managed CSI plugins.


Diffs (updated)
-

  src/csi/service_manager.hpp 76a80fb793d2293e56dec221da23290df6a4acc6 
  src/csi/service_manager.cpp 7a8d8e5dc3c3bae5251284652d73b33ca554f783 
  src/resource_provider/storage/provider.cpp 
0a8dc26e66db0242474bcbbd0b2ff9cec81c58f5 
  src/slave/csi_server.cpp b435d5db194ffad736838a5b98494944d3264045 


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

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


Testing
---


Thanks,

Qian Zhang

Re: Review Request 72759: Exposed Mesos agent ID to managed CSI plugins.

2020-08-18 Thread Greg Mann 


> On Aug. 13, 2020, 10:21 p.m., Greg Mann wrote:
> > src/csi/service_manager.cpp
> > Lines 740 (patched)
> > 
> >
> > Where does this env var name come from, 'MESOS_NODE_ID'?
> 
> Qian Zhang wrote:
> 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`?
> 
> Greg Mann wrote:
> Hm, I think this may not yield a good ID for the agent, since I think 
> it's possible that the value of `self().address.ip` could be the same across 
> all nodes in the cluster - I think it could be `0.0.0.0`/`INADDR_ANY`, for 
> example. Do you think we need to inject the Mesos agent ID into the service 
> manager so that we can use it as the node ID here?
> 
> Qian Zhang wrote:
> I think `self().address.ip` could not be `0.0.0.0/INADDR_ANY`, see 
> https://github.com/apache/mesos/blob/1.10.0/3rdparty/libprocess/src/process.cpp#L1142:L1159
>  for details.
> 
> > Do you think we need to inject the Mesos agent ID into the service 
> manager so that we can use it as the node ID here?
> 
> Yeah, that's my original thought, but it may involve a bit more code 
> changes in some other components, like CSI server, SLRP. But after second 
> thought I think it should be OK and makes more sense to use Mesos agent ID. I 
> have updated this patch accordingly. And could you please update 
> https://reviews.apache.org/r/72761 by passing Mesos agent ID to CSI server 
> (e.g. as a parameter of `CSIServer::start()`).

Yep, sgtm.


- Greg


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


On Aug. 18, 2020, 4:22 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72759/
> ---
> 
> (Updated Aug. 18, 2020, 4:22 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
> ---
> 
> Exposed Mesos agent ID to managed CSI plugins.
> 
> 
> Diffs
> -
> 
>   src/csi/service_manager.hpp 76a80fb793d2293e56dec221da23290df6a4acc6 
>   src/csi/service_manager.cpp 7a8d8e5dc3c3bae5251284652d73b33ca554f783 
>   src/resource_provider/storage/provider.cpp 
> 0a8dc26e66db0242474bcbbd0b2ff9cec81c58f5 
>   src/slave/csi_server.cpp b435d5db194ffad736838a5b98494944d3264045 
> 
> 
> Diff: https://reviews.apache.org/r/72759/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>