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

feat(ci/jenkins): add more tools to ci jenkins image #478

Merged

Conversation

purelind
Copy link
Contributor

Add more tools in ci jenkins image

  • mysql client
  • python package: s3cmd, requests
  • gocoverage

@ti-chi-bot ti-chi-bot bot requested a review from wuhuizuo October 31, 2024 08:29
Copy link

ti-chi-bot bot commented Oct 31, 2024

I have already done a preliminary review for you, and I hope to help you do a better job.

Summary:

  • The pull request adds three new tools to the ci/jenkins Docker image: mysql client, python packages s3cmd and requests, and gocoverage.
  • It also updates the base ci/base image by installing mysql-community-client, s3cmd, and requests.
  • The python27 package is installed and symbolic links for python and pip are created.
  • A cache mount is added to speed up the installation of OS packages.

Potential problems:

  • The python27 package is installed, but it is not clear why it is necessary to install an older version of Python.
  • The symbolic links for python and pip may cause issues with other scripts that rely on the default version of Python.
  • The mysql-community-client package is installed without verification of the package signature using the --nogpgcheck flag, which may pose a security risk.

Fixing suggestions:

  • Remove the installation of python27 and its symbolic links, unless it is necessary to use an older version of Python.
  • Avoid creating symbolic links for python and pip to avoid potential conflicts with other scripts.
  • Use the --nogpgcheck flag with caution and consider verifying the package signature before installing.

@ti-chi-bot ti-chi-bot bot added the size/S label Oct 31, 2024
Copy link

ti-chi-bot bot commented Oct 31, 2024

I have already done a preliminary review for you, and I hope to help you do a better job.

Summary of changes:

  • Added more tools to the CI Jenkins image, including MySQL client, Python packages (s3cmd and requests), and gocoverage.
  • Disabled the mysql module and installed mysql-community-client using the downloaded rpm file.
  • Installed gocovmerge and oras tool.
  • Added ownership of the /usr/local/pulsar directory to the jenkins user.

Potential problems:

  • The changes made in the MySQL installation may conflict with the existing MySQL setup, which may cause issues in running the CI/CD pipeline.
  • The addition of new tools may increase the size of the image, which may lead to longer build times and larger storage requirements.

Fixing suggestions:

  • It's better to use a separate MySQL container instead of installing the client in the Jenkins image. This will avoid conflicts with the existing MySQL setup.
  • Consider using a smaller image as a base instead of the current image, to avoid the increase in image size.

Copy link

ti-chi-bot bot commented Oct 31, 2024

I have already done a preliminary review for you, and I hope to help you do a better job.

From the pull request, we can identify that the following tools have been added to the ci jenkins image:

  • mysql client
  • python packages: s3cmd, requests
  • gocoverage

Here are some potential problems that I have identified from the diff:

  • In the base image, the MariaDB-client package was removed and replaced with the mysql-community-client package. This could impact the functionality of any scripts or jobs that were previously using the MariaDB-client.
  • There is no need to create a symbolic link for python2 and pip2 as those packages are already installed.
  • In the base image, pip3 was used to install the requests package, but in the jenkins image, pip was used to install the same package. This could cause conflicts between the two installations.
  • In the jenkins image, the ownership of /usr/local/pulsar was changed to jenkins:jenkins. It is unclear if this is necessary or not.

Here are some suggestions to fix the identified issues:

  • Instead of removing MariaDB-client and installing mysql-community-client, it is better to add mysql-community-client as a new package to avoid breaking any scripts or jobs that depend on MariaDB-client.
  • Remove the symbolic links for python2 and pip2.
  • Use pip3 to install the requests package in the jenkins image to avoid conflicts.
  • It is unclear if changing the ownership of /usr/local/pulsar is necessary or not. If it is not necessary, then it should be removed from the pull request.

Copy link

ti-chi-bot bot commented Oct 31, 2024

I have already done a preliminary review for you, and I hope to help you do a better job.

Based on the pull request title and description, the key changes that this pull request introduces are:

  • Adds MySQL client, python packages, and gocoverage tools to the Jenkins CI image.
  • Fixes an issue with the ownership of the /usr/local/pulsar folder.

Overall, the changes seem reasonable and useful for the Jenkins CI image. However, there are a few potential problems that should be addressed:

  • The mysql80-community-release-el8-1.noarch.rpm package is being installed from an external URL. This could be a potential security issue, as the content of the package cannot be verified. One possible solution is to download the package to the local repository and install it from there.
  • The dnf command is being used to install Python 2 and Python 3.12 packages. Python 2 is deprecated and will not be supported after 2022. It would be better to use Python 3.8 or higher instead of Python 2.
  • The pip command is being used to install Python packages for Python 2, and pip3 for Python 3.12. It would be better to use pip for Python 3.8 or higher and use a Python virtual environment to manage package dependencies for different versions of Python.

Some suggestions for fixing these issues are:

  • Download the mysql80-community-release-el8-1.noarch.rpm package to the local repository and install it from there.
  • Use Python 3.8 or higher instead of Python 2.
  • Use pip for Python 3.8 or higher and use a Python virtual environment to manage package dependencies for different versions of Python.

Additionally, it would be good to add some documentation on how to use the new tools in the Jenkins CI image.

Copy link

ti-chi-bot bot commented Oct 31, 2024

