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

fix(jenkins/release/jenkins): use offical chart #1200

Merged
merged 2 commits into from
Aug 3, 2024
Merged

Conversation

wuhuizuo
Copy link
Collaborator

@wuhuizuo wuhuizuo commented Aug 3, 2024

No description provided.

Signed-off-by: wuhuizuo <wuhuizuo@126.com>
@ti-chi-bot ti-chi-bot bot requested a review from purelind August 3, 2024 06:31
@ti-chi-bot ti-chi-bot bot added area/apps env/prod will deploy on the main product cluster size/L labels Aug 3, 2024
Copy link
Contributor

ti-chi-bot bot commented Aug 3, 2024

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

The pull request appears to be making several changes to the Jenkins setup in a Kubernetes environment. Here are the key changes:

  1. Switches the Jenkins Helm chart to an official chart instead of a local chart.
  2. Changes the Jenkins URL from HTTP to HTTPS.
  3. Changes the node selector for the Jenkins pod from enable-ci=true to ci-nvme-high-performance=true.
  4. Adds new secrets for Harbor.
  5. Enables the build-failure-analyzer and cloudevents in the Jenkins Configuration as Code (JCasC).
  6. Removes parameters such as podRetention, image, tag, command, args from the Jenkins agent configuration.
  7. Adds more plugins to install in the Jenkins controller.
  8. Changes the Jenkins image from a custom image to the official Jenkins image with JDK 17.
  9. Adds an init container to create the logs directory.
  10. Modifies the ingress configuration.
  11. Removes some configurations from the Prometheus configuration.
  12. Removes some configurations from the persistence configuration.
  13. Keeps the service account's image pull secret name as is.

Potential Problems:

  1. The switch from HTTP to HTTPS in the Jenkins URL requires a valid SSL certificate. Ensure that the SSL certificate is correctly configured for the domain.
  2. The new node selector ci-nvme-high-performance=true assumes that there are nodes labeled with this. Ensure these nodes exist and have enough resources to run Jenkins.
  3. The Harbor secrets need to be correctly set up in Kubernetes.
  4. The build-failure-analyzer and cloudevents require the respective plugins and configurations to be properly installed and set up.
  5. The new plugins added to the Jenkins controller may have compatibility issues with the current Jenkins version or with each other.
  6. Changing the Jenkins image may remove some custom configurations or plugins that were previously used. Ensure that the new image can support all the current workflows.
  7. The modifications to the ingress configuration may affect the routing to the Jenkins service. Ensure that the routing still works as expected.
  8. The removed configurations from the Prometheus and persistence configurations may affect the monitoring and persistence of data. Ensure that these are not needed or are handled in other ways.
  9. The pull request does not contain any tests to verify the changes.

Fixing Suggestions:

  1. Ensure that the SSL certificate is correctly configured for the Jenkins URL.
  2. Ensure that nodes with the label ci-nvme-high-performance=true exist and have enough resources.
  3. Create the Harbor secrets in Kubernetes.
  4. Install and configure the build-failure-analyzer and cloudevents plugins.
  5. Verify that the new plugins are compatible with the current Jenkins version and with each other.
  6. Test that the new Jenkins image supports all the current workflows.
  7. After applying the changes, verify that the routing to the Jenkins service works as expected.
  8. If the removed Prometheus and persistence configurations are needed, find other ways to implement them.
  9. Create tests to verify the changes, including running the Jenkins workflows and checking the Prometheus metrics.

Signed-off-by: wuhuizuo <wuhuizuo@126.com>
Copy link
Contributor

ti-chi-bot bot commented Aug 3, 2024

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

From the pull request, it seems that there are two key changes:

  1. The Jenkins chart version has been updated to use the official chart.
  2. Some plugins have been added to the controller.

There are no potential problems with these changes. However, there are some suggestions for improvement:

  1. In the values-controller.yaml file, the controller.image and controller.tag have been changed to use the official Jenkins chart. However, it is recommended to use a specific version instead of just latest, as this can cause issues if the chart is updated. Instead, specify a version number.
  2. In the values-controller-plugins.yaml file, the installLatestPlugins field has been set to false. This means that the chart will only install the specified plugins and not the latest versions. If the latest versions of the plugins are required, set this field to true.
  3. In the values-controller.yaml file, there is an initializeOnce field that has been set to true. This means that the Kubernetes job that initializes Jenkins will only run once. If changes are made to the configuration later, the job will not run again. It is recommended to set this field to false during development and testing.
  4. In the values-controller.yaml file, there is a customInitContainers field that has been added. This allows for additional init containers to be added to the Jenkins controller. However, the command and args fields are empty. It is recommended to specify a command and arguments for the init container, so that it does something useful.
  5. In the values-controller.yaml file, there is an ingress and secondaryingress field that have been added. These are used to configure the ingress for Jenkins. However, both fields have the same path and hostName. It is recommended to use different paths and host names for the primary and secondary ingress, so that they can be distinguished.

@wuhuizuo
Copy link
Collaborator Author

wuhuizuo commented Aug 3, 2024

/approve

Copy link
Contributor

ti-chi-bot bot commented Aug 3, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: wuhuizuo

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 added the approved label Aug 3, 2024
@ti-chi-bot ti-chi-bot bot merged commit 16817de into main Aug 3, 2024
3 of 4 checks passed
@ti-chi-bot ti-chi-bot bot deleted the fix/upgrade-jenkins branch August 3, 2024 06:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved area/apps env/prod will deploy on the main product cluster size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant