Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

JENKINS-40825 close resource leak, fix broken pipe error #182

Merged
merged 11 commits into from
Aug 9, 2017
Merged

JENKINS-40825 close resource leak, fix broken pipe error #182

merged 11 commits into from
Aug 9, 2017

Commits on Aug 9, 2017

  1. JENKINS-40825 close resource leak, fix broken pipe error 

    First of all, `Launcher.launch` is called not once, but again and again.
I.e. overwriting shared members of the enclosing class that a different
thread is still using (and only cleaning the ones that were written last).
That is a bad idea, and caused the thread and connection leak.

So, move them (`watch` and `proc`) to local variables, and close them
elsewhere:
* `proc.kill()` should be taken care of by the shell step (that needs to be
verified)
* `watch` was already cleaned up in `ContainerExecProc`
anyway.

Second of all, `waitQuietly` on the `started` latch was what caused
the "pipe not connected" errors:

In case of InterruptedException, it caused the launcher to just continue
as if `started` had already been counted down (i.e. the websocket connection
established), when in reality the thread  just interrupted itself because of a timeout
(probably from [`DurableTaskStep.check()`][1]).

[1]: https://github.com/jenkinsci/workflow-durable-task-step-plugin/blob/workflow-durable-task-step-2.13/src/main/java/org/jenkinsci/plugins/workflow/steps/durable_task/DurableTaskStep.java#L303
    Martin Sander committed Aug 9, 2017
    Configuration menu
    Copy the full SHA
    fb4d446 View commit details
    Browse the repository at this point in the history

  2. Implement kill 

    Strongly inspired by [Docker Pipeline Plugin][1], but sending
just a single command to the container to be faster.

The command only uses linux standard tools and works on
debian and alpine, so it should be sufficiently portable.

Gets rid of one TODO :-).

[1]: https://github.com/jenkinsci/docker-workflow-plugin/blob/45f0c04/src/main/java/org/jenkinsci/plugins/docker/workflow/WithContainerStep.java#L249,
    Martin Sander committed Aug 9, 2017
    Configuration menu
    Copy the full SHA
    4b607f0 View commit details
    Browse the repository at this point in the history

  3. Update Kubernetes client version as suggested by @iocanel 

    Seems to completely get rid of the resource leak regarding
InputStreamPumper.
    Martin Sander committed Aug 9, 2017
    Configuration menu
    Copy the full SHA
    0b08fc7 View commit details
    Browse the repository at this point in the history

  4. InterruptedIOException when being interrupted 

    as suggested by @iocanel
    Martin Sander committed Aug 9, 2017
    Configuration menu
    Copy the full SHA
    dc0efee View commit details
    Browse the repository at this point in the history

  5. Use client.pods().inNamespace(namespace).withName(podName).waitUntilR… 

    …eady

as suggested by @iocanel.

Also add reference to the Jenkins issue and more context when throwing an
Exception because of timeout.
    Martin Sander committed Aug 9, 2017
    Configuration menu
    Copy the full SHA
    516ba1f View commit details
    Browse the repository at this point in the history

  6. Make number of concurrent requests to Kubernetes configurable 

    This solves the actual problem causing JENKINS-40825 - OkHttp limiting
itself to 5 requests per hosts, which is not sufficient when running
20 builds at once.
Each build uses a websocket connection for the command it runs,
and additionally spawns more connections to check if the command
is still running.
    Martin Sander committed Aug 9, 2017
    Configuration menu
    Copy the full SHA
    eeea0d9 View commit details
    Browse the repository at this point in the history

  7. Use kubernetes-client Config instead of reaching into client

    Martin Sander committed Aug 9, 2017
    Configuration menu
    Copy the full SHA
    968d888 View commit details
    Browse the repository at this point in the history

  8. typo

    Martin Sander committed Aug 9, 2017
    Configuration menu
    Copy the full SHA
    15c6842 View commit details
    Browse the repository at this point in the history

  9. prevent excessive string creation 

    if log level is not set to FINEST..
    Martin Sander committed Aug 9, 2017
    Configuration menu
    Copy the full SHA
    f0f9198 View commit details
    Browse the repository at this point in the history

  10. Be more defensive about freeing resources 

    Although this *should* not happen, just free up the watches when the
ContainerStep finishes for good measure.
    Martin Sander committed Aug 9, 2017
    Configuration menu
    Copy the full SHA
    ef93e9a View commit details
    Browse the repository at this point in the history

  11. silence findbugs error 

    we should be fine if we get deserialized - how are we supposed to clean
up those procs in that case?
    Martin Sander committed Aug 9, 2017
    Configuration menu
    Copy the full SHA
    a11d177 View commit details
    Browse the repository at this point in the history