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

Ubuntu and Fedora #16

Merged
merged 10 commits into from
Mar 7, 2024
Merged

Ubuntu and Fedora #16

merged 10 commits into from
Mar 7, 2024

Conversation

efrecon
Copy link
Owner

@efrecon efrecon commented Mar 7, 2024

Also create Ubuntu images and make them the default. The set of packages is almost the same in both OSes.

Summary by CodeRabbit

  • New Features

    • Introduced multi-platform support for Ubuntu and Fedora operating systems in the CI/CD workflows.
    • Added functionality to build base Ubuntu and Fedora images with a minimal set of binaries and packages.
    • Enhanced project documentation to reflect support for multi-platform OCI images and modifications to the base images.
  • Refactor

    • Improved CI/CD workflow strategies to include matrix builds across different operating systems.
    • Streamlined package installation and user creation processes in installation scripts for better code organization and readability.
  • Documentation

    • Updated README.md to provide details on the project's functionality, including the support for multi-platform OCI images and changes to base images.
  • Chore

    • Updated Docker configurations and scripts to support the new multi-platform functionality and to optimize container storage settings.

Copy link
Contributor

coderabbitai bot commented Mar 7, 2024

Walkthrough

The project has introduced a matrix strategy for CI workflows to support multiple operating systems, Ubuntu and Fedora, enhancing the build process's flexibility. The Dockerfiles and scripts have been updated to accommodate these changes, including dynamic package installation and configuration adjustments based on the distribution. Additionally, the project now emphasizes creating fast-booting MicroVMs for self-hosted GitHub runners, supporting multi-platform OCI images, and refining the base image to optimize runner execution.

Changes

File(s) Summary of Changes
.github/workflows/ci.yml, .github/workflows/dev.yml Introduced a matrix strategy for Ubuntu and Fedora. Updated Dockerfile references to include the OS in image and file names, with corresponding build arguments.
.github/workflows/test.yml Added commands for Docker container information and running a hello-world container.
Dockerfile, Dockerfile.base.ubuntu Adjusted for different base images based on the OS (fedora for Dockerfile and setup for Ubuntu in Dockerfile.base.ubuntu), including script and library preparations, permissions, and base script execution.
README.md Updated to reflect support for multi-platform OCI images (Ubuntu and Fedora), emphasizing fast-booting MicroVMs for self-hosted GitHub runners and modifications to base images.
base/base.sh Refactored to streamline package installation for Fedora and Ubuntu, enhance logging, and improve code organization. Adjusted Docker installation and user creation based on distribution.
base/root/etc/containers/storage.conf Configured storage options for containers/storage library tools, including drivers, paths, image pulling options, UID/GID mappings, and thinpool settings.
runner.sh Updated to use a different OCI image (ghcr.io/efrecon/runner-krunvm-ubuntu:main) for improved compatibility and performance.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit-tests for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit tests for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit tests.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 929a7af and 07ab127.
Files selected for processing (9)
  • .github/workflows/ci.yml (2 hunks)
  • .github/workflows/dev.yml (2 hunks)
  • .github/workflows/test.yml (1 hunks)
  • Dockerfile (2 hunks)
  • Dockerfile.base.ubuntu (1 hunks)
  • README.md (4 hunks)
  • base/base.sh (3 hunks)
  • base/root/etc/containers/storage.conf (1 hunks)
  • runner.sh (1 hunks)
Files skipped from review due to trivial changes (1)
  • .github/workflows/test.yml
