You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Andrew Schwartzmeyer <an...@schwartzmeyer.com> on 2018/02/02 21:53:53 UTC

Re: Review Request 65127: Windows: Enabled docker health checks.

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




src/checks/checker_process.hpp
Lines 45 (patched)
<https://reviews.apache.org/r/65127/#comment276565>

    Nit: s/newere/newer



src/checks/checker_process.hpp
Lines 47 (patched)
<https://reviews.apache.org/r/65127/#comment276566>

    Should we tag this so a push to their image doesn't break us?



src/checks/checker_process.cpp
Lines 1144-1151 (patched)
<https://reviews.apache.org/r/65127/#comment276567>

    I still feel like there's a better way to output this status code.
    
    From command prompt:
    ```
    C:\Users\andrew>pwsh -c (Invoke-WebRequest -Uri google.com -UseBasicParsing -SkipCertificateCheck).StatusCode
    200
    
    C:\Users\andrew>
    ```
    
    What's the problem with the output number?


- Andrew Schwartzmeyer


On Jan. 30, 2018, 2:18 a.m., Akash Gupta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65127/
> -----------------------------------------------------------
> 
> (Updated Jan. 30, 2018, 2:18 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Andrew Schwartzmeyer.
> 
> 
> Bugs: MESOS-8498
>     https://issues.apache.org/jira/browse/MESOS-8498
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The network health checks called curl and then executed setns to
> enter to container network namespace, which did not work on Windows.
> To do the equivalent, Windows nows calls docker run with powershell's
> curl equivalent (Invoke-WebRequest) and uses the
> network=container:<ID> flag to enter the container's namespace. The
> command health check was trivially fixed by replacing the hardcoded
> `sh -c`.
> 
> 
> Diffs
> -----
> 
>   src/checks/checker_process.hpp 510f3b2e6e689faaf26595214ce377c2b5518f28 
>   src/checks/checker_process.cpp ddb197b8cc2c503fef5ae20af32f5881fff9833f 
> 
> 
> Diff: https://reviews.apache.org/r/65127/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>


Re: Review Request 65127: Windows: Enabled docker health checks.

Posted by Akash Gupta <ak...@hotmail.com>.

> On Feb. 2, 2018, 9:53 p.m., Andrew Schwartzmeyer wrote:
> > src/checks/checker_process.hpp
> > Lines 47 (patched)
> > <https://reviews.apache.org/r/65127/diff/6/?file=1950449#file1950449line47>
> >
> >     Should we tag this so a push to their image doesn't break us?

I'm not sure how to solve this problem. The newer images are not backwards compatible, so fixed tags will always break on newer Windows releases. Personally, I think it's safest to use this tag. Since we require the semi-annual channel anyway for this feature, I think the windows hosts will be updated and then we'll just pick up the latest powershell image. I don't think we can have a concrete solution until the Windows containers guy figure out what they want to do with container compatibility with different Windows host.


> On Feb. 2, 2018, 9:53 p.m., Andrew Schwartzmeyer wrote:
> > src/checks/checker_process.cpp
> > Lines 1144-1151 (patched)
> > <https://reviews.apache.org/r/65127/diff/6/?file=1950450#file1950450line1147>
> >
> >     I still feel like there's a better way to output this status code.
> >     
> >     
> >     From command prompt:
> >     ```
> >     C:\Users\andrew>pwsh -c (Invoke-WebRequest -Uri google.com -UseBasicParsing -SkipCertificateCheck).StatusCode
> >     200
> >     
> >     C:\Users\andrew>
> >     ```
> >     
> >     What's the problem with the output number?

It's "200" in UTF-16, while the common code parsing the http status is expecting it in UTF-8, why is why there's all that powershell stuff to write to file as ascii. If there is a powershell string.utf16_to_utf8(), I can use that.


- Akash


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


On Feb. 8, 2018, 5:51 p.m., Akash Gupta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65127/
> -----------------------------------------------------------
> 
> (Updated Feb. 8, 2018, 5:51 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Andrew Schwartzmeyer.
> 
> 
> Bugs: MESOS-8498
>     https://issues.apache.org/jira/browse/MESOS-8498
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The network health checks called curl and then executed setns to
> enter to container network namespace, which did not work on Windows.
> To do the equivalent, Windows nows calls docker run with powershell's
> curl equivalent (Invoke-WebRequest) and uses the
> network=container:<ID> flag to enter the container's namespace. The
> command health check was trivially fixed by replacing the hardcoded
> `sh -c`.
> 
> 
> Diffs
> -----
> 
>   src/checks/checker_process.hpp 510f3b2e6e689faaf26595214ce377c2b5518f28 
>   src/checks/checker_process.cpp ddb197b8cc2c503fef5ae20af32f5881fff9833f 
> 
> 
> Diff: https://reviews.apache.org/r/65127/diff/7/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>