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.

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

JENKINS-73889: Cannot reuse an open socket #204

Merged
merged 14 commits into from
Oct 27, 2024

Conversation

mpet
Copy link
Contributor

@mpet mpet commented Oct 18, 2024

@mpet mpet requested a review from a team as a code owner October 18, 2024 14:53
pom.xml Outdated Show resolved Hide resolved
pom.xml Outdated Show resolved Hide resolved
pom.xml Outdated Show resolved Hide resolved
@mpet
Copy link
Contributor Author

mpet commented Oct 21, 2024

@kuisathaverat could you please review the code?



# defaults
RUN \
Copy link
Contributor

@kuisathaverat kuisathaverat Oct 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Too many compiled sources from other repos, it will be hard to maintain, Is there an official Docker image for NetOpeer?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kuisathaverat no there is no official docker image. I cannot see any other way to do it. What do you suggest?

Copy link
Contributor

@kuisathaverat kuisathaverat Oct 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not feel comfortable adding 6min build from code five libs and one binary, for a feature we will not use in Jenkins.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed the Docker image to this repository as ghcr.io/jenkinsci/trilead-ssh2:netopeer-3.5.1, so we will keep the Dockerfile, but we will use a compiled Docker image. I saw the Docker image is huge, 1.6GB; maybe we can split it into two stages or remove compilation stuff

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kuisathaverat so no need to change the testcontainer code? Yes I agree it is large but could not really find something that supports ssh call home. Any optimization is fine for me.
Do you expect anything more from me?

Copy link
Contributor

@kuisathaverat kuisathaverat Oct 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also tried this locally:

$ docker build -t ghcr.io/jenkinsci/trilead-ssh2:netopeer-3.5.1 .
[+] Building 0.8s (2/2) FINISHED                                                                                                                                                                                                                            docker:default
 => [internal] load build definition from Dockerfile                                                                                                                                                                                                                  0.6s
 => => transferring dockerfile: 2B                                                                                                                                                                                                                                    0.3s 
 => [internal] load .dockerignore                                                                                                                                                                                                                                     0.6s
 => => transferring context: 2B                                                                                                                                                                                                                                       0.3s 
ERROR: failed to solve: failed to read dockerfile: open /var/lib/docker/tmp/buildkit-mount4081330339/Dockerfile: no such file or directory

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you need to change to the folder test/docker first

Copy link
Contributor Author

@mpet mpet Oct 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kuisathaverat When we use testcontainers we should be able to pull the image. However testcontainers cannot access the image:

FINE: http-outgoing-2 << "{"message":"Head "[https://ghcr.io/v2/jenkinsci/trilead-ssh2/manifests/netopeer-3.5.1\](https://ghcr.io/v2/jenkinsci/trilead-ssh2/manifests/netopeer-3.5.1%5C)": unauthorized"}[\n]"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works on my local and in the CI

@mpet
Copy link
Contributor Author

mpet commented Oct 21, 2024

@kuisathaverat fixed comments. Netopeer2 is not easy to replace unless you know of an alternative.

gcc \
libpsl-dev \
supervisor \
libpam0g-dev
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
libpam0g-dev
libpam0g-dev \
&& rm -rf /var/lib/apt/lists/*

make -j2 && \
make install

ENV EDITOR vim
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ENV EDITOR vim
RUN rm -fr /opt/dev
ENV EDITOR vim

Comment on lines 128 to 133
EXPOSE 830
EXPOSE 4334

# start netopeer2 server.
CMD ["/usr/sbin/netopeer2-server", "-d", "-v2 3"]
#CMD ["/usr/sbin/netopeer2-server", "-d", "-c", "SSH"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
EXPOSE 830
EXPOSE 4334
# start netopeer2 server.
CMD ["/usr/sbin/netopeer2-server", "-d", "-v2 3"]
#CMD ["/usr/sbin/netopeer2-server", "-d", "-c", "SSH"]
EXPOSE 830
EXPOSE 4334
# start netopeer2 server.
CMD ["/usr/sbin/netopeer2-server", "-d", "-v2 3"]
#CMD ["/usr/sbin/netopeer2-server", "-d", "-c", "SSH"]

@@ -0,0 +1,134 @@
FROM ubuntu:22.04

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# This Docker image is build and push to the repository it takes about 6 min to build
# The reason to pre-build it is because it compiles five libraries and an application everytime we run the test
# docker build -t ghcr.io/jenkinsci/trilead-ssh2:netopeer-3.5.1 .

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kuisathaverat in testcontainers we use docker image with name 'netopeer2'. Will testcontainers pick it up if we use
netopeer-3.5.1?

Copy link
Contributor

@kuisathaverat kuisathaverat Oct 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, you have to change to use a Docker image instead of build one

GenericContainer redis = new GenericContainer("ghcr.io/jenkinsci/trilead-ssh2:netopeer-3.5.1")
    .withExposedPorts(830)
    .withExposedPorts(4334)
    .waitingFor(Wait.forLogMessage(".*SOME STARTING MESSAGE*\\n", 1));

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kuisathaverat ok I can change this. And if I want to run this locally?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC ghcr.io it is public if the repo is public so it should work directly

@mpet
Copy link
Contributor Author

mpet commented Oct 22, 2024

@kuisathaverat do you have more comments or is there anything else to change?

@mpet
Copy link
Contributor Author

mpet commented Oct 23, 2024

@kuisathaverat do you have more comments or is there anything else to change?

Friendly reminder :-)

@mpet mpet requested a review from kuisathaverat October 24, 2024 06:00
@mpet
Copy link
Contributor Author

mpet commented Oct 25, 2024

@kuisathaverat what is this

image

@kuisathaverat
Copy link
Contributor

kuisathaverat commented Oct 25, 2024

All the PRs generate an incremental build package, which is a binary with the code from the PR. This allows for the distribution of early versions before the release.

By the way, I'm sorry, but it will take a few more days for me to review the changes here.

@kuisathaverat kuisathaverat merged commit 04c3bda into jenkinsci:main Oct 27, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants