Skip to contentSkip to conversation

fs/aci: Add App Container Image (ACI) support to filesystems

PATCH New Brian 'redbeard' Harrington 2 participants 3 messages May 23, 2016 – Jun 7, 2016 VFS

lore
Share & tools
lore

Brian 'redbeard' Harrington

author
#0 lore raw

PATCH v2 1/1 fs/aci: Add App Container Image (ACI) support to filesystems

App Container Images (ACI) are a Linux containerization image format defined as a part of the AppC specification as defined by CoreOS, Red Hat, Google, and community members. These images consist of a Linux userland and a JSON metadata file describing how to execute the actual runtime.

This filesystem package utilizes Buildroot for the generation of the userland and provides the user with a mechanism for configuring the contents of the manifest file.

---
Changes v1 -> v2
- Remove accidental 'default' of the fs/aci
- Remove dependency on host Jq
- Remove numerous "depends" lines in Kconfig, now in if block
- Set default name of localhost/buildroot as well as error check
- Allowed user to specify external manifest
- Enabled setting of timestamp annotation via a (non default) config option
- Set default version via sha256 of .config file
- Added dependency on specific architectures
- Moved logic for architecture checks into Kconfig
- Use qstrip to clean up variables
- Export variables to scripts in package to avoid numerous re-declarations
- Removed setting of xattrs in tar command
- Removed accidental re-use of TAR_OPTS
- Decoupled initial creation of tarball and adding of additional files to simplify logic
- Removed stray debugging "echo" lines used during development
- Removed extra blank lines
- Moved logic for generation of json manifest into a shell script

Signed-off-by Brian 'redbeard' Harrington <[email protected]>
---
 fs/Config.in       |   1 +
 fs/aci/Config.in   | 117 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/aci/aci.json.sh |  51 +++++++++++++++++++++++
 fs/aci/aci.mk      |  80 ++++++++++++++++++++++++++++++++++++
 4 files changed, 249 insertions(+)
 create mode 100644 fs/aci/Config.in
 create mode 100755 fs/aci/aci.json.sh
 create mode 100644 fs/aci/aci.mk
fs/Config.in +1 -0
index 51ccf28..1c468c5 100644
@@ -1,5 +1,6 @@
 menu "Filesystem images"
 
+source "fs/aci/Config.in"
 source "fs/axfs/Config.in"
 source "fs/cloop/Config.in"
 source "fs/cpio/Config.in"
fs/aci/Config.in new file +117 -0

117 lines not on this page · raw message

fs/aci/aci.json.sh new file +51 -0

51 lines not on this page · raw message

fs/aci/aci.mk new file +80 -0

80 lines not on this page · raw message

Signature · 1 line
2.4.11
#1 lore raw

PATCH v2 1/1 fs/aci: Add App Container Image (ACI) support to filesystems

Hello,

Thanks for following up on this!

Quoted 10 lines · Brian 'redbeard' Harrington

App Container Images (ACI) are a Linux containerization image format defined as a part of the AppC specification as defined by CoreOS, Red Hat, Google, and community members. These images consist of a Linux userland and a JSON metadata file describing how to execute the actual runtime.

This filesystem package utilizes Buildroot for the generation of the userland and provides the user with a mechanism for configuring the contents of the manifest file.

