mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Avinash sridharan <avin...@mesosphere.io>
Subject Re: Review Request 63910: Added 3 tests for TCP/HTTP(S) health check support for Docker container.
Date Wed, 22 Nov 2017 12:22:23 GMT


> On Nov. 17, 2017, 7:25 p.m., Avinash sridharan wrote:
> > src/tests/health_check_tests.cpp
> > Lines 2450 (patched)
> > <https://reviews.apache.org/r/63910/diff/1/?file=1896789#file1896789line2558>
> >
> >     We should try and see if we can publish this image to :https://hub.docker.com/u/mesos/
> >     
> >     Also, I assuming this a custorm image. We should commit the `Dockerfile` for
this image to our test repo. More importantly want to the cert being used in this test to
be part of the repo.
> 
> Qian Zhang wrote:
>     Agree, do you know who has the permission to publish image to https://hub.docker.com/u/mesos/?
And for the Dockerfile & the cert (and even the Python code `https_server.py`), I think
we can just put them in the description of the Docker image so that we can see all of them
in the same place.

Qian for the time being lets just commit the Dockerfile. We can figure out the uploading of
the image in a separate commit. We need to get consensus from AlexR as to where the Dockerfile
should reside in the repo. If necessary we can this as a separate patch after we commit this
patch. Please feel free to drop this if we are going to do this in a separate patch.


> On Nov. 17, 2017, 7:25 p.m., Avinash sridharan wrote:
> > src/tests/health_check_tests.cpp
> > Lines 2525 (patched)
> > <https://reviews.apache.org/r/63910/diff/1/?file=1896789#file1896789line2648>
> >
> >     As you mentioned, given that this test is identical to the HTTP test above,
can we just parameterize the `HealthCheck.type` field as well in the above test case?
> 
> Qian Zhang wrote:
>     Yeah, I thought about that before, but the problem is, besides `ROOT_DOCKER_USERNETWORK_HealthyTaskViaHTTP`
and `ROOT_DOCKER_USERNETWORK_HealthyTaskViaTCP`, we have another test `ROOT_DOCKER_USERNETWORK_HealthyTaskViaHTTPS`,
I am not sure how it can work if we parameterize the `HealthCheck.type` field as well.

Understood, we take this up as an optimization in a separate commit.


- Avinash


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


On Nov. 19, 2017, 1:41 p.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63910/
> -----------------------------------------------------------
> 
> (Updated Nov. 19, 2017, 1:41 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Avinash sridharan.
> 
> 
> Bugs: MESOS-8050
>     https://issues.apache.org/jira/browse/MESOS-8050
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Removed 3 tests which only run on IPv4 Docker network.
>   ROOT_DOCKER_DockerHealthyTaskViaHTTP
>   ROOT_DOCKER_DockerHealthyTaskViaHTTPS
>   ROOT_DOCKER_DockerHealthyTaskViaTCP
> 
> Added 3 tests which run on both IPv4 and IPv6 Docker networks, so they
> should cover the above 3 tests.
>   ROOT_DOCKER_USERNETWORK_HealthyTaskViaHTTP
>   ROOT_DOCKER_USERNETWORK_HealthyTaskViaHTTPS
>   ROOT_DOCKER_USERNETWORK_HealthyTaskViaTCP
> 
> 
> Diffs
> -----
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 67945ddc4f98ffa072f584af8106967e7ff336d3

>   src/tests/health_check_tests.cpp c0dcba265363f2149b217b189ee5a8e925e40ea1 
>   src/tests/mesos.hpp 345b883a8c629bf5bed83e9236632c277f2eb0eb 
> 
> 
> Diff: https://reviews.apache.org/r/63910/diff/2/
> 
> 
> Testing
> -------
> 
> Ran all newly added tests repeatedly (20 times), they all succeeded.
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


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