Skip to content

Conversation

@guguducken
Copy link
Contributor

@guguducken guguducken commented Jun 5, 2025

User description

Sometimes, the network between the runner and the apt software repository is very poor, which will cause packaging failure. We need to package all dependent software into the base image.

Approved by: @XuPeng-SH, @fengttt

What type of PR is this?

  • API-change
  • BUG
  • Improvement
  • Documentation
  • Feature
  • Test and CI
  • Code Refactoring

Which issue(s) this PR fixes:

issue https://github.com/matrixorigin/MO-Cloud/issues/5495

What this PR does / why we need it:

Sometimes, the network between the runner and the apt software repository is very poor, which will cause packaging failure. We need to package all dependent software into the base image.


PR Type

Enhancement, Documentation


Description

  • Refactored Dockerfile to use pre-built base images with dependencies

    • Removed in-Dockerfile apt installations for build and runtime
    • Switched to matrixorigin/golang and matrixorigin/ubuntu base images
    • Combined ldconfig and service check into a single RUN command
  • Added README documenting new base image dependency management


Changes walkthrough 📝

Relevant files
Enhancement
Dockerfile
Refactor Dockerfile to use pre-built images and remove apt installs

optools/images/Dockerfile

  • Removed apt-get install steps for build and runtime dependencies
  • Switched to using matrixorigin/golang and matrixorigin/ubuntu images
  • Combined ldconfig and service check into one RUN command
  • Simplified Dockerfile for more reliable builds
  • +3/-25   
    Documentation
    README.md
    Add README for base image dependency management                   

    optools/images/README.md

  • Added documentation explaining new base image strategy
  • Provided links to Dockerfiles and build workflows for base images
  • Instructions for updating dependencies via base image rebuilds
  • +10/-0   

    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • Sometimes, the network between the runner and the apt software repository is very poor, which will cause packaging failure. We need to package all dependent software into the base image.
    
    Approved by: @XuPeng-SH, @fengttt
    @qodo-code-review
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Version Pinning

    The Dockerfile uses base images without specific version tags (only matrixorigin/ubuntu:22.04). Consider using more specific version tags for both base images to ensure build reproducibility.

    FROM matrixorigin/ubuntu:22.04
    
    Error Handling

    The RUN command that executes mo-service -h for validation doesn't have explicit error handling. If the service check fails, the build might still succeed without proper indication.

    RUN ldconfig && /mo-service -h
    

    @matrix-meow matrix-meow added the size/S Denotes a PR that changes [10,99] lines label Jun 5, 2025
    @mergify mergify bot requested a review from XuPeng-SH June 5, 2025 06:46
    @mergify mergify bot added the kind/test-ci label Jun 5, 2025
    @qodo-code-review
    Copy link

    qodo-code-review bot commented Jun 5, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Add dependency details
    Suggestion Impact:The suggestion was directly implemented in the commit. The exact text suggested was added to the README, providing details about the dependencies included in the matrixorigin/golang and matrixorigin/ubuntu base images.

    code diff:

    -At present, we have transferred the installation process of the software required during the packaging process and the running process to the packaging of the base image. 
    +At present, we have transferred the installation process of the software required during the packaging process and the running process to the packaging of the base image.
    +
    +The `matrixorigin/golang` image includes build dependencies (build-essential, git, cmake, etc.), while the `matrixorigin/ubuntu` image contains runtime dependencies (dnsutils, curl, git, cmake, libcurl4-openssl-dev, libgomp1).

    The README lacks information about what specific dependencies are included in
    each base image. This makes it difficult for developers to know what's available
    without checking external repositories.

    optools/images/README.md [1-3]

    -At present, we have transferred the installation process of the software required during the packaging process and the running process to the packaging of the base image. 
    +At present, we have transferred the installation process of the software required during the packaging process and the running process to the packaging of the base image.
    +
    +The `matrixorigin/golang` image includes build dependencies (build-essential, git, cmake, etc.), while the `matrixorigin/ubuntu` image contains runtime dependencies (dnsutils, curl, git, cmake, libcurl4-openssl-dev, libgomp1).
     
     If you want to update some software, please go to the corresponding address to modify it and publish a new base image.

    [Suggestion processed]

    Suggestion importance[1-10]: 5

    __

    Why: Valid documentation improvement that would help developers understand what dependencies are available in each base image without checking external repositories. However, it's a minor enhancement to documentation rather than a critical fix.

    Low
    • Update

    Co-authored-by: qodo-merge-pro[bot] <151058649+qodo-merge-pro[bot]@users.noreply.github.com>
    @mergify mergify bot merged commit 39daeb3 into matrixorigin:2.2-dev Jun 5, 2025
    18 checks passed
    @guguducken guguducken deleted the modify-2.2-dockerfile branch June 6, 2025 05:00
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Labels

    kind/test-ci Review effort 2/5 size/S Denotes a PR that changes [10,99] lines

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    4 participants