moby / buildkit

concurrent, cache-efficient, and Dockerfile-agnostic builder toolkit
https://github.com/moby/moby/issues/34227
Apache License 2.0
8.08k stars 1.14k forks source link

containerd: split runtime options definitions by OS #5056

Closed tonistiigi closed 3 months ago

tonistiigi commented 3 months ago

In https://github.com/moby/buildkit/blob/v0.14.1/cmd/buildkitd/main_containerd_worker.go#L380-L386 we currently import three packages just to get a type definition for runtime options struct. Iiuc runc options can't exist on Windows and Hcsshim options can't exist on Linux. This means we should be able to separate these imports with build tags and reduce binary size.

As a follow-up, it may also make sense to see if we need this at all. Iiuc nothing actually reads these types and they get marshalled into protobuf before sent to containerd. So the intermediate type info probably isn't needed.

cc @profnandaa

profnandaa commented 3 months ago

Yes, on the Windows side, nothing special, just defaults:

   309:         if cfg.Runtime.Name != "" {
   310:                 opts := getRuntimeOptionsType(cfg.Runtime.Name)
   311:
=> 312:                 t, err := toml.TreeFromMap(cfg.Runtime.Options)
   313:                 if err != nil {
   314:                         return nil, errors.Wrapf(err, "failed to parse runtime options config")
   315:                 }
   316:                 err = t.Unmarshal(opts)
   317:                 if err != nil {
(dlv) p opts
interface {}(*github.com/Microsoft/hcsshim/cmd/containerd-shim-runhcs-v1/options.Options) *{
        Debug: false,
        DebugType: Options_NPIPE (0),
        RegistryRoot: "",
        SandboxImage: "",
        SandboxPlatform: "",
        SandboxIsolation: Options_PROCESS (0),
        BootFilesRootPath: "",
        VmProcessorCount: 0,
        VmMemorySizeInMb: 0,
        GPUVHDPath: "",
        ScaleCpuLimitsToSandbox: false,
        DefaultContainerScratchSizeInGb: 0,
        DefaultVmScratchSizeInGb: 0,
        ShareScratch: false,
        NCProxyAddr: "",
        LogLevel: "",
        IoRetryTimeoutInSec: 0,
        DefaultContainerAnnotations: map[string]string nil,
        NoInheritHostTimezone: false,
        ScrubLogs: false,
        XXX_NoUnkeyedLiteral: struct {} {},
        XXX_unrecognized: []uint8 len: 0, cap: 0, nil,
        XXX_sizecache: 0,}

In the meantime, I have just split up the code but going to investigate further if the whole thing is needed at all.

tonistiigi commented 3 months ago

fixed by #5061