GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Support multi-arch builds by Pearl1594 · Pull Request #3 · cloudstack/cloudstack-csi-driver

Conversation

@Pearl1594

@Pearl1594 Pearl1594 commented Nov 3, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Collaborator

Issue #, if available:

Description of changes:
Builds multi-arch docker images to enable running csi driver on both amd64 & arm64.

Testing performed:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@Pearl1594
Support multi-arch builds
54dfc3c
@Pearl1594
Pearl1594 marked this pull request as ready for review November 4, 2025 11:01
@Pearl1594
Pearl1594 requested review from kiranchavala, vishesh92 and weizhouapache November 4, 2025 11:05
@vishesh92
vishesh92 requested a review from Copilot November 4, 2025 11:30
kiranchavala
kiranchavala approved these changes Nov 4, 2025
View reviewed changes

@kiranchavala kiranchavala left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM, tested with the docker images for arm and a arm cluster in cloudstack

Able to create PVC and pod were using the it for persistent storage

pearl1594/cloudstack-csi-driver:test

Able to deploy

Test manifest file

manifest-arm.yaml

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Copilot AI reviewed Nov 4, 2025
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

There was a problem hiding this 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 modernizes the Docker build process by implementing multi-stage builds with multi-architecture support. The changes enable building container images for both linux/amd64 and linux/arm64 platforms directly in the CI/CD pipeline without requiring pre-built binaries.

Key changes:

  • Dockerfiles now use multi-stage builds with Go compilation inside the builder stage
  • GitHub Actions workflow updated to use Docker Buildx for multi-platform builds
  • Build arguments (LDFLAGS, TARGETOS, TARGETARCH) are passed to Docker builds for cross-compilation

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
cmd/cloudstack-csi-driver/Dockerfile Added multi-stage build with builder stage for Go compilation and multi-arch support
cmd/cloudstack-csi-sc-syncer/Dockerfile Added multi-stage build with builder stage for Go compilation and multi-arch support
.github/workflows/release.yaml Updated to use Docker Buildx for multi-platform builds; removed make container step; added Go build step for syncer binary upload

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread .github/workflows/release.yaml Outdated
Comment on lines +86 to +96
- name: Build syncer binary for upload
if: startsWith(github.ref, 'refs/tags/v')
run: |
REV=$(git describe --long --tags --match='v*' --dirty 2>/dev/null || git rev-list -n1 HEAD)
GIT_COMMIT=$(git rev-parse HEAD)
BUILD_DATE=$(date -u -Iseconds)
PKG=github.com/cloudstack/cloudstack-csi-driver
LDFLAGS="-s -w -X ${PKG}/pkg/driver.driverVersion=${REV} -X ${PKG}/pkg/driver.gitCommit=${GIT_COMMIT} -X ${PKG}/pkg/driver.buildDate=${BUILD_DATE}"

mkdir -p bin
CGO_ENABLED=0 go build -ldflags "${LDFLAGS}" -o ./bin/cloudstack-csi-sc-syncer ./cmd/cloudstack-csi-sc-syncer

Copilot AI Nov 4, 2025

Copy link

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

The 'Build syncer binary for upload' step attempts to run go build but there's no Go setup step in this workflow. This will fail because Go is not installed on the runner. Add a setup-go action before this step, similar to the PR check workflow which uses actions/setup-go@v5 with go-version: \"1.23\".

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Copilot uses AI. Check for mistakes.
Comment thread cmd/cloudstack-csi-driver/Dockerfile
Comment on lines +3 to +4
ARG TARGETOS
ARG TARGETARCH

Copilot AI Nov 4, 2025

Copy link

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

The build arguments TARGETOS and TARGETARCH are declared but Docker Buildx automatically provides these as built-in arguments when using --platform. You should add default values for these arguments to ensure the Dockerfile works correctly in non-buildx contexts. For example: ARG TARGETOS=linux and ARG TARGETARCH=amd64.

