mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jan Schlicht <...@mesosphere.io>
Subject Re: Review Request 51865: Added validation for `ContainerInfo`.
Date Mon, 19 Sep 2016 10:52:00 GMT


> On Sept. 19, 2016, 11:24 a.m., Jan Schlicht wrote:
> > src/master/validation.cpp, lines 63-78
> > <https://reviews.apache.org/r/51865/diff/4/?file=1499473#file1499473line63>
> >
> >     This will cause problems with tasks that don't use a container image: While
`ContainerInfo.type` is required, the `DockerInfo` or `MesosInfo` fields are not. If a user
doesn't want to use a container image, he will leave these fields empty and probably set the
type to `MESOS`.
> >     To fix this, the validation should check if the right type is set, when an image
is specified. Like
> >     ```
> >       if (container.has_mesos() && container.type() == ContainerInfo::MESOS)
{
> >         return Error("Expecting 'mesos' to be set for mesos container");
> >       }
> >     ```

Oops, mistake in the example, should be
```
if (container.has_mesos() && container.type() != ContainerInfo::MESOS) {
  return Error("Expecting 'mesos' to be set for mesos container");
}
```


- Jan


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


On Sept. 16, 2016, 12:19 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51865/
> -----------------------------------------------------------
> 
> (Updated Sept. 16, 2016, 12:19 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-6157
>     https://issues.apache.org/jira/browse/MESOS-6157
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/master/validation.cpp aa743d730bebebc0353fa0a7a31f68bc94e7e25d 
>   src/tests/container_logger_tests.cpp 1b121a2fcce2d874aeefc4257b9d4e594866e78d 
>   src/tests/containerizer/cni_isolator_tests.cpp 0d611c196870b6adabea52a48abcd344c8dad5d1

>   src/tests/containerizer/docker_containerizer_tests.cpp 1671d45171307cda62184505ce1dbadc476abca6

>   src/tests/containerizer/docker_volume_isolator_tests.cpp ca7bffd3b1773a11a4679d114885d3edd977b02b

>   src/tests/containerizer/filesystem_isolator_tests.cpp df4642d2667407b1758ffe2efcfdbf9968cf2c33

>   src/tests/containerizer/isolator_tests.cpp 9bb1e69209f34b18b5b64c3daf5ea26780f2ab74

>   src/tests/master_validation_tests.cpp 16c5773aa44016f923e00cb348ded6b8c46d4b4b 
>   src/tests/slave_tests.cpp b44b6fc5627ad97a33be151cb21133da57f3efd8 
> 
> Diff: https://reviews.apache.org/r/51865/diff/
> 
> 
> Testing
> -------
> 
> make check on Mac OS and Linux.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message