-
Notifications
You must be signed in to change notification settings - Fork 206
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
Nydusify: ignore arch for single manifest #359
Conversation
de8842e
to
72081b6
Compare
@@ -118,7 +118,7 @@ func (parser *Parser) pullIndex(ctx context.Context, desc *ocispec.Descriptor) ( | |||
} | |||
|
|||
func (parser *Parser) parseImage( | |||
ctx context.Context, desc *ocispec.Descriptor, onlyManifest *ocispec.Manifest, | |||
ctx context.Context, desc *ocispec.Descriptor, onlyManifest *ocispec.Manifest, ignoreArch bool, |
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.
There is no way for nydusify
to specify ignoreArch
?
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.
How is it going to be used? Please add an example in the docs.
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.
Updated comment. No way to specify. For a single manifest image, it's an automatic behavior to ignore the image arch for ease of use, attempt to continue converting, and give a warning message. For the manifest list, nydusify selects host specified arch to convert, but the user still can specify an interesting platform (by --platform), the ultimate goal is that nydusify/nydus-image can convert any arch's image on any arch's host.
72081b6
to
c4c7909
Compare
For a single manifest image, we just ignore the arch, so that allowing to do a default conversion on a different arch's host, for example converting an arm64 image on an amd64 host. Signed-off-by: Yan Song <[email protected]>
c4c7909
to
fc07aa5
Compare
} | ||
|
||
// Just give user a simple hint telling option was ignored. | ||
if config.Architecture != parser.interestedArch { | ||
return nil, errors.Errorf("Specified %s architecture was not found", parser.interestedArch) | ||
err = errors.Errorf("Found arch %s, but the specified target arch (--platform) is %s", config.Architecture, parser.interestedArch) |
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.
Should this be move into else{}
branch?
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 same error message should be returned if no ignoreArch.
For a single manifest image, we just ignore the arch, so that allowing
to do a default conversion on a different arch's host, for example
converting an arm64 image on an amd64 host.
Signed-off-by: Yan Song [email protected]