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

Switch kernel build to linuxkit pkg #2583

Open
ijc opened this issue Oct 10, 2017 · 8 comments
Open

Switch kernel build to linuxkit pkg #2583

ijc opened this issue Oct 10, 2017 · 8 comments

Comments

@ijc
Copy link
Contributor

ijc commented Oct 10, 2017

Once moby/moby#2578 is put to bed I'd like to convert the kernel build over.

Looking at kernel/Makefile I can see 4 possible issues which may need enhancements to linuxkit pkg.

  1. Templating to handle multiple kernel versions. I think this will be fine, it'll be multiple invocations of linuxkit pkg against either a statically or automatically generated build-foo.yml.
  2. The use of --build-arg. Should be simple enough to add a similar argument to linuxkit pkg build (and push) which is propagated to the build.
  3. Invokes docker push and push-manifest more than once. This is because it builds and pushes linuxkit/kernel:X.Y.Z$(EXTRA)-$(TAG) by default (X.Y.Z is the kernel version, $(EXTRA) is an optional per image suffix like -dbg, $(TAG) is the usual tree-sh and -dirty suffix) but also tags and pushes linuxkit/kernel:X.Y.Z$(EXTRA) (no hash). This differs from the usual pattern of linuxkit/pkg:$(TAG). For this we could add a new field to the build.yml called tag: foo where foo here would be X.Y.Z$(EXTRA) (per-kernel). If this field is set then the build/push will be of both foo-$(TAG) and foo (but not the default $(TAG). Other names such as alias or prefix might be tempting but don't seem to match the semantics (in particular because the "default" name is not used so it is neither an alias nor is it strictly a prefix).
  4. Secondary builds (perf, zfs) which use the just-now-built kernel image. I think the build-arg stuff above handles that?

/cc @rn any of that sound insane? Any other wrinkles with the kernel build I've missed?

@rn
Copy link
Member

rn commented Oct 10, 2017

  • I think having tag: foo in the build.yml is fine. if present we should push to <org>/<img>:<tag> and <org>/<img>:<tag>-<hash>
  • Specifying --build-arg in the build.yml would be good, I think. I prefer if I only have to change the kernel version in two places (currently Makefile and kernel-config-X). If this is an argument to linuxkit and is in build.yml it'll be three files.
  • The secondary builds are tricky as they require the hash from the main kernel build. There is added complication as one can't do docker build with content trust where the FROM line comes from a build arg. It's a bug, so we do a trusted pull first (which weirdly works). This will be tricky/special case for linuxkit pkg. See big comment just above build_perf in Makefile

@ijc
Copy link
Contributor Author

ijc commented Oct 10, 2017

  • I think having tag: foo in the build.yml is fine. if present we should push to /: and /:-

👍

  • Specifying --build-arg in the build.yml would be good, I think. I prefer if I only have to change the kernel version in two places (currently Makefile and kernel-config-X). If this is an argument to linuxkit and is in build.yml it'll be three files.

Good idea, lets have the tool read an array of build args from a yml key (lets call it build-args).

That seems less open to abuse or mistakes (such as not putting the command invocation line in git and therefore missing it in the hash) than a CLI option, although maybe we will eventually find a use case for that too.

  • The secondary builds are tricky as they require the hash from the main kernel build. There is added complication as one can't do docker build with content trust where the FROM line comes from a build arg. It's a bug, so we do a trusted pull first (which weirdly works). This will be tricky/special case for linuxkit pkg. See big comment just above build_perf in Makefile

Will have to think on this one some more. Perhaps if I do so for long enough the bug will be fixed ;-) (is there an issue on moby/moby for it?),

Perhaps we can just generate Dockerfile.perf from a template with sed?

@rn
Copy link
Member

rn commented Oct 10, 2017

@ijc
Copy link
Contributor Author

ijc commented Oct 11, 2017

