-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45806/#review127688
-----------------------------------------------------------
Thanks for the update!
Doing a quick `git-grep` after applying the patch, I still see a few references
to `mesos-slave` throughout the code base. Alot of these are in the
documentation (which I think should be covered by MESOS-3783). However, the
following other files still have references:
```
src/slave/containerizer/mesos/launch.cpp
src/tests/balloon_framework_test.sh
support/generate-endpoint-help.py
support/test-upgrade.py
```
These files should likely be updated to refer to `mesos-agent` as part of this
patch.
There is also this file:
```
3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake
```
which it's not clear we should change as part of this patch, but I talked to
@vinod and he thinks we should. The Cmake stuff isn't really supported yet, so
he thinks it's OK to just sort of blindly change it and let those working on
the Cmake port sort things out. I'd suggest:
```
# Define target for AGENT.
##########################
set(
AGENT_TARGET mesos-agent
CACHE STRING "Target we use to refer to the agent executable")
```
Also, we should probably make it clear in both the commit message and the
"Testing" section of the review that a full `bootstrap` is required once this
patch has landed. Otherwise, `configure` will break because of the missing
`mesos-slave-*.sh` scripts.
```
cd mesos
./bootstrap
mkdir build
cd build
../configure --prefix=${HOME}/install/mesos
make
```
I ran through this as well as doing a `make install` and inspecting the
contents of `${HOME}/install/mesos` I see:
```
[klueska@core-dev build]$ find ${HOME}/install/mesos/ -name "*slave*"
${HOME}/install/mesos/include/mesos/slave
${HOME}/install/mesos/sbin/mesos-start-slaves.sh
${HOME}/install/mesos/sbin/mesos-stop-slaves.sh
${HOME}/install/mesos/sbin/mesos-slave
${HOME}/install/mesos/share/mesos/webui/master/static/slave.html
${HOME}/install/mesos/share/mesos/webui/master/static/slave_executor.html
${HOME}/install/mesos/share/mesos/webui/master/static/slave_framework.html
${HOME}/install/mesos/share/mesos/webui/master/static/slaves.html
${HOME}/install/mesos/etc/mesos/mesos-slave-env.sh.template
[klueska@core-dev build]$ find ${HOME}/install/mesos/ -name "*agent*"
${HOME}/install/mesos/sbin/mesos-start-agents.sh
${HOME}/install/mesos/sbin/mesos-stop-agents.sh
${HOME}/install/mesos/sbin/mesos-agent
${HOME}/install/mesos/etc/mesos/mesos-agent-env.sh.template
```
which seems consistent with what we want for this patch.
However there are still references to the word 'slave' in places such as
`include/mesos/slave` and the webui stuff. The webui stuff should be covered
by MESOS-3779, but there is no ticket covering a rename of the
`include/mesos/slave` folder. I talked to @vinod about this, and he suggested
making a separate ticket to do this rename. Glancing through `src/Makefile.am`,
it may be enough to just change:
```
slavedir = $(pkgincludedir)/slave
slave_HEADERS = \
$(top_srcdir)/include/mesos/slave/container_logger.hpp \
$(top_srcdir)/include/mesos/slave/isolator.hpp \
$(top_srcdir)/include/mesos/slave/isolator.proto \
$(top_srcdir)/include/mesos/slave/oversubscription.hpp \
$(top_srcdir)/include/mesos/slave/oversubscription.proto \
$(top_srcdir)/include/mesos/slave/qos_controller.hpp \
$(top_srcdir)/include/mesos/slave/resource_estimator.hpp
nodist_slave_HEADERS = \
../include/mesos/slave/isolator.pb.h \
../include/mesos/slave/oversubscription.pb.h
```
to
```
agentdir = $(pkgincludedir)/agent
agent_HEADERS = \
$(top_srcdir)/include/mesos/slave/container_logger.hpp \
$(top_srcdir)/include/mesos/slave/isolator.hpp \
$(top_srcdir)/include/mesos/slave/isolator.proto \
$(top_srcdir)/include/mesos/slave/oversubscription.hpp \
$(top_srcdir)/include/mesos/slave/oversubscription.proto \
$(top_srcdir)/include/mesos/slave/qos_controller.hpp \
$(top_srcdir)/include/mesos/slave/resource_estimator.hpp
nodist_agent_HEADERS = \
../include/mesos/slave/isolator.pb.h \
../include/mesos/slave/oversubscription.pb.h
```
and then add another command to your install/uninstall-hooks to create a
symlink from `agent->slave`. Possibly renaming `copy-agent-env-template` to
something more meaningful to cover both the existing stuff, plus setting up
this symlink.
Finally, the CHANGELOG should be updated as part of this patch to point out all
of the changes that have been made.
Thanks!
src/Makefile.am (line 2097)
<https://reviews.apache.org/r/45806/#comment191077>
I would probably put a space here after the colon.
- Kevin Klues
On April 7, 2016, 7:10 a.m., zhou xing wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45806/
> -----------------------------------------------------------
>
> (Updated April 7, 2016, 7:10 a.m.)
>
>
> Review request for mesos, Kevin Klues and Vinod Kone.
>
>
> Bugs: MESOS-3782
> https://issues.apache.org/jira/browse/MESOS-3782
>
>
> Repository: mesos
>
>
> Description
> -------
>
> [#mesos-3782]
> In this patch, we did the following changes:
> 1. Duplicate executable file 'mesos-slave' with 'mesos-agent'
> 2. Rename the scripts in folder 'mesos/bin' & 'mesos/src/deploy'
> to use term 'agent' instead of term 'slave'
> 3. Change the content of ths scripts to use term 'agent' instead
> of term 'slave'
> 4. Duplicate the renamed scripts renamed in #2 during the configure
> and make process, the duplicated scripts still use term 'slave'
> in their names.
>
>
> Diffs
> -----
>
> bin/gdb-mesos-slave.sh.in dbeec8464b26bd808f7a50b8412a2778f1458f22
> bin/lldb-mesos-slave.sh.in 896c411b2b05d3c4a14288002520a5391a88d955
> bin/mesos-local-flags.sh.in ab5b6c8bd8847485c5a47d637c9f4fe88c59ae65
> bin/mesos-slave-flags.sh.in
> bin/mesos-slave.sh.in 1e3b748ed4dc32ba6bd8adece20f439bce38abc3
> bin/valgrind-mesos-slave.sh.in 900c5883d96cf14e121e566bcf1ad4dc9a47abf6
> configure.ac c693b825294f82ace8a14563cf2229820e159e3c
> src/Makefile.am 71c4308cccfa5c56b93f6c3928dd2a1cf3ba9741
> src/deploy/mesos-deploy-env.sh.template
> bea5584fbcc68825b1c35b370aed17b0e432edd5
> src/deploy/mesos-slave-env.sh.template
> 31567d6a47e19385aed56edfc7e67457c8cdde3e
> src/deploy/mesos-start-cluster.sh.in
> f7a003d9a8e5bbb4f11353988e55e715da0b2b4f
> src/deploy/mesos-start-slaves.sh.in
> 50860f40e33fcbb1787be6c035873de4bcc83de5
> src/deploy/mesos-stop-cluster.sh.in
> e5f8c1fb400c56715774889632aa74d9eac33645
> src/deploy/mesos-stop-slaves.sh.in 3dd9b51edff2beb3ccc8d5dd44f0cdc265f623f9
>
> Diff: https://reviews.apache.org/r/45806/diff/
>
>
> Testing
> -------
>
> make
> make check
>
>
> Thanks,
>
> zhou xing
>
>
