feat: support memory/cpu driver options for docker-container

[whatwewant]

fixes #2049

limit builder container memory

use: docker buildx create --driver docker-container --driver-opt memory=32G --name zmicro

[whatwewant]

closes #2049

[jsternberg]

I like the idea and I think these options could be helpful for those using the docker-container driver. I'd like to use a different method for parsing the options though to avoid pulling in new dependencies and to make it more consistent with the other docker products that have the same functionality.

[jsternberg]

I think it might be better if we utilize the equivalent types from the docker create command: https://github.com/docker/cli/blob/0e70f1b7b831565336006298b9443b015c3c87a5/cli/command/container/opts.go#L34-L132

This plugin also has docker/cli as a dependency so we should be able to utilize the github.com/docker/cli/opts package which I think we should use instead so the options here are consistent with the equivalent options.

An alternative is to utilize the method that compose does to parse the options, but it's a bit more adhoc. The underlying implementation seems to be the same, but we may not want to reuse that section of code mostly because it is not easily reusable outside of a YAML context.

[crazy-max]

Agree, let's avoid adding unnecessary dep. opts.MemBytes can be used similar to

buildx/commands/build.go

Line 74 in e018f8b

shmSize dockeropts.MemBytes

[whatwewant]

@jsternberg @crazy-max done, the types have been aligned to github.com/docker/cli/opts, and the unnecessary deps have been removed

[jsternberg]

Matching with my recommendation above, I think we can do this:

if d.memory != "" {
    var memory opts.MemBytes
    if err := memory.Set(d.memory); err != nil {
        return err
    }
    hc.Resources.Memory = int64(memory)
}

[whatwewant]

done

[jsternberg]

I'm hesitant to add the dependencies that are added here since the functionality included in this PR is stuff that's also been done in other docker code. My preference is going to be that we utilize those methods rather than add a new way to parse the units.

The same applies to the other two dependencies here.

If there's a compelling reason why this is brand new functionality for docker and these dependencies make implementing that functionality easier, these dependencies seem solid. My preference is just going to be on not adding dependencies if there's an existing alternative.

[jsternberg]

I think strconv.ParseInt makes more sense here and then we don't need a dependency. The container create command uses the cli flags and that uses strconv.ParseInt internally.

[whatwewant]

@jsternberg done

[jsternberg]

@tonistiigi tagging you since this touches user-facing configuration options so.

[crazy-max]

Needs docs update in https://github.com/docker/buildx/blob/master/docs/reference/buildx_create.md#docker-container-driver-1

And on docs website as well in follow-up https://docs.docker.com/build/drivers/docker-container/#synopsis (cc @dvdksn)

[whatwewant]

Needs docs update in https://github.com/docker/buildx/blob/master/docs/reference/buildx_create.md#docker-container-driver-1

And on docs website as well in follow-up https://docs.docker.com/build/drivers/docker-container/#synopsis (cc @dvdksn)

added @crazy-max @dvdksn

[dvdksn]

(docs) LGTM

[crazy-max]

Can you squash your commits please? Otherwise LGTM

[whatwewant]

@crazy-max something wrong ?

Commits squashed