Suggested change
ARG TARGETOS
ARG TARGETARCH
ARG TARGETOS=linux
ARG TARGETARCH=amd64

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Copilot uses AI. Check for mistakes.
Comment thread cmd/cloudstack-csi-sc-syncer/Dockerfile
Comment on lines +3 to +4
ARG TARGETOS
ARG TARGETARCH

Copilot AI Nov 4, 2025

Copy link

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

The build arguments TARGETOS and TARGETARCH are declared but Docker Buildx automatically provides these as built-in arguments when using --platform. You should add default values for these arguments to ensure the Dockerfile works correctly in non-buildx contexts. For example: ARG TARGETOS=linux and ARG TARGETARCH=amd64.

Suggested change
ARG TARGETOS
ARG TARGETARCH
ARG TARGETOS=linux
ARG TARGETARCH=amd64

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Copilot uses AI. Check for mistakes.
Comment thread .github/workflows/release.yaml
Comment on lines +47 to +51
REV=$(git describe --long --tags --match='v*' --dirty 2>/dev/null || git rev-list -n1 HEAD)
GIT_COMMIT=$(git rev-parse HEAD)
BUILD_DATE=$(date -u -Iseconds)
PKG=github.com/cloudstack/cloudstack-csi-driver
LDFLAGS="-s -w -X ${PKG}/pkg/driver.driverVersion=${REV} -X ${PKG}/pkg/driver.gitCommit=${GIT_COMMIT} -X ${PKG}/pkg/driver.buildDate=${BUILD_DATE}"

Copilot AI Nov 4, 2025

Copy link

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

The LDFLAGS construction is duplicated across three separate steps (lines 47-51, 69-73, and 89-93). Consider extracting this logic into a reusable composite action or a separate script to reduce code duplication and ensure consistency.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Copilot uses AI. Check for mistakes.
vishesh92
vishesh92 reviewed Nov 4, 2025
View reviewed changes
Comment thread .github/workflows/release.yaml Outdated
- name: Build syncer binary for upload
if: startsWith(github.ref, 'refs/tags/v')
run: |
REV=$(git describe --long --tags --match='v*' --dirty 2>/dev/null || git rev-list -n1 HEAD)

@vishesh92 vishesh92 Nov 4, 2025

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Can we move all these params to a single step? this will make the github actions cleaner.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
vishesh92
vishesh92 reviewed Nov 4, 2025
View reviewed changes
Comment thread .github/workflows/release.yaml Outdated
LDFLAGS="-s -w -X ${PKG}/pkg/driver.driverVersion=${REV} -X ${PKG}/pkg/driver.gitCommit=${GIT_COMMIT} -X ${PKG}/pkg/driver.buildDate=${BUILD_DATE}"

mkdir -p bin
CGO_ENABLED=0 go build -ldflags "${LDFLAGS}" -o ./bin/cloudstack-csi-sc-syncer ./cmd/cloudstack-csi-sc-syncer

@vishesh92 vishesh92 Nov 4, 2025

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Would this build binaries for all architectures?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@Pearl1594
extract common bits and build arm / amd syncer binaries
e32c051
weizhouapache
weizhouapache approved these changes Nov 4, 2025
View reviewed changes

@weizhouapache weizhouapache left a comment

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

lgtm

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
vishesh92
vishesh92 approved these changes Nov 4, 2025
View reviewed changes

@vishesh92 vishesh92 left a comment

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

clgtm

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@Pearl1594
update alpine version used
9be27d7
@Pearl1594
Pearl1594 merged commit 701bc2f into main Nov 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@kiranchavala kiranchavala kiranchavala approved these changes
@vishesh92 vishesh92 vishesh92 approved these changes
+1 more reviewer
@weizhouapache weizhouapache weizhouapache approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@Pearl1594 @kiranchavala @vishesh92 @weizhouapache
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.