Additional comments: 20
Dockerfile.base.ubuntu (4)
  • 2-2: The default ARG UBUNTU_VERSION=23.10 sets the Ubuntu version to 23.10. This is a good practice as it allows for easy updates and customization. However, consider verifying if this is the most stable and compatible version for the project's needs.
  • 5-5: The ARG INSTALL_NAMESPACE=/opt/gh-runner-krunvm defines a custom installation namespace. This is a good practice for organizing project-related files and avoiding conflicts with system directories.
  • 7-10: Copying scripts and setting executable permissions before running the base installation script is a standard procedure in Dockerfiles. However, ensure that the scripts in base/*.sh and lib/*.sh are properly sanitized and do not contain any sensitive information or security vulnerabilities.
  • 11-11: Copying the base/root/ directory to the root of the filesystem can potentially overwrite system files or configurations. Ensure that this operation is safe and does not introduce security risks or unwanted changes to the base image.
Dockerfile (2)
  • 3-3: Setting ARG DISTRO=fedora by default aligns with the PR's objective to support both Ubuntu and Fedora. However, since the PR aims to make Ubuntu the default, consider if this default value should be changed to Ubuntu or if the choice of default is determined elsewhere in the project.
  • 13-13: The command "${INSTALL_NAMESPACE}/bin/install.sh" -v runs the installation script with verbosity. Ensure that the install.sh script is robust against failures and correctly handles errors, especially since it's being run in a Docker build process where failure handling is crucial.
.github/workflows/ci.yml (2)
  • 11-16: Introducing a matrix strategy for os: [ubuntu, fedora] in the build-base job is a great way to support building images for both operating systems. This approach enhances the project's flexibility and aligns with the PR objectives. Ensure that the image and file values dynamically adjust based on the operating system.
  • 25-33: Similarly, the matrix strategy in the build-main job supports building images for both Ubuntu and Fedora. Adding DISTRO=${{ matrix.os }} to the build-args is a good practice for passing the operating system context to the Docker build process. Ensure that the downstream Dockerfiles and scripts correctly utilize this argument.
.github/workflows/dev.yml (2)
  • 11-16: The matrix strategy for os: [ubuntu, fedora] in the build-base job within the dev.yml workflow is consistent with the changes in the ci.yml file. This consistency is crucial for maintaining a uniform build process across different environments. Ensure that the platforms: linux/amd64 specification aligns with the project's platform support objectives.
  • 26-35: The matrix strategy in the build-main job, including the DISTRO=${{ matrix.os }} build argument, is correctly applied here as well. This ensures that development builds also benefit from the multi-platform support. Verify that the VERSION=${{ needs.build-base.outputs.version }} correctly captures the version from the build-base job output.
base/base.sh (6)
  • 75-84: The introduction of the apt_install function for Ubuntu package installation is a good practice for code reuse and maintainability. Ensure that apt-get update and apt-get install -y "$@" are correctly handling errors and that the script exits if these commands fail.
  • 87-161: The install_base function dynamically handles package installation based on the distribution type (fedora or ubuntu). This is a significant improvement in code organization and flexibility. Ensure that the package lists for both distributions are up-to-date and aligned with the project's requirements.
  • 72-183: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [163-191]

The install_docker function correctly handles Docker installation for both Fedora and Ubuntu, including setting up repositories and installing Docker packages. The replacement of the real Docker binary with a wrapper for network configuration is a clever workaround. Ensure that this approach does not introduce security risks or compatibility issues.

  • 193-211: The install_gh function for installing the GitHub CLI is well-implemented for both Fedora and Ubuntu. This ensures that the GitHub CLI is available in the runner environment, enhancing automation capabilities. Verify that the repository configurations and package installations are secure and up-to-date.
  • 213-217: The install_yq function downloads and installs yq directly from GitHub releases. While this approach is straightforward, ensure that the download is secure (using HTTPS) and consider verifying the integrity of the downloaded binary.
  • 219-246: The create_user function dynamically handles user and group creation based on the distribution, including adding the user to the appropriate sudo group. This is a critical step for runner configuration. Ensure that user and group IDs do not conflict with existing system users and that the NOPASSWD sudo configuration does not introduce security risks.
README.md (2)
  • 8-9: The update to the README.md file to mention support for multi-platform OCI images for Ubuntu (default) and Fedora aligns with the PR objectives. This clarification enhances the project's documentation and helps users understand the available options.
  • 120-124: The explanation of the base images installing a minimal set of binaries and packages is an important detail that helps set expectations for users. It's good practice to highlight the differences between these base images and regular GitHub runners. Ensure that the list of installed packages is documented somewhere accessible for users to review.
runner.sh (1)
  • 43-43: The change to use ghcr.io/efrecon/runner-krunvm-ubuntu:main as the default OCI image aligns with the PR's objective to make Ubuntu the default OS for the project's Docker images. This is a straightforward update and correctly implements the desired functionality.
base/root/etc/containers/storage.conf (1)
  • 1-247: The modifications in storage.conf appear to be comprehensive and well-documented, providing clear instructions and defaults for container storage management. This includes the setup for the default storage driver, paths for container storage, and various options for managing storage layers, performance optimizations, and security settings. The inclusion of fuse-overlayfs as the mount_program and enabling metacopy=on for mountopt are particularly noteworthy for improving performance and efficiency in container storage operations. These changes are consistent with best practices for container storage configuration and should contribute positively to the project's infrastructure.

@efrecon efrecon merged commit be55cdf into main Mar 7, 2024
12 of 13 checks passed
@efrecon efrecon deleted the feature/ubuntu branch March 7, 2024 22:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant