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

Add option for specifying platform of docker image #12

Merged
merged 2 commits into from
Feb 8, 2022
Merged

Add option for specifying platform of docker image #12

merged 2 commits into from
Feb 8, 2022

Conversation

jeromefroe
Copy link
Contributor

Hi! 👋 Firstly, thank you this awesome package!

This PR adds an option for specifying the platform of the docker image that gets
pulled by the Run function. This option is useful for users with Apple M1 laptops
that need to pull an image that doesn't have support for the linux/arm64 platform.
Instead, such users could pull the standard linux/amd64 image and run it instead
(via Rosetta).

Copy link
Owner

@dhui dhui left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!
There's just one minor issue to address.

if c.PullResp == nil {
return nil, Err
}
if c.Platform != opts.Platform {
Copy link
Owner

Choose a reason for hiding this comment

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

Don't perform this check. The mock is designed to be simple and not contain any logic so the test consumer can easily setup test scenarios.

expectErr bool
}{
{name: "success", client: mockdockerclient.ImageAPIClient{
PullResp: mockdockerclient.MockReadCloser{MockReader: successReader}}, expectErr: false},
{name: "with specific platform", client: mockdockerclient.ImageAPIClient{
Copy link
Owner

Choose a reason for hiding this comment

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

Thanks for adding tests!

Copy link
Owner

@dhui dhui left a comment

Choose a reason for hiding this comment

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

Thanks for addressing the PR feedback!

@dhui dhui merged commit e17f1dd into dhui:master Feb 8, 2022
@jeromefroe jeromefroe deleted the add-option-for-platform-of-image branch February 8, 2022 20:25
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