We need your Signed-off-by here (and not after the --- line like you're doing today).

Quoted 51 lines

---
Changes v1 -> v2
- Remove accidental 'default' of the fs/aci
- Remove dependency on host Jq
- Remove numerous "depends" lines in Kconfig, now in if block
- Set default name of localhost/buildroot as well as error check
- Allowed user to specify external manifest
- Enabled setting of timestamp annotation via a (non default) config option
- Set default version via sha256 of .config file
- Added dependency on specific architectures
- Moved logic for architecture checks into Kconfig
- Use qstrip to clean up variables
- Export variables to scripts in package to avoid numerous re-declarations
- Removed setting of xattrs in tar command
- Removed accidental re-use of TAR_OPTS
- Decoupled initial creation of tarball and adding of additional files to simplify logic
- Removed stray debugging "echo" lines used during development
- Removed extra blank lines
- Moved logic for generation of json manifest into a shell script

Signed-off-by Brian 'redbeard' Harrington <[email protected]>
---
 fs/Config.in       |   1 +
 fs/aci/Config.in   | 117 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/aci/aci.json.sh |  51 +++++++++++++++++++++++
 fs/aci/aci.mk      |  80 ++++++++++++++++++++++++++++++++++++
 4 files changed, 249 insertions(+)
 create mode 100644 fs/aci/Config.in
 create mode 100755 fs/aci/aci.json.sh
 create mode 100644 fs/aci/aci.mk
fs/Config.in +1 -0
index 51ccf28..1c468c5 100644
@@ -1,5 +1,6 @@
 menu "Filesystem images"
 
+source "fs/aci/Config.in"
 source "fs/axfs/Config.in"
 source "fs/cloop/Config.in"
 source "fs/cpio/Config.in"
fs/aci/Config.in new file +0 -0
new file mode 100644
index 0000000..4df4791

fs/aci/Config.in line 3

Quoted 3 lines
fs/aci/Config.in new file +3 -0
@@ -0,0 +1,117 @@
+config BR2_TARGET_ROOTFS_ACI
+	bool "app container image (aci)"
+	default n

This line is not needed, as being disabled is the default state for a bool option.

fs/aci/Config.in line 5

Quoted 2 lines
fs/aci/Config.in new file +2 -0
+        depends on BR2_arm || BR2_armeb || BR2_aarch64_eb || BR2_i386 || BR2_x86_64 || BR2_powerpc64 || BR2_powerpc64le
+        depends on !BR2_ARM_CPU_ARMV4

Wrong indentation for those two lines, it should be indented with a tab.

fs/aci/Config.in line 9

Quoted 4 lines
fs/aci/Config.in new file +4 -0
+	help
+	  Build an App Container Image for use with a Linux
+	  containerization engine like rkt, Kurma, 3ofCoins, nosecone,
+	  etc.

Maybe you could include some easy way to test such image, at least in the commit log?

Quoted 5 lines
fs/aci/Config.in new file +5 -0
+
+if BR2_TARGET_ROOTFS_ACI
+
+config BR2_TARGET_ROOTFS_ACI_ARCH
+	string

fs/aci/Config.in line 24

Quoted 10 lines
fs/aci/Config.in new file +10 -0
+	default "i386"       if BR2_i386
+	default "arm64"      if BR2_x86_64
+	default "aarch64"    if BR2_aarch64
+	default "aarch64_be" if BR2_aarch64_be
+	default "ppc64"      if BR2_powerpc64
+	default "armv6l"     if BR2_arm && BR2_ARM_CPU_ARMV6
+	default "armv7l"     if BR2_arm && BR2_ARM_CPU_ARMV7A
+	default "armv7l"     if BR2_arm && BR2_ARM_CPU_ARMV7M
+	default "armv7b"     if BR2_armeb && BR2_ARM_CPU_ARMV7A
+	default "armv7b"     if BR2_armeb && BR2_ARM_CPU_ARMV7M

I don't think supporting BR2_ARM_CPU_ARMV7M makes sense. I hardly see an ARM noMMU system being used in this context, and I actually doubt it works.

fs/aci/Config.in line 67

Quoted 4 lines
fs/aci/Config.in new file +4 -0
+config BR2_TARGET_ROOTFS_ACI_USE_CUSTOM_MANIFEST
+        bool "custom aci manifest"
+        help
+          Use custom aci manifest.

Trailing spaces.

fs/aci/Config.in line 71

Quoted 4 lines
fs/aci/Config.in new file +4 -0
+
+if BR2_TARGET_ROOTFS_ACI_USE_CUSTOM_MANIFEST
+config BR2_TARGET_ROOTFS_ACI_MANIFEST_PATH
+        string "custom path"

Trailing spaces (please fix globally, there are more occurrences of this).

fs/aci/Config.in line 72

Quoted 1 line
fs/aci/Config.in new file +1 -0
+        default ""

Not needed, being empty is the default for a string.

fs/aci/Config.in line 73

Quoted 1 line
fs/aci/Config.in new file +1 -0
+	depends on BR2_TARGET_ROOTFS_ACI_USE_CUSTOM_MANIFEST

It is useless to have both a "depends on" *and* enclose the option in an "if...endif" block for the same condition. My preference is that when there is a single option that should be conditionally visible, use "depends on". When there's more, use a "if ... endif" block.

Also indentation is wrong.

fs/aci/Config.in line 78

Quoted 5 lines
fs/aci/Config.in new file +5 -0
+        help
+          Path to custom ACI manifest.
+endif # BR2_TARGET_ROOTFS_ACI_USE_CUSTOM_MANIFEST
+
+if !BR2_TARGET_ROOTFS_ACI_OPTIONS_MANIFEST_CUSTOM

Please add an empty linew here.

fs/aci/Config.in line 88

Quoted 10 lines
fs/aci/Config.in new file +10 -0
+config BR2_TARGET_ROOTFS_ACI_OPTIONS_NAME
+	string "name"
+	default "localhost/buildroot"
+	depends on !BR2_TARGET_ROOTFS_ACI_USE_CUSTOM_MANIFEST
+	help
+	  A human-readable name for this App Container Image (e.g. 
+	  example.com/my_app).
+
+comment "aci images must have a name"
+	depends on BR2_TARGET_ROOTFS_ACI_OPTIONS_NAME = "localhost/buildroot"

Why? Isn't this default name correct?

fs/aci/Config.in line 50

Quoted 1 line
fs/aci/Config.in new file +1 -0
+	depends on !BR2_TARGET_ROOTFS_ACI_USE_CUSTOM_MANIFEST

Not needed, you are already in a if ... endif block.

Quoted 8 lines
fs/aci/Config.in new file +8 -0
+
+config BR2_TARGET_ROOTFS_ACI_OPTIONS_EXEC
+	string "exec command"
+	depends on !BR2_TARGET_ROOTFS_ACI_USE_CUSTOM_MANIFEST
+	help
+	  Executable to launch on container instantiation and any relevant flags
+	  (e.g. /bin/sh).
+

fs/aci/Config.in line 107

Quoted 10 lines
fs/aci/Config.in new file +10 -0
+config BR2_TARGET_ROOTFS_ACI_OPTIONS_VERSION
+	string "version"
+	depends on !BR2_TARGET_ROOTFS_ACI_USE_CUSTOM_MANIFEST
+	help
+	  When combined with "name", this SHOULD be unique for every 
+	  build of an app (on a given "os"/"arch" combination - e.g. v1.3.2).
+
+comment "aci images should be supplied with a version"
+	depends on BR2_TARGET_ROOTFS_ACI_OPTIONS_VERSION = ""
+	depends on !BR2_TARGET_ROOTFS_ACI_USE_CUSTOM_MANIFEST

Not needed, you're already in a if .. endif block for the same condition.

fs/aci/Config.in line 111

Quoted 4 lines
fs/aci/Config.in new file +4 -0
+
+config BR2_TARGET_ROOTFS_ACI_TIMESTAMP
+	bool "include created timestamp"
+	default n

Not needed.

fs/aci/Config.in line 73

Quoted 1 line
fs/aci/Config.in new file +1 -0
+	depends on !BR2_TARGET_ROOTFS_ACI_USE_CUSTOM_MANIFEST

Ditto.

fs/aci/Config.in line 75

Quoted 2 lines
fs/aci/Config.in new file +2 -0
+	help
+	  Include ACI annotation of timestamp when make was run.

I am not sure this option is needed. Just always include a timestamp for now. We can revisit than when/if we implement reproducible builds.

fs/aci/aci.json.sh line 2

Quoted 11 lines
fs/aci/Config.in new file +3 -0
+
+endif # !BR2_TARGET_ROOTFS_ACI_OPTIONS_MANIFEST_CUSTOM
+endif # BR2_TARGET_ROOTFS_ACI
fs/aci/aci.json.sh new file +2 -0
new file mode 100755
index 0000000..560e062
@@ -0,0 +1,51 @@
+#!/bin/sh
+if [ "${ROOTFS_ACI_EXEC}n" == "n" ]; then

You can test if a string is empty rather than doing this.

if test -z "${ROOTFS_ACI_EXEC}" ; then

	...

fs/aci/aci.json.sh line 10

Quoted 8 lines
fs/aci/aci.json.sh new file +8 -0
+	aciuser='"user": "0",'
+	acigroup='"group": "0"'
+else
+	aciuser='"user": '"\"${ROOTFS_ACI_EXEC}\","
+	acigroup='"group": '"\"${ROOTFS_ACI_EXEC}\""
+fi
+
+if [ "${ROOTFS_ACI_TIMESTAMP}n" != "n" ]; then

Ditto.

Quoted 31 lines
fs/aci/aci.json.sh new file +31 -0
+createdat=",
+	{
+		\"name\": \"created\",
+		\"value\": \"${ROOTFS_ACI_TIMESTAMP}\"
+	}"
+fi
+
+
+cat <<EOF
+{
+	"acKind":"ImageManifest",
+	"acVersion":"0.7.4",
+	"name": "${ROOTFS_ACI_NAME}",
+	"labels":
+		[{
+			"name":"os",
+			"value":"linux"
+		},{
+			"name":"arch",
+			"value": "${ROOTFS_ACI_ARCH}"
+		},{ 
+			"name":"version",
+			"value": "${ROOTFS_ACI_VERSION}"
+		}],
+	"app":
+	{
+		"exec":
+			[
+			"${ROOTFS_ACI_EXEC}"
+			],
+		${aciuser}

fs/aci/aci.json.sh line 51

Quoted 10 lines
fs/aci/aci.json.sh new file +10 -0
+		${acigroup}
+	},
+	"annotations":
+	[{
+		"name": "buildroot.org/aci",
+		"value": "Generated by Buildroot"
+	}${createdat}
+	]
+}
+EOF

I must say I would probably prefer a template json file, and have the .mk file simply SED some values in the template.

Quoted 17 lines
fs/aci/aci.mk new file +11 -0
new file mode 100644
index 0000000..abec378
@@ -0,0 +1,80 @@
+################################################################################
+#
+# Generate App Container Image (ACI)
+# https://github.com/appc/spec/blob/master/spec/aci.md
+#
+################################################################################
+
+ROOTFS_ACI_NAME = $(call qstrip,$(BR2_TARGET_ROOTFS_ACI_OPTIONS_NAME))
+ROOTFS_ACI_EXEC = $(call qstrip,$(BR2_TARGET_ROOTFS_ACI_OPTIONS_EXEC))
+ROOTFS_ACI_ARCH = $(call qstrip,$(BR2_TARGET_ROOTFS_ACI_ARCH))
+ROOTFS_ACI_VERSION = $(call qstrip,$(BR2_TARGET_ROOTFS_ACI_OPTIONS_VERSION))

fs/aci/aci.mk line 21

Quoted 10 lines
fs/aci/aci.mk new file +10 -0
+ROOTFS_ACI_MANIFEST = $(call qstrip,$(BR2_TARGET_ROOTFS_ACI_MANIFEST_PATH))
+
+# Passing the generated variables into the running environment for make targets
+# leads to a cleaner implementation of the resulting shell scripts. In this way
+# nothing getting passed to tar should need quotes
+export ROOTFS_ACI_NAME
+export ROOTFS_ACI_EXEC 
+export ROOTFS_ACI_ARCH
+export ROOTFS_ACI_VERSION 
+export ROOTFS_ACI_MANIFEST

I dislike this. Please pass the variables that are needed in the environment.

fs/aci/aci.mk line 26

Quoted 4 lines
fs/aci/aci.mk new file +4 -0
+# If the user does not supply a version number, generate one which should play
+# nicely with reproducible builds, ideally signalling that for a given pairing
+# of buildroot version and config generates a consistent output.
+ifndef $(ROOTFS_ACI_VERSION)

Please use:

ifeq ($(ROOTFS_ACI_VERSION),)

fs/aci/aci.mk line 27

Quoted 2 lines
fs/aci/aci.mk new file +2 -0
+ROOTFS_ACI_VERSION = $(shell cat .config | sha256sum | cut -f1 -d' ')
+endif

Then the Config.in comment about the version being mandatory should be removed, because you generate a version number dynamically.

fs/aci/aci.mk line 35

Quoted 7 lines
fs/aci/aci.mk new file +7 -0
+
+# In continuing support of deterministic builds, allow the user to annotate a
+# manifest with a timestamp but do not make this the default behavior.  In this
+# case it will be the timestamp of when "make" is run.
+ifeq ($(BR2_TARGET_ROOTFS_ACI_TIMESTAMP),y)
+ROOTFS_ACI_TIMESTAMP = $(shell date --rfc-3339=ns | tr " " "T")
+export ROOTFS_ACI_TIMESTAMP

Argh, don't expert variabls.

fs/aci/aci.mk line 48

Quoted 13 lines
fs/aci/aci.mk new file +13 -0
+endif
+
+# For the generation of our ACI we pass the variables from the main
+# configuration into a scrip to generate a JSON manifest unless the user
+# provided their own manifest
+ifneq ($(ROOTFS_ACI_MANIFEST),)
+define ROOTFS_ACI_COPY_MANIFEST
+	cp '$(ROOTFS_ACI_MANIFEST)' $(BINARIES_DIR)/manifest
+endef
+ROOTFS_ACI_PRE_GEN_HOOKS += ROOTFS_ACI_COPY_MANIFEST
+else
+define ROOTFS_ACI_GENERATE_MANIFEST
+	fs/aci/aci.json.sh > $(BINARIES_DIR)/manifest

That's where the SED magic should be done. And you could also maybe do it when a custom manifest is passed. See in fs/ubifs/ubi.mk what we're doing with ubinize.cfg for an example.

fs/aci/aci.mk

Quoted 7 lines
fs/aci/aci.mk new file +7 -0
+endef
+ROOTFS_ACI_PRE_GEN_HOOKS += ROOTFS_ACI_GENERATE_MANIFEST
+endif
+
+# We then create the TAR archive of all components, making sure all paths are
+# represented correctly. Unfortunately as per fs/common.mk:8 the only a single
+# command is allowed.

Maybe this needs to be fixed?

fs/aci/aci.mk line 57

Quoted 3 lines
fs/aci/aci.mk new file +2 -0
 The spec requires storing any relevant xattrs but as of 
+# 2016-05-22 buildroot does not generate these, though tooling inside the
+# target system can be added via the package BR2_PACKAGE_ATTR

I dislike the comments that mention a specific date.

fs/aci/aci.mk line 67

Quoted 10 lines
fs/aci/aci.mk new file +10 -0
+define ROOTFS_ACI_CMD
+	tar -cf $@ -C $(BINARIES_DIR) --owner=0 --group=0 manifest && \
+	tar -rf $@ -C $(TARGET_DIR) --numeric-owner --transform 's,^\./,rootfs/,' .
+endef
+
+# ACI images are required to end in ".aci" regardless of compression
+ifneq ($(BR2_TARGET_ROOTFS_ACI_NONE),y)
+define ROOTFS_ACI_MOVE
+	mv -f $(BINARIES_DIR)/rootfs.aci$(ROOTFS_ACI_COMPRESS_EXT) \
+		$(BINARIES_DIR)/rootfs.aci

Isn't a link more appropriate here?

fs/aci/aci.mk line 76

Quoted 9 lines
fs/aci/aci.mk new file +9 -0
+endef
+
+ROOTFS_ACI_POST_GEN_HOOKS += ROOTFS_ACI_MOVE
+endif
+
+ifeq ($(ROOTFS_ACI_NAME),)
+ifeq ($(call qstrip,$(ROOTFS_ACI_MANIFEST)),)
+$(error You supplied neither BR2_TARGET_ROOTFS_ACI_MANIFEST_PATH, \
+ROOTFS_ACI_NAME, nor BR2_TARGET_ROOTFS_ACI_OPTIONS_NAME)

It's weird to have BR2_... and ROOTFS_ACI_... options in the same error message. Doesn't seem good.

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#2 lore raw

PATCH v2 1/1 fs/aci: Add App Container Image (ACI) support to filesystems

In reply to Brian 'redbeard' Harrington

Hello,

fs/aci/aci.mk

Quoted 3 lines · Brian 'redbeard' Harrington
fs/aci/aci.mk new file +3 -0
+# We then create the TAR archive of all components, making sure all paths are
+# represented correctly. Unfortunately as per fs/common.mk:8 the only a single
+# command is allowed.

FYI, as of a few minutes ago, this limitation has been lifted, thanks to commit
https://git.busybox.net/buildroot/commit/?id=6dd7bbb59134799ed3d7343f238b3b02592faebf
from Yann.

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com