Skip to content

Conversation

BSWANG
Copy link
Member

@BSWANG BSWANG commented Jul 8, 2025

No description provided.

@BSWANG BSWANG requested a review from Copilot July 8, 2025 14:18
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors driver installation logic to support container OS versions ≥ 3.4 (Lifsea) and Ubuntu, splits unsupported vs. Linux-specific utilities, and updates build scripts and Dockerfile to use Alibaba Cloud mirrors.

  • Added a Linux-specific utils file (utils_linux.go) and a stub for unsupported platforms (utils_unsupported.go)
  • Introduced getInstallScript and containerOSDriverInstall for dynamic install-script generation in utils.go
  • Updated driver wrappers (DefaultDriver, CompatDriver, OFEDDriver) to select execution method and added Lifsea repo support in Dockerfile

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
internal/drivers/utils_unsupported.go Stubbed out hostExec for non-Linux platforms
internal/drivers/utils_linux.go Linux implementations: driver checks, exec wrappers, RDMA utilities
internal/drivers/utils.go New helpers: getInstallScript, containerOSDriverInstall
internal/drivers/ofed.go OFEDDriver now picks hostExec or containerExec based on platform
internal/drivers/default.go DefaultDriver uses new install helpers but unconditionally calls containerExec for modprobe
internal/drivers/compat.go CompatDriver updated similarly to DefaultDriver
hack/lifsea.repo Added Lifsea repository configuration
Dockerfile Streamlined RUN env exports, added public_mirror stage, Lifsea repo
Comments suppressed due to low confidence (1)

internal/drivers/utils.go:8

  • The new getInstallScript helper generates complex shell commands but lacks unit tests. Consider adding tests to verify output for both compat = true and compat = false scenarios.
func getInstallScript(compat bool) string {

Signed-off-by: bingshen.wbs <bingshen.wbs@alibaba-inc.com>
@l1b0k l1b0k merged commit 452eca1 into AliyunContainerService:main Jul 9, 2025
3 checks passed
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.

2 participants