Skip to content

Conversation

@mat1e
Copy link
Member

@mat1e mat1e commented Jun 10, 2022

Replace #899

I purpose to change transport layer from Netty to ApacheHttpClient.

It has been developed to fix jenkins-docker-timeout-issue that come with netty implmentation in docker-java.

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

@mat1e mat1e mentioned this pull request Jun 10, 2022
6 tasks
@mat1e mat1e force-pushed the feature/from_netty_to_apachehttpclient branch from b1284b5 to 4ba94a7 Compare November 18, 2022 14:30
@pjdarton
Copy link
Member

FYI I re-triggered the build (just in case the failure was a one-off) but that doesn't seem to be the case. It looks like the all of the integration tests are reporting:

java.lang.NoClassDefFoundError: com/google/common/collect/MultimapBuilder
	at com.github.dockerjava.core.DefaultDockerCmdExecFactory$DefaultWebTarget.<init>(DefaultDockerCmdExecFactory.java:57)
	at com.github.dockerjava.core.DefaultDockerCmdExecFactory.getBaseResource(DefaultDockerCmdExecFactory.java:40)
	at com.github.dockerjava.core.AbstractDockerCmdExecFactory.createListContainersCmdExec(AbstractDockerCmdExecFactory.java:280)
	at com.github.dockerjava.core.DockerClientImpl.listContainersCmd(DockerClientImpl.java:379)
	at io.jenkins.dockerjavaapi.client.DelegatingDockerClient.listContainersCmd(DelegatingDockerClient.java:264)
	at com.nirima.jenkins.plugins.docker.DockerCloud.countContainersInDocker(DockerCloud.java:593)
	at io.jenkins.docker.connector.DockerComputerConnectorTest.dockerIsStillBusy(DockerComputerConnectorTest.java:151)
	at io.jenkins.docker.connector.DockerComputerConnectorTest.cleanup(DockerComputerConnectorTest.java:116)

i.e. it looks like dockerjava requires com.google.common.collect.MultimapBuilder but it isn't on the classpath.

@MarkEWaite MarkEWaite self-requested a review January 7, 2023 00:38
@microscotch
Copy link

microscotch commented Jan 8, 2023

Hi @mat1e,

I think I found a way to fix latest issue with your PR.

For testing purpose, I'm running latest Jenkins LTS (currently 2.375.1) from its ARM64 docker image.
Dockerhost's local docker socket was binded to the Jenkins container in order to setup a docker cloud which use the binded socket (eg. unix:///var/run/docker.sock).
Unfortunately when I run cloud configuration test I received exception java.lang.UnsatisfiedLinkError: no netty_transport_native_epoll_aarch_64 in java.library.path.

After googling raised exception, I found this post which redirects me to your PR.

I read the thread and try to fix latest issue about exception java.lang.NoClassDefFoundError: com/google/common/collect/MultimapBuilder mentionned by @pjdarton in his last post.

Indeed, this class is part of guava, the dependency @MarkEWaite asks you to remove because of upgrade of guava, therefor to be able to resolve class com/google/common/collect/MultimapBuilder, plugin have now to depends at least on Jenkins 2.320 (which bundles guava 31.0.1).

So, I updated pom.xml in order to:

Then I've installed docker-java-api-3.2.13-64.v2d092835754e.hpi and my build result hpi from my Jenkins plugin manager.

After restarting my Jenkins, docker cloud configuration works like a charm, and Jenkins is able to provide docker template as expected 🙂

Best regards,

Microscotch.

@microscotch
Copy link

microscotch commented Jan 9, 2023

Hi @mat1e,

I've just perform some tests to check my hypothesis, which validate my hypothesis regarding the minimal jenkins version required.

My test protocol was to build plugin with maven test activated:

-------------------------------------------------------------------------------------------
| Attempt | Jenkins | docker-java-plugin      | docker-java-transport-httpclient5 | Tests |
| ------- | ------- | ----------------------- | --------------------------------- | ----- |
| #1      | 2.320   | 3.2.13-64.v2d092835754e | Removed                           | OK    |
| #2      | 2.303.3 | 3.2.13-37.vf3411c9828b9 | 3.2.13                            | KO    |
| #3      | 2.303.3 | 3.2.13-64.v2d092835754e | Removed                           | KO    |
| #4      | 2.320   | 3.2.13-37.vf3411c9828b9 | 3.2.13                            | OK    |
-------------------------------------------------------------------------------------------
  • #1 ✔️ Suggested fix
  • #2 ❌ Your PR
  • #3 ❌ Suggested fix without original minimal Jenkins version required
  • #4 ✔️ Your PR with suggested minimal Jenkins version required upgraded