It is valid to do e.g. COPY --from=docker:17.06.0-ce rather than having to do FROM docker:17.06.0-ce AS docker then COPY --from=docker. Unfortunately COPY --from=${IMAGE} doesn't work:

Step 5/5 : COPY --from=${IMAGE} /perf /usr/bin/perf
invalid from flag value ${IMAGE}: invalid reference format: repository name must be lowercase
Makefile:175: recipe for target 'build_perf_4.9.x' failed

Which is a shame. Even if it did work I expect there would be some chance it would also be incompatible with DCT.

@rn
Copy link
Member

rn commented Aug 13, 2018

Taking a stab at this. One other problem with the kernel build, not yet mentioned above, is also that we currently have to rebuild all kernels even if we only change a subset of the kernels because the git tree hash changes for all kernels. Given the number of kernels we have, this is getting a bit tedious as well.

We already have added extra-sources addition to linuxkit pkg (via moby/moby#3108) which makes it easier to share some of the common files between kernels,

So here is a proposal for a new directory layout. Basically every linuxkit package built as part of the kernel build gets its own directory and some shared files are in the top level directory. Something like this:

├── 4.14.x
│   ├── build.yml
│   ├── config.aarch64
│   ├── config.aarch64-rt
│   ├── config.s390x
│   ├── config.x86_64
│   ├── config.x86_64-rt
│   └── patches
│       ├── 0001-NVDIMM-reducded-ND_MIN_NAMESPACE_SIZE-from-4MB-to-4K.patch
...
│       └── 0021-scsi-storvsc-Avoid-excessive-host-scan-on-controller.patch
├── 4.14.x-perf
│   └── build.yml
├── 4.14.x-perf
│   └── build.yml
├── 4.14.x-dbg-perf
│   └── build.yml
├── 4.14.x-zfs
│   └── build.yml
├── 4.14.x-rt
│   ├── build.yml
│   └── patches
│       ├── 0001-rtmutex-Make-rt_mutex_futex_unlock-safe-for-irq-off-.patch
...
│       └── 0414-Linux-4.14.59-rt37-REBASE.patch
├── 4.17.x
│   ├── build.yml
│   ├── config.aarch64
│   ├── config.s390x
│   └── config.x86_64
├── 4.17.x-dbg
│   └── build.yml
├── 4.4.x
│   ├── build.yml
│   └── config.x86_64
├── 4.9.x
│   ├── build.yml
│   ├── config.x86_64
│   └── patches
│       ├── 0001-tools-build-Add-test-for-sched_getcpu.patch
...
│       └── 0012-vmbus-dynamically-enqueue-dequeue-the-channel-on-vmb.patch
├── Dockerfile
├── Dockerfile.kconfig
├── Dockerfile.perf
├── Dockerfile.zfs
├── Makefile
├── config.dbg
├── keys.asc
└── ucode
    ├── intel-ucode-license.txt
    └── intel-ucode-md5sums

For this to work we need a few additional improvements to linuxkit pkg:

  • Extend extra-sources. If the src path of the extra source is a file calculate the sha1 of the file content and only use the git tree hash if src is a directory. This allows us to share the Dockerfile (and other files) between the different kernel versions without affecting the hash if the file does not change.
  • Add build-args to the build.yml schema. This will be a list of strings with each string arg=value. These are passed to the docker build as --build-arg. This will allow re-using the Dockerfile for different kernel builds, very much like what we do today in the Makefile.
  • Add extra-tag to the build.yml schema (as suggested above). With extra-tag: foo this will tag/push and image with foo-<hash> as well as foo.
  • Extend the build-args, introduced above to use a special value, something like KERNEL_TAG={../4.14.x} which would replace {../4.14.x} with the output of linuxkit pkg show-tag. This will allow us to build dependent packages such as perf or zfs.

With the above 4.14.x/build.yml would look something like this (might change the paths in the target build context):

image: kernel
extra-tag: 4.14.61
extra-sources:
  - ../Dockerfile:/Dockerfile
  - ../keys.asc:/keys.asc
build-args:
  - KERNEL_VERSION=4.14.61
  - KERNEL_SERIES=4.14.x

And 4.14.x-dbg/build.yml something like this:

image: kernel
extra-tag: 4.14.61-dbg
extra-sources:
  - ../Dockerfile:/Dockerfile
  - ../keys.asc:/keys.asc
  - ../4.14.x:/
  - ../config.dbg:/config-dbg
build-args:
  - KERNEL_VERSION=4.14.61
  - KERNEL_SERIES=4.14.x
  - DEBUG=-dbg

The -rt build file would be similar.

The 4.14.x-perf/build.yml would look something like this:

image: kernel-perf
extra-tag: 4.14.61
extra-sources:
  - ../Dockerfile.perf:/Dockerfile
build-args:
  - KERNEL_IMAGE={../4.14.x}

Note 1: The above layout and proposal now has the same kernel version repeated in quite a few build.yml files, which is a little tedious. But I normally use a script (git grep | xargs sed ...) to update the kernel version and we could have a kernel-update.sh in the ./kernel directory which does the right thing.

Note 2: With the proposal the hash for the -perf (or -zfs) packages will differ from the hash of their corresponding kernel packages, but, while a bit irritating, I don't think we ever use the hash in any YAML files (we just use the kernel version) so I don't think this is an issue.

@ijc it would good if you could comment on this proposal.

@ijc
Copy link
Contributor Author

ijc commented Aug 14, 2018

Overall looks good.

The repetition of the versions you mention could be resolved by having e.g. 4.14.x/{kernel,dbg,perf} instead of ``4.14.x{,-dbg,-perf}. Or you could have them all in the same dir but differently named build.yml` files so you would use `linuxklit pkg build --build-yml=build-perf.yml` to build the variants. Might also allow you to have the same hashes?

The Extend the build-args, introduced above to use a special value thing seems a bit too special to me. Is that just for the FROM used in the derivative image? Perhaps a distinct parent-images section in the yaml would be less magic? i.e. a map from a build arg name to an image name which then gets set much as you proposed (it's just now more distinct from a regular build-arg and also more targetted).

I wonder if adding support for multi-stage builds to build.yml (i.e. a target field) would help you here at all, allowing you to combine into a single Dockerfile are reducing the need for cross play between different dockerfiles?

@rn
Copy link
Member

rn commented Aug 14, 2018

The repetition of the versions you mention could be resolved by having e.g. 4.14.x/{kernel,dbg,perf} instead of ``4.14.x{,-dbg,-perf}. Or you could have them all in the same dir but differently namedbuild.ymlfiles so you would uselinuxklit pkg build --build-yml=build-perf.yml` to build the variants. Might also allow you to have the same hashes?

I think this is a good idea. Same dir, different build.yml. They will still have different hashes as the extra-sources will be different.

The Extend the build-args, introduced above to use a special value thing seems a bit too special to me. Is that just for the FROM used in the derivative image? Perhaps a distinct parent-images section in the yaml would be less magic? i.e. a map from a build arg name to an image name which then gets set much as you proposed (it's just now more distinct from a regular build-arg and also more targetted).

I like this. Definitely better than the special value hack which I wasn't too happy with in the first place.

I wonder if adding support for multi-stage builds to build.yml (i.e. a target field) would help you here at all, allowing you to combine into a single Dockerfile are reducing the need for cross play between different dockerfiles?

That might be a good idea for perf and zfs (and bcc once merged). And I think this might result in most cases for the related packages to have the same hash as well.

@ijc
Copy link
Contributor Author

ijc commented Aug 14, 2018

They will still have different hashes as the extra-sources will be different.

I don't think that has to be the case, it would just mean some of them had some redundant or unnecessary entries, I guess. The multi-stage target thing would have the same effect I guess.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants