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

feat: support nydusify for build and push #128

Merged
merged 1 commit into from
Mar 19, 2025
Merged

feat: support nydusify for build and push #128

merged 1 commit into from
Mar 19, 2025

Conversation

chlins
Copy link
Contributor

@chlins chlins commented Mar 18, 2025

This pull request introduces an experimental feature to "nydusify" model artifacts in both the build and push processes. The changes include updates to the configurations, command-line flags, and the implementation of the Nydusify function.

New Feature: Nydusify Model Artifacts

  • Added a new Nydusify flag to the build and push configurations in pkg/config/build.go and pkg/config/push.go respectively. This flag is used to determine whether to nydusify the model artifact. [1] [2]
  • Updated the init functions in cmd/build.go and cmd/push.go to include the new Nydusify flag and mark it as hidden. [1] [2]
  • Implemented the Nydusify function in pkg/backend/nydusify.go, which converts a given image to a Nydus image using the nydusify command.

Build and Push Process Updates

  • Modified the Build and Push methods in pkg/backend/build.go and pkg/backend/push.go to call the Nydusify function if the Nydusify flag is set. [1] [2]
  • Added validation to ensure that the Nydusify flag can only be used when the OutputRemote flag is also set in the build configuration.

aftersnow
aftersnow previously approved these changes Mar 18, 2025
@chlins chlins force-pushed the feat/nydusify branch 2 times, most recently from d2d4dc8 to 70507e8 Compare March 19, 2025 02:37
Signed-off-by: chlins <chlins.zhang@gmail.com>
Copy link
Contributor

@aftersnow aftersnow left a comment

Choose a reason for hiding this comment

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

lgtm

@chlins chlins merged commit 426fea3 into main Mar 19, 2025
5 checks passed
@chlins chlins deleted the feat/nydusify branch March 19, 2025 03:20
@@ -73,5 +75,16 @@ func runBuild(ctx context.Context, workDir string) error {
}

fmt.Printf("Successfully built model artifact: %s\n", buildConfig.Target)

// nydusify the model artifact if needed.
if buildConfig.Nydusify {
Copy link

Choose a reason for hiding this comment

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

The invocation of nydusify depends on the image that has already been pushed to the image registry using modctl push. Therefore, calling nydusify directly without executing modctl push first is meaningless.

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.

None yet

3 participants