I have already done a preliminary review for you, and I hope to help you do a better job.

Key changes:

  • Added MySQL client, Python packages (s3cmd, requests), and gocoverage to the CI Jenkins image.
  • Updated the Dockerfile for the CI base image to include Python 2 and 3, and installed s3cmd and requests packages for Python 2 and requests package for Python 3.
  • Disabled the MySQL module and installed MySQL client from the official repository.

Potential problems:

  • The addition of more tools may increase the size of the image, which may cause slower builds and deployments.
  • The use of --nogpgcheck in the MySQL installation command may increase the security risks.
  • The use of hardcoded password in the Dockerfile is not secure.

Fixing suggestions:

  • Consider using multi-stage build to minimize the image size.
  • Remove the --nogpgcheck flag and verify the MySQL RPM package signature.
  • Use environment variables or secrets to pass the password to the container instead of hardcoding it in the Dockerfile.

Overall, the changes seem reasonable, and the reviewer can approve the pull request with the suggestions for improvement.

Copy link

ti-chi-bot bot commented Oct 31, 2024

I have already done a preliminary review for you, and I hope to help you do a better job.

Summary of changes:

  • Added more tools to the ci/jenkins image, including mysql client, python packages s3cmd and requests, and gocoverage.
  • Removed the installation of MariaDB-client.
  • Added installation of mysql-community-client.
  • Added permissions for the jenkins user to the /usr/local/pulsar directory.
  • Added installation of gocovmerge.
  • Updated the PATH environment variable to include the GOPATH.

Potential problems:

  • The change in the installation of the MySQL client may break existing scripts or pipelines that rely on the MariaDB-client.
  • The change in permissions for the jenkins user may cause security vulnerabilities.
  • The addition of gocovmerge may increase the image size unnecessarily.

Fixing suggestions:

  • Consider keeping both MariaDB-client and MySQL-client installed to avoid breaking existing scripts or pipelines.
  • Review the permissions set for the jenkins user and ensure that it does not cause any security vulnerabilities.
  • Consider using a smaller alternative to gocovmerge to reduce the image size.

@ti-chi-bot ti-chi-bot bot added size/M and removed size/S labels Oct 31, 2024
@purelind purelind changed the title feat(ci/jenkins): add more tools to ci jenkins imaga feat(ci/jenkins): add more tools to ci jenkins image Oct 31, 2024
Copy link

ti-chi-bot bot commented Oct 31, 2024

I have already done a preliminary review for you, and I hope to help you do a better job.

Based on the pull request title and description, the author has added more tools to the CI Jenkins image, including a MySQL client, Python packages (s3cmd and requests), and gocoverage.

Looking at the diff, the author has added the necessary changes to the Dockerfile, including installing the required packages and updating the environment variables. The author has also added a condition to check the architecture and only proceed with the Pulsar installation if the architecture is x86_64. Additionally, the author has added the installation of gocovmerge and updated the environment variable for the desired Bazel version.

However, there are a few potential problems with this pull request. Firstly, the author has specified specific versions for the Python packages, which might cause compatibility issues if new versions are released. Instead, it would be better to use ranges or keep up with the latest stable release. Secondly, the MySQL package installation step might fail if the package repository is not available or the connection is slow. It might be better to add a retry mechanism or a fallback package. Lastly, the Pulsar installation step is not critical for the CI Jenkins image and might not be necessary to include in this pull request.

To fix these issues, the author can consider using version ranges or the latest stable releases for the Python packages, adding a retry mechanism or fallback package for the MySQL installation step, and removing the Pulsar installation step.

Copy link

ti-chi-bot bot commented Oct 31, 2024

I have already done a preliminary review for you, and I hope to help you do a better job.

Summary of changes:

The pull request adds more tools to the CI Jenkins image, including the mysql client, python packages s3cmd and requests, and gocoverage. It also updates the Dockerfile for the base image to install Python 2, Python 3, and the mysql client. Additionally, the Dockerfile for Jenkins image adds pulsar permission for Linux amd64 architecture, sets the GOPATH environment variable, and installs gocovmerge.

Potential problems:

  • The MariaDB-client package is no longer installed in the base image, which may break existing workflows that rely on it.
  • The mysql-community-client package is installed from a third-party repository. It may be worth checking if this package conflicts with other packages or dependencies.
  • While the s3cmd and requests Python packages are installed for Python 2, only requests is installed for Python 3. It may be worth installing s3cmd for Python 3 as well.
  • The Dockerfile for Jenkins image installs gocovmerge using the go tool. This may cause issues if the Jenkins build agent does not have a compatible version of Go installed.

Fixing suggestions:

  • Consider keeping MariaDB-client in the base image to avoid breaking existing workflows that rely on it.
  • Check if the mysql-community-client package conflicts with other packages or dependencies before installing it.
  • Install s3cmd for both Python 2 and Python 3 to avoid compatibility issues.
  • Consider using a pre-built binary or package for gocovmerge instead of installing it using the go tool. Alternatively, include a specific version of Go in the Jenkins build agent image to ensure compatibility.

Copy link

ti-chi-bot bot commented Oct 31, 2024

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by:

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ti-chi-bot ti-chi-bot bot merged commit e14e3c6 into PingCAP-QE:main Oct 31, 2024
12 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.

1 participant