Add support for outputing multiple windows images from one build #480
Conversation
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
cpuguy83
force-pushed
the
windows_multi
branch
from
7187467 to
0693f94
Compare
MorrisLaw
left a comment
MorrisLaw
left a comment
There was a problem hiding this comment.
Initial pass through. looking good (coming from my somewhat limited perspective)
frontend/build.go
Outdated
| } | ||
|
|
||
| // BuildWithPlatform is a helper function to build a spec with a given platform | ||
| // It takes care of looping through each tarrget platform and executing the build with the platform args substituted in the spec. |
There was a problem hiding this comment.
super nit:
| // It takes care of looping through each tarrget platform and executing the build with the platform args substituted in the spec. | |
| // It takes care of looping through each target platform and executing the build with the platform args substituted in the spec. |
| } | ||
|
|
||
| // in certain conditions we allow input platform to be extended from base image | ||
| if p.OS == "windows" && img.OS == p.OS { |
There was a problem hiding this comment.
The raw string for the OS is added here and also already exists in this file for the windowsAmd64 initialization. Should we make this a constant?
There was a problem hiding this comment.
This is all copied code, I'd rather not modify what doesn't need to be.
| if err := json.Unmarshal(dt, &img); err != nil { | ||
| return nil, nil, errors.Wrap(err, "error unmarshalling base image config") | ||
| if err := json.Unmarshal(cfgs[idx], &img); err != nil { | ||
| return nil, nil, nil, errors.Wrap(err, "error unmarshalling base image config") |
There was a problem hiding this comment.
Are this message and line 245 both true in that they're both unmarshalling base image config?
There was a problem hiding this comment.
Yes, these are both true.
We take the config from the base image and then modify for whatever is in the dalec spec.
This is also returning the original base image config which gets annotated in the image metadata.
cpuguy83
force-pushed
the
windows_multi
branch
from
0693f94 to
22a7e3c
Compare
|
q: why you decided to go for |
Good question. Mostly because I wasn't sure how this would look and trying to decide if it is truly needed in the spec. |
|
That said, I'd like to get the underlying functionality in first, I think, because adding support in the spec is going to be a much broader change. |
This makes it so frontends can have their own args that the dalec core does not need to know about. This also moves the custom args for windowscross local to that implementation. Signed-off-by: Brian Goff <cpuguy83@gmail.com>
This allows the builder to specify a file (either in the main build context or a custom one) which contains a list of base images that the windowscross/container target should produce images for. This is useful for when you want to have one image for different versions of Windows, given that for Windows containers the container OS must match the host OS. Signed-off-by: Brian Goff <cpuguy83@gmail.com>
cpuguy83
force-pushed
the
windows_multi
branch
from
22a7e3c to
f5f9965
Compare
|
I have re-implemented this using just the spec here: #492 |
3 participants
This allows builders to specify a file that contains a list of base images that the windowscross/container target should produce images for.
The result is a single multiplatform image: https://oci.dag.dev/?image=docker.io/cpuguy83/test@sha256:d88cf477b7f2464d4581f9490a8c1946300093461837272537e83879105f55e6&mt=application%2Fvnd.oci.image.index.v1%2Bjson&size=1693