You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by James Peach <jp...@apache.org> on 2017/01/05 00:46:40 UTC

Review Request 55191: Add hostname support to the network/cni isolator.

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

Review request for mesos, Avinash sridharan, Jie Yu, and Jiang Yan Xu.


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


Repository: mesos


Description
-------

Since the task info contains a `hostname` field, allow schedulers to
use this to specify the hostname that is assigned to the container.


Diffs
-----

  src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 276b9003aa2671b9ca819e44fb1cd60ae9dff167 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp ea91c71fdfac48a2fc1d31a0ee088a73244be367 

Diff: https://reviews.apache.org/r/55191/diff/


Testing
-------

make check (Fedora 25)

Manual testing:
```
[jpeach@jpeach mesos-config]$ /opt/mesos/bin/mesos-execute --master=127.0.0.1:5050 --task=file://$(pwd)/sleep-task.json
...
[jpeach@jpeach mesos-config]$ cat sleep-task.json
{
  "name": "sleep",
  "agent_id": { "value": "any" },
  "task_id": {
    "value": "sleep-1"
  },
  "resources": [
    {
      "name": "cpus",
      "type": "SCALAR",
      "scalar": {
        "value": 0.4
      },
      "role": "*"
    },
    {
      "name": "mem",
      "type": "SCALAR",
      "scalar": {
        "value": 32
      },
      "role": "*"
    }
  ],
  "command": {
    "value": "ip link > link; ip addr > addr; ip route > route; sleep 1000",
    "environment": {
        " variables": [
            { "name": "GLOG_v", "value": "2" }
        ]
    }
  },
  "container": {
    "type": "MESOS",
    "hostname": "sleep.jpeach.org",
    "network_infos": {
        "name": "cni-test"
    },
    "mesos": {
      "image": {
        "type": "DOCKER",
        "docker": {
            "name": "gala"
        }
      }
    }
  }
}
...
[root@jpeach jpeach]# nsenter -t 2934 -m -u -i -n -w -r
[root@sleep sandbox]# cat /etc/hostname
sleep.jpeach.org[root@sleep sandbox]# hostname
sleep.jpeach.org
[root@sleep sandbox]#
```


Thanks,

James Peach


Re: Review Request 55191: Add hostname support to the network/cni isolator.

Posted by Avinash sridharan <av...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55191/#review160553
-----------------------------------------------------------


Ship it!




Thanks for taking this on. Looks good. Can you run the `CniIsolator` unit-tests. You can do
`sudo bin/mesos-tests.sh --gtest_filter=*Cni*` 

Also, can you update the JIRA. The title still says "UTS namespace" isolator. 

Probably need to add a unit-test for this, but unfortunately we can't do that till we have support for running unit-tests using CNI plugins. In review on that one.

- Avinash sridharan


On Jan. 5, 2017, 12:46 a.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55191/
> -----------------------------------------------------------
> 
> (Updated Jan. 5, 2017, 12:46 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6556
>     https://issues.apache.org/jira/browse/MESOS-6556
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Since the task info contains a `hostname` field, allow schedulers to
> use this to specify the hostname that is assigned to the container.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 276b9003aa2671b9ca819e44fb1cd60ae9dff167 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp ea91c71fdfac48a2fc1d31a0ee088a73244be367 
> 
> Diff: https://reviews.apache.org/r/55191/diff/
> 
> 
> Testing
> -------
> 
> make check (Fedora 25)
> 
> Manual testing:
> ```
> [jpeach@jpeach mesos-config]$ /opt/mesos/bin/mesos-execute --master=127.0.0.1:5050 --task=file://$(pwd)/sleep-task.json
> ...
> [jpeach@jpeach mesos-config]$ cat sleep-task.json
> {
>   "name": "sleep",
>   "agent_id": { "value": "any" },
>   "task_id": {
>     "value": "sleep-1"
>   },
>   "resources": [
>     {
>       "name": "cpus",
>       "type": "SCALAR",
>       "scalar": {
>         "value": 0.4
>       },
>       "role": "*"
>     },
>     {
>       "name": "mem",
>       "type": "SCALAR",
>       "scalar": {
>         "value": 32
>       },
>       "role": "*"
>     }
>   ],
>   "command": {
>     "value": "ip link > link; ip addr > addr; ip route > route; sleep 1000",
>     "environment": {
>         " variables": [
>             { "name": "GLOG_v", "value": "2" }
>         ]
>     }
>   },
>   "container": {
>     "type": "MESOS",
>     "hostname": "sleep.jpeach.org",
>     "network_infos": {
>         "name": "cni-test"
>     },
>     "mesos": {
>       "image": {
>         "type": "DOCKER",
>         "docker": {
>             "name": "gala"
>         }
>       }
>     }
>   }
> }
> ...
> [root@jpeach jpeach]# nsenter -t 2934 -m -u -i -n -w -r
> [root@sleep sandbox]# cat /etc/hostname
> sleep.jpeach.org[root@sleep sandbox]# hostname
> sleep.jpeach.org
> [root@sleep sandbox]#
> ```
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 55191: Add hostname support to the network/cni isolator.

Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55191/#review160689
-----------------------------------------------------------


Fix it, then Ship it!





src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 654)
<https://reviews.apache.org/r/55191/#comment231901>

    s/namespaces/namespace
    Not yours, please help to fix this typo along with this patch :-)



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 945)
<https://reviews.apache.org/r/55191/#comment231900>

    s/addreses/addresses
    Not yours, please help to fix this typo along with this patch :-)