So your PR can be fixed just by updating maven property jenkins.version to at least 2.320 🙂

And once next version of docker-java-plugin will be released, dependency docker-java-transport-httpclient5 might be alse removed as you explained previously.

@mat1e
Copy link
Member Author

mat1e commented Jan 12, 2023

Hi @microscotch,
Many thanks for your help! I blocked on the guava ClassNotFoundException...

@mat1e
Copy link
Member Author

mat1e commented Jan 12, 2023

@pjdarton @MarkEWaite, I need to change the target jenkins version for the build in the JenkinsFile but I don't have the permission to change it as described here

@MarkEWaite
Copy link
Contributor

@pjdarton @MarkEWaite, I need to change the target jenkins version for the build in the JenkinsFile but I don't have the permission to change it as described here

I launched a "Replay" of the most recent build using the modified Jenkinsfile that is included in the pull request.

https://ci.jenkins.io/job/Plugins/job/docker-plugin/view/change-requests/job/PR-900/11/

@MarkEWaite
Copy link
Contributor

MarkEWaite commented Jan 12, 2023

@mat1e the pull request has been merged:

The bill of materials is now included in the pom and the plugin is based on Jenkins 2.332.4 as minimum Jenkins version

@MarkEWaite
Copy link
Contributor

@mat1e I've merged from the master branch into a copy of this branch, removed a few more items from the pom file, and made it visible at https://github.com/MarkEWaite/docker-plugin/commits/feature/from_netty_to_apachehttpclient

@mat1e
Copy link
Member Author

mat1e commented Jan 19, 2023

merged from the master branch into a copy of this branch, removed a few more items from the pom file, and made it visible at https://github.com/MarkEWaite/docker-plugin/commits/feature/from_netty_to_apachehttpclient

@MarkEWaite, It is ok for me if you want to replace the current PR with your branch.

@shindouj
Copy link

Hello @MarkEWaite, @mat1e,

This PR looks so close to be merged - if neither of you can update this PR and/or create a new one based on Mark's branch, I am more than happy to click through it and create one for you all. Please let us know how do you want this deadlock to be resolved.

As a user that is blocked on ARM64 with this PR, I can't wait for this to be merged and deployed. :)

@MarkEWaite
Copy link
Contributor

Hello @MarkEWaite, @mat1e,

This PR looks so close to be merged - if neither of you can update this PR and/or create a new one based on Mark's branch, I am more than happy to click through it and create one for you all. Please let us know how do you want this deadlock to be resolved.

As a user that is blocked on ARM64 with this PR, I can't wait for this to be merged and deployed. :)

Are you willing to test the incremental build of the plugin?

@shindouj
Copy link

Hello @MarkEWaite, @mat1e,
This PR looks so close to be merged - if neither of you can update this PR and/or create a new one based on Mark's branch, I am more than happy to click through it and create one for you all. Please let us know how do you want this deadlock to be resolved.
As a user that is blocked on ARM64 with this PR, I can't wait for this to be merged and deployed. :)

Are you willing to test the incremental build of the plugin?

Sure, as long as you're not expecting a perfect QA process on my side I can check it on my Jenkins instance(s). :)

@MarkEWaite
Copy link
Contributor

MarkEWaite commented Jan 24, 2023

Sure, as long as you're not expecting a perfect QA process on my side I can check it on my Jenkins instance(s). :)

Thanks for being willing to do that. The pre-release build is https://ci.jenkins.io/job/Plugins/job/docker-plugin/job/PR-934/1/artifact/io/jenkins/docker/docker-plugin/1.2.11-rc1065.c3707157f5aa/docker-plugin-1.2.11-rc1065.c3707157f5aa.hpi

Share your experiences with it as a comment here and that will be the catalyst to persuade me to merge it and release it.

Even better, share your results in #934 so that they are noted in the pull request that will be merged.

@MarkEWaite
Copy link
Contributor

Closing this pull request in favor of #934

@MarkEWaite MarkEWaite closed this Jan 24, 2023
@pjdarton
Copy link
Member

Thank you (all) for persevering with this.
It was a long battle to get this working & I applaud your efforts. I'm sorry I had to abandon things (my own work pulled me away from here) and I'm glad to see that it worked out in the end.

@MarkEWaite
Copy link
Contributor

Thank you (all) for persevering with this. ... I'm glad to see that it worked out in the end.

Thanks so much for your efforts @pjdarton ! Your contributions to the plugin are greatly appreciated. Any time you feel like contributing, we're thrilled to have whatever help you can offer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants