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

chore(dockerfiles/bases): upgrade docker image pingcap-base to v1.9.1 #361

Merged
merged 6 commits into from
Aug 3, 2024

Conversation

wuhuizuo
Copy link
Contributor

Signed-off-by: wuhuizuo wuhuizuo@126.com

prepare file `br` to build image tidb-ligntning of release-6.5

Signed-off-by: wuhuizuo <wuhuizuo@126.com>
@ti-chi-bot ti-chi-bot bot requested a review from purelind July 30, 2024 12:49
@ti-chi-bot ti-chi-bot bot added the size/M label Jul 30, 2024
Copy link

ti-chi-bot bot commented Jul 30, 2024

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

This pull request primarily upgrades the Docker image pingcap-base from v1.9.0 to v1.9.1. The changes are made across multiple Dockerfiles and YAML files.

Summary of changes:

  1. The Docker image version is upgraded in various Dockerfiles for different components, including tikv-base, ng-monitoring-base, pd-base, tidb-base, tiflash-base, tools-base, etc.
  2. The Docker image version is also upgraded in the skaffold.yaml file.
  3. The Docker image version is upgraded in the packages.yaml.tmpl file.

Potential problems and suggestions:

  1. Breaking Changes: The upgraded Docker image might introduce breaking changes which can affect the application. It is recommended to go through the changelog or release notes of the new Docker image version v1.9.1 to understand the changes.

  2. Compatibility: Ensure that the new Docker image is compatible with the existing system and other Docker images.

  3. Testing: After the upgrade, comprehensive testing should be performed to ensure that the application works as expected.

  4. Syntax Errors: There seems to be extra white spaces introduced in packages.yaml.tmpl file after src: which may cause syntax error. It is recommended to remove the trailing white spaces.

  5. Sign-off: The commit is signed-off which is a good practice. It shows who is the author of the commit and agrees to the Developer Certificate of Origin.

@wuhuizuo
Copy link
Contributor Author

depends on #360

@wuhuizuo wuhuizuo marked this pull request as draft July 30, 2024 12:50
@wuhuizuo wuhuizuo marked this pull request as ready for review July 30, 2024 12:51
@wuhuizuo
Copy link
Contributor Author

/approve

@ti-chi-bot ti-chi-bot bot added the approved label Jul 31, 2024
packages/packages.yaml.tmpl Outdated Show resolved Hide resolved
packages/packages.yaml.tmpl Outdated Show resolved Hide resolved
Copy link

ti-chi-bot bot commented Jul 31, 2024

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

The pull request mainly focuses on upgrading the Docker image pingcap-base from v1.9.0 to v1.9.1 across multiple Dockerfiles and a skaffold.yaml file. This upgrade has been applied to various Dockerfiles, including those for different products and builders.

Potential Problems:

  1. Version Compatibility: The upgraded version v1.9.1 might not be compatible with the rest of the system or other dependencies which the Docker images rely on. It could lead to unexpected behavior or failure in runtime.
  2. Missing Tests: There is no information about whether or not any tests were conducted to ensure that the upgrade doesn't break anything.

Suggestions:

  1. Run Regression Tests: If not already done, run regression tests to ensure that the upgrade doesn't break any existing functionality.
  2. Perform Integration Tests: Make sure that the image works correctly with other services or dependencies it interacts with.
  3. Verify the Change in a Staging Environment: Before merging the pull request, it would be safe to first deploy the changes in a staging environment and monitor if everything runs smoothly.
  4. Document any Changes: If the new version introduces any significant changes or deprecations, these should be clearly documented so that other developers are aware of them.

Copy link

ti-chi-bot bot commented Jul 31, 2024

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

The pull request is about upgrading the docker image pingcap-base version from v1.9.0 to v1.9.1 in multiple Dockerfiles and yaml files.