- Qian Zhang


On Jan. 6, 2017, 1:04 a.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55191/
> -----------------------------------------------------------
> 
> (Updated Jan. 6, 2017, 1:04 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6556
>     https://issues.apache.org/jira/browse/MESOS-6556
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Since the task info contains a `hostname` field, allow schedulers to
> use this to specify the hostname that is assigned to the container.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 276b9003aa2671b9ca819e44fb1cd60ae9dff167 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp ea91c71fdfac48a2fc1d31a0ee088a73244be367 
> 
> Diff: https://reviews.apache.org/r/55191/diff/
> 
> 
> Testing
> -------
> 
> make check (Fedora 25)
> 
> Manual testing:
> ```
> [jpeach@jpeach mesos-config]$ /opt/mesos/bin/mesos-execute --master=127.0.0.1:5050 --task=file://$(pwd)/sleep-task.json
> ...
> [jpeach@jpeach mesos-config]$ cat sleep-task.json
> {
>   "name": "sleep",
>   "agent_id": { "value": "any" },
>   "task_id": {
>     "value": "sleep-1"
>   },
>   "resources": [
>     {
>       "name": "cpus",
>       "type": "SCALAR",
>       "scalar": {
>         "value": 0.4
>       },
>       "role": "*"
>     },
>     {
>       "name": "mem",
>       "type": "SCALAR",
>       "scalar": {
>         "value": 32
>       },
>       "role": "*"
>     }
>   ],
>   "command": {
>     "value": "ip link > link; ip addr > addr; ip route > route; sleep 1000",
>     "environment": {
>         " variables": [
>             { "name": "GLOG_v", "value": "2" }
>         ]
>     }
>   },
>   "container": {
>     "type": "MESOS",
>     "hostname": "sleep.jpeach.org",
>     "network_infos": {
>         "name": "cni-test"
>     },
>     "mesos": {
>       "image": {
>         "type": "DOCKER",
>         "docker": {
>             "name": "gala"
>         }
>       }
>     }
>   }
> }
> ...
> [root@jpeach jpeach]# nsenter -t 2934 -m -u -i -n -w -r
> [root@sleep sandbox]# cat /etc/hostname
> sleep.jpeach.org[root@sleep sandbox]# hostname
> sleep.jpeach.org
> [root@sleep sandbox]#
> ```
> 
> CNI Tests (Fedora 25):
> ```
> [jpeach@jpeach build]$ sudo bin/mesos-tests.sh --gtest_filter=*Cni*
> ...
> [==========] Running 7 tests from 1 test case.
> [----------] Global test environment set-up.
> [----------] 7 tests from CniIsolatorTest
> ...
> [----------] 7 tests from CniIsolatorTest (16490 ms total)
> 
> [----------] Global test environment tear-down
> [==========] 7 tests from 1 test case ran. (16514 ms total)
> [  PASSED  ] 7 tests.
> ```
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 55191: Add hostname support to the network/cni isolator.

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



Patch looks great!

Reviews applied: [55191]

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

- Mesos ReviewBot


On Jan. 5, 2017, 5:04 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55191/
> -----------------------------------------------------------
> 
> (Updated Jan. 5, 2017, 5:04 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6556
>     https://issues.apache.org/jira/browse/MESOS-6556
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Since the task info contains a `hostname` field, allow schedulers to
> use this to specify the hostname that is assigned to the container.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 276b9003aa2671b9ca819e44fb1cd60ae9dff167 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp ea91c71fdfac48a2fc1d31a0ee088a73244be367 
> 
> Diff: https://reviews.apache.org/r/55191/diff/
> 
> 
> Testing
> -------
> 
> make check (Fedora 25)
> 
> Manual testing:
> ```
> [jpeach@jpeach mesos-config]$ /opt/mesos/bin/mesos-execute --master=127.0.0.1:5050 --task=file://$(pwd)/sleep-task.json
> ...
> [jpeach@jpeach mesos-config]$ cat sleep-task.json
> {
>   "name": "sleep",
>   "agent_id": { "value": "any" },
>   "task_id": {
>     "value": "sleep-1"
>   },
>   "resources": [
>     {
>       "name": "cpus",
>       "type": "SCALAR",
>       "scalar": {
>         "value": 0.4
>       },
>       "role": "*"
>     },
>     {
>       "name": "mem",
>       "type": "SCALAR",
>       "scalar": {
>         "value": 32
>       },
>       "role": "*"
>     }
>   ],
>   "command": {
>     "value": "ip link > link; ip addr > addr; ip route > route; sleep 1000",
>     "environment": {
>         " variables": [
>             { "name": "GLOG_v", "value": "2" }
>         ]
>     }
>   },
>   "container": {
>     "type": "MESOS",
>     "hostname": "sleep.jpeach.org",
>     "network_infos": {
>         "name": "cni-test"
>     },
>     "mesos": {
>       "image": {
>         "type": "DOCKER",
>         "docker": {
>             "name": "gala"
>         }
>       }
>     }
>   }
> }
> ...
> [root@jpeach jpeach]# nsenter -t 2934 -m -u -i -n -w -r
> [root@sleep sandbox]# cat /etc/hostname
> sleep.jpeach.org[root@sleep sandbox]# hostname
> sleep.jpeach.org
> [root@sleep sandbox]#
> ```
> 
> CNI Tests (Fedora 25):
> ```
> [jpeach@jpeach build]$ sudo bin/mesos-tests.sh --gtest_filter=*Cni*
> ...
> [==========] Running 7 tests from 1 test case.
> [----------] Global test environment set-up.
> [----------] 7 tests from CniIsolatorTest
> ...
> [----------] 7 tests from CniIsolatorTest (16490 ms total)
> 
> [----------] Global test environment tear-down
> [==========] 7 tests from 1 test case ran. (16514 ms total)
> [  PASSED  ] 7 tests.
> ```
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 55191: Add hostname support to the network/cni isolator.

Posted by James Peach <jp...@apache.org>.

> On Jan. 6, 2017, 6:58 p.m., Jie Yu wrote:
> > i feel that we can write a unit test using `__MESOS_TEST__`, can't we?
> 
> Avinash sridharan wrote:
>     Actually you are right. Didn't realize we are actually setting up a new UTS name space for __MESOS_TEST__
>     https://github.com/apache/mesos/blob/e6005743ea247f381c055dce2e875730b07a3864/src/slave/containerizer/mesos/isolators/network/cni/cni.cpp#L699
>     
>     In which case we safely use __MESOS_TEST__ to write a unit-test.

I updated `CniIsolatorTest.ROOT_VerifyCheckpointedInfo` to verify the hostname is checkpointed.


- James


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


On Jan. 5, 2017, 5:04 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55191/
> -----------------------------------------------------------
> 
> (Updated Jan. 5, 2017, 5:04 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6556
>     https://issues.apache.org/jira/browse/MESOS-6556
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Since the task info contains a `hostname` field, allow schedulers to
> use this to specify the hostname that is assigned to the container.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 276b9003aa2671b9ca819e44fb1cd60ae9dff167 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp ea91c71fdfac48a2fc1d31a0ee088a73244be367 
> 
> Diff: https://reviews.apache.org/r/55191/diff/
> 
> 
> Testing
> -------
> 
> make check (Fedora 25)
> 
> Manual testing:
> ```
> [jpeach@jpeach mesos-config]$ /opt/mesos/bin/mesos-execute --master=127.0.0.1:5050 --task=file://$(pwd)/sleep-task.json
> ...
> [jpeach@jpeach mesos-config]$ cat sleep-task.json
> {
>   "name": "sleep",
>   "agent_id": { "value": "any" },
>   "task_id": {
>     "value": "sleep-1"
>   },
>   "resources": [
>     {
>       "name": "cpus",
>       "type": "SCALAR",
>       "scalar": {
>         "value": 0.4
>       },
>       "role": "*"
>     },
>     {
>       "name": "mem",
>       "type": "SCALAR",
>       "scalar": {
>         "value": 32
>       },
>       "role": "*"
>     }
>   ],
>   "command": {
>     "value": "ip link > link; ip addr > addr; ip route > route; sleep 1000",
>     "environment": {
>         " variables": [
>             { "name": "GLOG_v", "value": "2" }
>         ]
>     }
>   },
>   "container": {
>     "type": "MESOS",
>     "hostname": "sleep.jpeach.org",
>     "network_infos": {
>         "name": "cni-test"
>     },
>     "mesos": {
>       "image": {
>         "type": "DOCKER",
>         "docker": {
>             "name": "gala"
>         }
>       }
>     }
>   }
> }
> ...
> [root@jpeach jpeach]# nsenter -t 2934 -m -u -i -n -w -r
> [root@sleep sandbox]# cat /etc/hostname
> sleep.jpeach.org[root@sleep sandbox]# hostname
> sleep.jpeach.org
> [root@sleep sandbox]#
> ```
> 
> CNI Tests (Fedora 25):
> ```
> [jpeach@jpeach build]$ sudo bin/mesos-tests.sh --gtest_filter=*Cni*
> ...
> [==========] Running 7 tests from 1 test case.
> [----------] Global test environment set-up.
> [----------] 7 tests from CniIsolatorTest
> ...
> [----------] 7 tests from CniIsolatorTest (16490 ms total)
> 
> [----------] Global test environment tear-down
> [==========] 7 tests from 1 test case ran. (16514 ms total)
> [  PASSED  ] 7 tests.
> ```
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 55191: Add hostname support to the network/cni isolator.

Posted by Avinash sridharan <av...@mesosphere.io>.

> On Jan. 6, 2017, 6:58 p.m., Jie Yu wrote:
> > i feel that we can write a unit test using `__MESOS_TEST__`, can't we?

Actually you are right. Didn't realize we are actually setting up a new UTS name space for __MESOS_TEST__
https://github.com/apache/mesos/blob/e6005743ea247f381c055dce2e875730b07a3864/src/slave/containerizer/mesos/isolators/network/cni/cni.cpp#L699

In which case we safely use __MESOS_TEST__ to write a unit-test.


- Avinash


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


On Jan. 5, 2017, 5:04 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55191/
> -----------------------------------------------------------
> 
> (Updated Jan. 5, 2017, 5:04 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6556
>     https://issues.apache.org/jira/browse/MESOS-6556
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Since the task info contains a `hostname` field, allow schedulers to
> use this to specify the hostname that is assigned to the container.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 276b9003aa2671b9ca819e44fb1cd60ae9dff167 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp ea91c71fdfac48a2fc1d31a0ee088a73244be367 
> 
> Diff: https://reviews.apache.org/r/55191/diff/
> 
> 
> Testing
> -------
> 
> make check (Fedora 25)
> 
> Manual testing:
> ```
> [jpeach@jpeach mesos-config]$ /opt/mesos/bin/mesos-execute --master=127.0.0.1:5050 --task=file://$(pwd)/sleep-task.json
> ...
> [jpeach@jpeach mesos-config]$ cat sleep-task.json
> {
>   "name": "sleep",
>   "agent_id": { "value": "any" },
>   "task_id": {
>     "value": "sleep-1"
>   },
>   "resources": [
>     {
>       "name": "cpus",
>       "type": "SCALAR",
>       "scalar": {
>         "value": 0.4
>       },
>       "role": "*"
>     },
>     {
>       "name": "mem",
>       "type": "SCALAR",
>       "scalar": {
>         "value": 32
>       },
>       "role": "*"
>     }
>   ],
>   "command": {
>     "value": "ip link > link; ip addr > addr; ip route > route; sleep 1000",
>     "environment": {
>         " variables": [
>             { "name": "GLOG_v", "value": "2" }
>         ]
>     }
>   },
>   "container": {
>     "type": "MESOS",
>     "hostname": "sleep.jpeach.org",
>     "network_infos": {
>         "name": "cni-test"
>     },
>     "mesos": {
>       "image": {
>         "type": "DOCKER",
>         "docker": {
>             "name": "gala"
>         }
>       }
>     }
>   }
> }
> ...
> [root@jpeach jpeach]# nsenter -t 2934 -m -u -i -n -w -r
> [root@sleep sandbox]# cat /etc/hostname
> sleep.jpeach.org[root@sleep sandbox]# hostname
> sleep.jpeach.org
> [root@sleep sandbox]#
> ```
> 
> CNI Tests (Fedora 25):
> ```
> [jpeach@jpeach build]$ sudo bin/mesos-tests.sh --gtest_filter=*Cni*
> ...
> [==========] Running 7 tests from 1 test case.
> [----------] Global test environment set-up.
> [----------] 7 tests from CniIsolatorTest
> ...
> [----------] 7 tests from CniIsolatorTest (16490 ms total)
> 
> [----------] Global test environment tear-down
> [==========] 7 tests from 1 test case ran. (16514 ms total)
> [  PASSED  ] 7 tests.
> ```
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 55191: Add hostname support to the network/cni isolator.

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55191/#review160729
-----------------------------------------------------------


Fix it, then Ship it!




i feel that we can write a unit test using `__MESOS_TEST__`, can't we?


src/slave/containerizer/mesos/isolators/network/cni/cni.hpp (line 110)
<https://reviews.apache.org/r/55191/#comment231927>

    I would update the constructor of `Info` to take an optional `hostname`.



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (lines 670 - 677)
<https://reviews.apache.org/r/55191/#comment231929>

    I would re-write the logic here:
    ```
    Option<string> rootfs;
    if (containerConfig.has_rootfs()) {
      rootfs = containerConfig.rootfs();
    }
    
    infos.put(containerId, Owned<Info>(new Info(
        containerNetworks,
        rootfs,
        hostname)));
    ```



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 914)
<https://reviews.apache.org/r/55191/#comment231940>

    s/cniInfo/info/


- Jie Yu


On Jan. 5, 2017, 5:04 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55191/
> -----------------------------------------------------------
> 
> (Updated Jan. 5, 2017, 5:04 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6556
>     https://issues.apache.org/jira/browse/MESOS-6556
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Since the task info contains a `hostname` field, allow schedulers to
> use this to specify the hostname that is assigned to the container.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 276b9003aa2671b9ca819e44fb1cd60ae9dff167 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp ea91c71fdfac48a2fc1d31a0ee088a73244be367 
> 
> Diff: https://reviews.apache.org/r/55191/diff/
> 
> 
> Testing
> -------
> 
> make check (Fedora 25)
> 
> Manual testing:
> ```
> [jpeach@jpeach mesos-config]$ /opt/mesos/bin/mesos-execute --master=127.0.0.1:5050 --task=file://$(pwd)/sleep-task.json
> ...
> [jpeach@jpeach mesos-config]$ cat sleep-task.json
> {
>   "name": "sleep",
>   "agent_id": { "value": "any" },
>   "task_id": {
>     "value": "sleep-1"
>   },
>   "resources": [
>     {
>       "name": "cpus",
>       "type": "SCALAR",
>       "scalar": {
>         "value": 0.4
>       },
>       "role": "*"
>     },
>     {
>       "name": "mem",
>       "type": "SCALAR",
>       "scalar": {
>         "value": 32
>       },
>       "role": "*"
>     }
>   ],
>   "command": {
>     "value": "ip link > link; ip addr > addr; ip route > route; sleep 1000",
>     "environment": {
>         " variables": [
>             { "name": "GLOG_v", "value": "2" }
>         ]
>     }
>   },
>   "container": {
>     "type": "MESOS",
>     "hostname": "sleep.jpeach.org",
>     "network_infos": {
>         "name": "cni-test"
>     },
>     "mesos": {
>       "image": {
>         "type": "DOCKER",
>         "docker": {
>             "name": "gala"
>         }
>       }
>     }
>   }
> }
> ...
> [root@jpeach jpeach]# nsenter -t 2934 -m -u -i -n -w -r
> [root@sleep sandbox]# cat /etc/hostname
> sleep.jpeach.org[root@sleep sandbox]# hostname
> sleep.jpeach.org
> [root@sleep sandbox]#
> ```
> 
> CNI Tests (Fedora 25):
> ```
> [jpeach@jpeach build]$ sudo bin/mesos-tests.sh --gtest_filter=*Cni*
> ...
> [==========] Running 7 tests from 1 test case.
> [----------] Global test environment set-up.
> [----------] 7 tests from CniIsolatorTest
> ...
> [----------] 7 tests from CniIsolatorTest (16490 ms total)
> 
> [----------] Global test environment tear-down
> [==========] 7 tests from 1 test case ran. (16514 ms total)
> [  PASSED  ] 7 tests.
> ```
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 55191: Add hostname support to the network/cni isolator.

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



Patch looks great!

Reviews applied: [55191]

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

- Mesos ReviewBot


On Jan. 18, 2017, 12:54 a.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55191/
> -----------------------------------------------------------
> 
> (Updated Jan. 18, 2017, 12:54 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6556
>     https://issues.apache.org/jira/browse/MESOS-6556
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Since the task info contains a `hostname` field, allow schedulers to
> use this to specify the hostname that is assigned to the container.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 4df69ab98244f2475e6b5330f374bb58159eca41 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp ea91c71fdfac48a2fc1d31a0ee088a73244be367 
>   src/tests/containerizer/cni_isolator_tests.cpp 82697aea98add4adc2cf7eac5f30dd9b467803c9 
> 
> Diff: https://reviews.apache.org/r/55191/diff/
> 
> 
> Testing
> -------
> 
> make check (Fedora 25)
> 
> Manual testing:
> ```
> [jpeach@jpeach mesos-config]$ /opt/mesos/bin/mesos-execute --master=127.0.0.1:5050 --task=file://$(pwd)/sleep-task.json
> ...
> [jpeach@jpeach mesos-config]$ cat sleep-task.json
> {
>   "name": "sleep",
>   "agent_id": { "value": "any" },
>   "task_id": {
>     "value": "sleep-1"
>   },
>   "resources": [
>     {
>       "name": "cpus",
>       "type": "SCALAR",
>       "scalar": {
>         "value": 0.4
>       },
>       "role": "*"
>     },
>     {
>       "name": "mem",
>       "type": "SCALAR",
>       "scalar": {
>         "value": 32
>       },
>       "role": "*"
>     }
>   ],
>   "command": {
>     "value": "ip link > link; ip addr > addr; ip route > route; sleep 1000",
>     "environment": {
>         " variables": [
>             { "name": "GLOG_v", "value": "2" }
>         ]
>     }
>   },
>   "container": {
>     "type": "MESOS",
>     "hostname": "sleep.jpeach.org",
>     "network_infos": {
>         "name": "cni-test"
>     },
>     "mesos": {
>       "image": {
>         "type": "DOCKER",
>         "docker": {
>             "name": "gala"
>         }
>       }
>     }
>   }
> }
> ...
> [root@jpeach jpeach]# nsenter -t 2934 -m -u -i -n -w -r
> [root@sleep sandbox]# cat /etc/hostname
> sleep.jpeach.org[root@sleep sandbox]# hostname
> sleep.jpeach.org
> [root@sleep sandbox]#
> ```
> 
> CNI Tests (Fedora 25):
> ```
> [jpeach@jpeach build]$ sudo bin/mesos-tests.sh --gtest_filter=*Cni*
> ...
> [==========] Running 7 tests from 1 test case.
> [----------] Global test environment set-up.
> [----------] 7 tests from CniIsolatorTest
> ...
> [----------] 7 tests from CniIsolatorTest (16490 ms total)
> 
> [----------] Global test environment tear-down
> [==========] 7 tests from 1 test case ran. (16514 ms total)
> [  PASSED  ] 7 tests.
> ```
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 55191: Add hostname support to the network/cni isolator.

Posted by Jie Yu <yu...@gmail.com>.

> On Jan. 18, 2017, 11:02 a.m., Jie Yu wrote:
> > src/tests/containerizer/cni_isolator_tests.cpp, line 288
> > <https://reviews.apache.org/r/55191/diff/2/?file=1607197#file1607197line288>
> >
> >     I don't like the idea of piggybacking on this test. I'll probably create a new one.

I added  a test for you.


- Jie


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


On Jan. 18, 2017, 12:54 a.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55191/
> -----------------------------------------------------------
> 
> (Updated Jan. 18, 2017, 12:54 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6556
>     https://issues.apache.org/jira/browse/MESOS-6556
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Since the task info contains a `hostname` field, allow schedulers to
> use this to specify the hostname that is assigned to the container.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 4df69ab98244f2475e6b5330f374bb58159eca41 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp ea91c71fdfac48a2fc1d31a0ee088a73244be367 
>   src/tests/containerizer/cni_isolator_tests.cpp 82697aea98add4adc2cf7eac5f30dd9b467803c9 
> 
> Diff: https://reviews.apache.org/r/55191/diff/
> 
> 
> Testing
> -------
> 
> make check (Fedora 25)
> 
> Manual testing:
> ```
> [jpeach@jpeach mesos-config]$ /opt/mesos/bin/mesos-execute --master=127.0.0.1:5050 --task=file://$(pwd)/sleep-task.json
> ...
> [jpeach@jpeach mesos-config]$ cat sleep-task.json
> {
>   "name": "sleep",
>   "agent_id": { "value": "any" },
>   "task_id": {
>     "value": "sleep-1"
>   },
>   "resources": [
>     {
>       "name": "cpus",
>       "type": "SCALAR",
>       "scalar": {
>         "value": 0.4
>       },
>       "role": "*"
>     },
>     {
>       "name": "mem",
>       "type": "SCALAR",
>       "scalar": {
>         "value": 32
>       },
>       "role": "*"
>     }
>   ],
>   "command": {
>     "value": "ip link > link; ip addr > addr; ip route > route; sleep 1000",
>     "environment": {
>         " variables": [
>             { "name": "GLOG_v", "value": "2" }
>         ]
>     }
>   },
>   "container": {
>     "type": "MESOS",
>     "hostname": "sleep.jpeach.org",
>     "network_infos": {
>         "name": "cni-test"
>     },
>     "mesos": {
>       "image": {
>         "type": "DOCKER",
>         "docker": {
>             "name": "gala"
>         }
>       }
>     }
>   }
> }
> ...
> [root@jpeach jpeach]# nsenter -t 2934 -m -u -i -n -w -r
> [root@sleep sandbox]# cat /etc/hostname
> sleep.jpeach.org[root@sleep sandbox]# hostname
> sleep.jpeach.org
> [root@sleep sandbox]#
> ```
> 
> CNI Tests (Fedora 25):
> ```
> [jpeach@jpeach build]$ sudo bin/mesos-tests.sh --gtest_filter=*Cni*
> ...
> [==========] Running 7 tests from 1 test case.
> [----------] Global test environment set-up.
> [----------] 7 tests from CniIsolatorTest
> ...
> [----------] 7 tests from CniIsolatorTest (16490 ms total)
> 
> [----------] Global test environment tear-down
> [==========] 7 tests from 1 test case ran. (16514 ms total)
> [  PASSED  ] 7 tests.
> ```
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 55191: Add hostname support to the network/cni isolator.

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55191/#review162058
-----------------------------------------------------------




src/slave/containerizer/mesos/isolators/network/cni/cni.hpp (lines 93 - 94)
<https://reviews.apache.org/r/55191/#comment233303>

    we don't indent like this.



src/tests/containerizer/cni_isolator_tests.cpp (line 288)
<https://reviews.apache.org/r/55191/#comment233312>

    I don't like the idea of piggybacking on this test. I'll probably create a new one.


- Jie Yu


On Jan. 18, 2017, 12:54 a.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55191/
> -----------------------------------------------------------
> 
> (Updated Jan. 18, 2017, 12:54 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6556
>     https://issues.apache.org/jira/browse/MESOS-6556
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Since the task info contains a `hostname` field, allow schedulers to
> use this to specify the hostname that is assigned to the container.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 4df69ab98244f2475e6b5330f374bb58159eca41 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp ea91c71fdfac48a2fc1d31a0ee088a73244be367 
>   src/tests/containerizer/cni_isolator_tests.cpp 82697aea98add4adc2cf7eac5f30dd9b467803c9 
> 
> Diff: https://reviews.apache.org/r/55191/diff/
> 
> 
> Testing
> -------
> 
> make check (Fedora 25)
> 
> Manual testing:
> ```
> [jpeach@jpeach mesos-config]$ /opt/mesos/bin/mesos-execute --master=127.0.0.1:5050 --task=file://$(pwd)/sleep-task.json
> ...
> [jpeach@jpeach mesos-config]$ cat sleep-task.json
> {
>   "name": "sleep",
>   "agent_id": { "value": "any" },
>   "task_id": {
>     "value": "sleep-1"
>   },
>   "resources": [
>     {
>       "name": "cpus",
>       "type": "SCALAR",
>       "scalar": {
>         "value": 0.4
>       },
>       "role": "*"
>     },
>     {
>       "name": "mem",
>       "type": "SCALAR",
>       "scalar": {
>         "value": 32
>       },
>       "role": "*"
>     }
>   ],
>   "command": {
>     "value": "ip link > link; ip addr > addr; ip route > route; sleep 1000",
>     "environment": {
>         " variables": [
>             { "name": "GLOG_v", "value": "2" }
>         ]
>     }
>   },
>   "container": {
>     "type": "MESOS",
>     "hostname": "sleep.jpeach.org",
>     "network_infos": {
>         "name": "cni-test"
>     },
>     "mesos": {
>       "image": {
>         "type": "DOCKER",
>         "docker": {
>             "name": "gala"
>         }
>       }
>     }
>   }
> }
> ...
> [root@jpeach jpeach]# nsenter -t 2934 -m -u -i -n -w -r
> [root@sleep sandbox]# cat /etc/hostname
> sleep.jpeach.org[root@sleep sandbox]# hostname
> sleep.jpeach.org
> [root@sleep sandbox]#
> ```
> 
> CNI Tests (Fedora 25):
> ```
> [jpeach@jpeach build]$ sudo bin/mesos-tests.sh --gtest_filter=*Cni*
> ...
> [==========] Running 7 tests from 1 test case.
> [----------] Global test environment set-up.
> [----------] 7 tests from CniIsolatorTest
> ...
> [----------] 7 tests from CniIsolatorTest (16490 ms total)
> 
> [----------] Global test environment tear-down
> [==========] 7 tests from 1 test case ran. (16514 ms total)
> [  PASSED  ] 7 tests.
> ```
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 55191: Add hostname support to the network/cni isolator.

Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55191/
-----------------------------------------------------------

(Updated Jan. 18, 2017, 12:54 a.m.)


Review request for mesos, Avinash sridharan, Jie Yu, and Jiang Yan Xu.


Changes
-------

Rebase and address review feedback.


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


Repository: mesos


Description
-------

Since the task info contains a `hostname` field, allow schedulers to
use this to specify the hostname that is assigned to the container.


Diffs (updated)
-----

  src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 4df69ab98244f2475e6b5330f374bb58159eca41 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp ea91c71fdfac48a2fc1d31a0ee088a73244be367 
  src/tests/containerizer/cni_isolator_tests.cpp 82697aea98add4adc2cf7eac5f30dd9b467803c9 

Diff: https://reviews.apache.org/r/55191/diff/


Testing
-------

make check (Fedora 25)

Manual testing:
```
[jpeach@jpeach mesos-config]$ /opt/mesos/bin/mesos-execute --master=127.0.0.1:5050 --task=file://$(pwd)/sleep-task.json
...
[jpeach@jpeach mesos-config]$ cat sleep-task.json
{
  "name": "sleep",
  "agent_id": { "value": "any" },
  "task_id": {
    "value": "sleep-1"
  },
  "resources": [
    {
      "name": "cpus",
      "type": "SCALAR",
      "scalar": {
        "value": 0.4
      },
      "role": "*"
    },
    {
      "name": "mem",
      "type": "SCALAR",
      "scalar": {
        "value": 32
      },
      "role": "*"
    }
  ],
  "command": {
    "value": "ip link > link; ip addr > addr; ip route > route; sleep 1000",
    "environment": {
        " variables": [
            { "name": "GLOG_v", "value": "2" }
        ]
    }
  },
  "container": {
    "type": "MESOS",
    "hostname": "sleep.jpeach.org",
    "network_infos": {
        "name": "cni-test"
    },
    "mesos": {
      "image": {
        "type": "DOCKER",
        "docker": {
            "name": "gala"
        }
      }
    }
  }
}
...
[root@jpeach jpeach]# nsenter -t 2934 -m -u -i -n -w -r
[root@sleep sandbox]# cat /etc/hostname
sleep.jpeach.org[root@sleep sandbox]# hostname
sleep.jpeach.org
[root@sleep sandbox]#
```

CNI Tests (Fedora 25):
```
[jpeach@jpeach build]$ sudo bin/mesos-tests.sh --gtest_filter=*Cni*
...
[==========] Running 7 tests from 1 test case.
[----------] Global test environment set-up.
[----------] 7 tests from CniIsolatorTest
...
[----------] 7 tests from CniIsolatorTest (16490 ms total)

[----------] Global test environment tear-down
[==========] 7 tests from 1 test case ran. (16514 ms total)
[  PASSED  ] 7 tests.
```


Thanks,

James Peach


Re: Review Request 55191: Add hostname support to the network/cni isolator.

Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55191/
-----------------------------------------------------------

(Updated Jan. 5, 2017, 5:04 p.m.)


Review request for mesos, Avinash sridharan, Jie Yu, and Jiang Yan Xu.


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


Repository: mesos


Description
-------

Since the task info contains a `hostname` field, allow schedulers to
use this to specify the hostname that is assigned to the container.


Diffs
-----

  src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 276b9003aa2671b9ca819e44fb1cd60ae9dff167 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp ea91c71fdfac48a2fc1d31a0ee088a73244be367 

Diff: https://reviews.apache.org/r/55191/diff/


Testing (updated)
-------

make check (Fedora 25)

Manual testing:
```
[jpeach@jpeach mesos-config]$ /opt/mesos/bin/mesos-execute --master=127.0.0.1:5050 --task=file://$(pwd)/sleep-task.json
...
[jpeach@jpeach mesos-config]$ cat sleep-task.json
{
  "name": "sleep",
  "agent_id": { "value": "any" },
  "task_id": {
    "value": "sleep-1"
  },
  "resources": [
    {
      "name": "cpus",
      "type": "SCALAR",
      "scalar": {
        "value": 0.4
      },
      "role": "*"
    },
    {
      "name": "mem",
      "type": "SCALAR",
      "scalar": {
        "value": 32
      },
      "role": "*"
    }
  ],
  "command": {
    "value": "ip link > link; ip addr > addr; ip route > route; sleep 1000",
    "environment": {
        " variables": [
            { "name": "GLOG_v", "value": "2" }
        ]
    }
  },
  "container": {
    "type": "MESOS",
    "hostname": "sleep.jpeach.org",
    "network_infos": {
        "name": "cni-test"
    },
    "mesos": {
      "image": {
        "type": "DOCKER",
        "docker": {
            "name": "gala"
        }
      }
    }
  }
}
...
[root@jpeach jpeach]# nsenter -t 2934 -m -u -i -n -w -r
[root@sleep sandbox]# cat /etc/hostname
sleep.jpeach.org[root@sleep sandbox]# hostname
sleep.jpeach.org
[root@sleep sandbox]#
```

CNI Tests (Fedora 25):
```
[jpeach@jpeach build]$ sudo bin/mesos-tests.sh --gtest_filter=*Cni*
...
[==========] Running 7 tests from 1 test case.
[----------] Global test environment set-up.
[----------] 7 tests from CniIsolatorTest
...
[----------] 7 tests from CniIsolatorTest (16490 ms total)

[----------] Global test environment tear-down
[==========] 7 tests from 1 test case ran. (16514 ms total)
[  PASSED  ] 7 tests.
```


Thanks,

James Peach