Key Changes:

  1. In various Dockerfiles, the base image is updated from v1.9.0 to v1.9.1. This includes Dockerfiles for tikv-base, ng-monitoring-base, pd-base, tidb-base, tiflash-base, tools-base, and pingcap-base.
  2. In skaffold.yaml, the template version is changed from v1.9.0-fips to v1.9.1-fips.
  3. In packages.yaml.tmpl, some formatting changes are made but no significant content change is present.

Potential Problems:

  1. No unit tests or integration tests are included in the PR to verify if the upgraded base image version will work correctly with the rest of the system.
  2. There's no explanation or context in the PR description about why this upgrade is necessary, what improvements v1.9.1 brings compared to v1.9.0, or if there are any potential breaking changes introduced in the new version.

Suggestions:

  1. It would be helpful to include a brief description of the reasons for the upgrade and what changes we can expect from the new version in the PR.
  2. Please include tests to ensure the new base image version doesn't introduce any regressions.
  3. Keep an eye on the CI/CD pipeline after merging this PR to catch any potential issues early. If any problems do arise, rolling back to the previous version v1.9.0 might be necessary until the issues are resolved.

Copy link

ti-chi-bot bot commented Aug 1, 2024

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

This pull request is primarily about upgrading the base docker image version from v1.9.0 to v1.9.1 in various Dockerfile's and skaffold.yaml file. The base images include pingcap-base, ng-monitoring-base, pd-base, tidb-base, tiflash-base, tools-base, tikv-base and others.

Key changes:

  1. Updated the base image versions in multiple Dockerfiles used for building and final image creation.
  2. The skaffold.yaml file has also been updated to reflect the new version number.

Potential Problems:

  1. Compatibility Issues: The new base image version (v1.9.1) might introduce changes that could break the current setup. The dependent code and dependencies installed in the Dockerfile might not work as expected with the new base image.
  2. Missing Tests: There is no information regarding the testing of changes in the pull request. If the tests have not been performed, it might lead to problems in the production.

Suggestions:

  1. It is highly recommended to test these changes thoroughly in a test environment before merging the pull request. Ensure that the applications work as expected using the new base image.
  2. Make sure that the new version of the base image is available and there are no issues with pulling the image from the repository.
  3. If there are any breaking changes introduced by the new base image, the Dockerfile might need to be updated to accommodate these changes.

Copy link

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.

This pull request is about upgrading the Docker image pingcap-base from version v1.9.0 to v1.9.1.

Key changes include:

  1. The Docker image version is updated in multiple Dockerfiles used for building various services, including tikv-base, ng-monitoring, pd, tidb-dashboard, tidb-operator, tidb, tiflash, tiflow, tikv, and others.
  2. The Docker image version is also updated in skaffold.yaml, which is a configuration file for Skaffold, a tool for automating the workflow for building, pushing, and deploying Docker images.

Potential problems:

  1. The new Docker image v1.9.1 needs to be tested thoroughly to make sure it doesn't introduce regressions. This includes unit tests, integration tests, and ideally, smoke tests in a staging environment.
  2. If any of the services have dependencies that are not compatible with the new Docker image version, those services may fail to build or run.

Fixing suggestions:

  1. Run the CI/CD pipeline to make sure all the services can be built and run correctly with the new Docker image.
  2. If the pipeline fails, check the error messages, fix the issues accordingly, and run the pipeline again. Repeat this process until the pipeline passes.
  3. After the pipeline passes, consider deploying the services to a staging environment and run smoke tests to validate the changes.
  4. If everything looks good in the staging environment, this pull request can be merged. Otherwise, investigate and fix the issues found in the staging environment before merging.

@wuhuizuo
Copy link
Contributor Author

wuhuizuo commented Aug 3, 2024

/approve

Copy link

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 merged commit 56d843e into main Aug 3, 2024
29 checks passed
@ti-chi-bot ti-chi-bot bot deleted the fix/base-image-security branch August 3, 2024 09:50
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