| Message ID | 20230512014056.2107657-1-christian@aperture.us |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [v1,1/1] package/crio: new package | expand |
Hello Christian, Thanks for the patch. See below some review. On Thu, 11 May 2023 18:40:56 -0700 Christian Stewart via buildroot <buildroot@buildroot.org> wrote: > package/Config.in | 1 + > package/crio/Config.in | 54 ++++++++++++++++++++++++++++ > package/crio/crio.hash | 3 ++ > package/crio/crio.mk | 82 ++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 140 insertions(+) Entry in DEVELOPERS file missing. > diff --git a/package/crio/Config.in b/package/crio/Config.in > new file mode 100644 > index 0000000000..35a38c587e > --- /dev/null > +++ b/package/crio/Config.in > @@ -0,0 +1,54 @@ > +config BR2_PACKAGE_CRIO > + bool "crio" > + depends on BR2_PACKAGE_HOST_GO_TARGET_ARCH_SUPPORTS > + depends on BR2_PACKAGE_HOST_GO_TARGET_CGO_LINKING_SUPPORTS > + depends on BR2_TOOLCHAIN_HAS_THREADS > + depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_4 # iproute2, __kernel_{u,}long_t > + depends on !BR2_TOOLCHAIN_USES_UCLIBC # no fexecve uClibc has fexecve() now > + depends on BR2_USE_MMU # libgpgme, iproute2, fork() depends on BR2_PACKAGE_LIBGPG_ERROR_ARCH_SUPPORTS # libgpgme is missing > + select BR2_PACKAGE_IPROUTE2 > + select BR2_PACKAGE_IPTABLES Neither of these are referenced in the .mk file. Are these runtime dependencies? If so, please indicate this via a comment. For this kind of package, a runtime test in support/testing/ would be very good. You can ask Julien Olivain for help here :-) > +config BR2_PACKAGE_CRIO_DRIVER_BTRFS > + bool "btrfs filesystem driver" > + depends on BR2_USE_MMU # btrfs-progs > + depends on BR2_TOOLCHAIN_HAS_THREADS # btrfs-progs > + select BR2_PACKAGE_BTRFS_PROGS > + help > + Build the btrfs filesystem driver. Config.in comment missing. > + > +config BR2_PACKAGE_CRIO_DRIVER_DEVICEMAPPER > + bool "devicemapper filesystem driver" > + depends on BR2_TOOLCHAIN_HAS_THREADS # lvm2 > + depends on BR2_USE_MMU # lvm2 > + depends on !BR2_STATIC_LIBS # lvm2 > + select BR2_PACKAGE_LVM2 > + help > + Build the devicemapper filesystem driver. Ditto. > + > +config BR2_PACKAGE_CRIO_DRIVER_OSTREE > + bool "ostree storage driver" > + depends on BR2_PACKAGE_LIBGPG_ERROR_ARCH_SUPPORTS # libostree, libgpgme, libgpg-error > + depends on BR2_TOOLCHAIN_HAS_THREADS # libostree, libglib2 > + depends on BR2_USE_WCHAR # libostree, libglib2 > + depends on BR2_USE_MMU # libostree, e2fsprogs, libglib2, libgpgme Don't need to duplicate the whole set of comments, it can be just: depends on .... # libostree because all what you select is libostree > + # doesn't build with musl due to lack of TEMP_FAILURE_RETRY() don't need to replicate this comment here > + depends on !BR2_TOOLCHAIN_USES_MUSL # libostree This is enough. > + select BR2_PACKAGE_LIBOSTREE > + help > + Build the ostree storage driver. Config.in comment needed. > + > +endif > + > +comment "crio needs a glibc or musl toolchain w/ threads" > + depends on BR2_PACKAGE_HOST_GO_TARGET_ARCH_SUPPORTS && \ > + BR2_PACKAGE_HOST_GO_TARGET_CGO_LINKING_SUPPORTS > + depends on !BR2_TOOLCHAIN_HAS_THREADS || BR2_TOOLCHAIN_USES_UCLIBC Needs to be adjusted as per the above comments. > +################################################################################ > +# > +# crio > +# > +################################################################################ > + > +CRIO_VERSION = 1.27.0 > +CRIO_SITE = $(call github,cri-o,cri-o,v$(CRIO_VERSION)) > +CRIO_LICENSE = Apache-2.0 > +CRIO_LICENSE_FILES = LICENSE > + > +CRIO_CPE_ID_VENDOR = kubernetes > +CRIO_CPE_ID_PRODUCT = cri-o > + > +CRIO_BUILD_TARGETS = cmd/crio cmd/crio-status > +CRIO_DEPENDENCIES += libgpgme Changed += to = here. > +define CRIO_BUILD_PINNS > + $(TARGET_MAKE_ENV) $(MAKE) CC="$(TARGET_CC)" CFLAGS="$(TARGET_CFLAGS)" \ > + LDFLAGS="$(TARGET_LDFLAGS)" STRIP="$(TARGET_STRIP)" \ Would $(TARGET_CONFIGURE_OPTS) work here to replace CC, CFLAGS, LDFLAGS and STRIP? > + -C $(@D)/pinns ../bin/pinns > +endef > +CRIO_POST_BUILD_HOOKS += CRIO_BUILD_PINNS Perhaps a short comment above define CRIO_BUILD_PINNS to explain why it is needed, and not done by the normal build process? > + > +define CRIO_INSTALL_TARGET_CMDS > + $(HOST_GO_COMMON_ENV) $(TARGET_MAKE_ENV) $(MAKE) -C $(@D) \ > + DESTDIR=$(TARGET_DIR) PREFIX=$(TARGET_DIR)/usr ETCDIR=$(TARGET_DIR)/etc \ > + OPT_CNI_BIN_DIR=$(TARGET_DIR)/opt/cni/bin install.bin-nobuild There's no proper installation step with the Go build infrastructure? > + $(INSTALL) -d -m 700 $(TARGET_DIR)/etc/cni > + $(INSTALL) -d -m 700 $(TARGET_DIR)/etc/cni/net.d These two lines are not needed. Intermediate directories are created... > + $(INSTALL) -D -m 644 $(@D)/contrib/cni/10-crio-bridge.conflist \ > + $(TARGET_DIR)/etc/cni/net.d/10-crio-bridge.conflist ... by the -D option here. > +endef > + > +define CRIO_INSTALL_INIT_SYSTEMD > + $(HOST_GO_COMMON_ENV) $(TARGET_MAKE_ENV) $(MAKE) -C $(@D) \ > + DESTDIR=$(TARGET_DIR) PREFIX=$(TARGET_DIR)/usr ETCDIR=$(TARGET_DIR)/etc \ > + OPT_CNI_BIN_DIR=$(TARGET_DIR)/opt/cni/bin install.systemd > + $(SED) 's,/usr/local/bin,/usr/bin,g' \ > + $(TARGET_DIR)/usr/lib/systemd/system/{crio,crio-wipe}.service Meh, the service files are incorrectly generated during the build? Or are they not generated and simply contain values that need to manually be tweaked? Could you rework your patch to address the above suggestions? Thanks a lot! Thomas
Hi Thomas, Christian, All, On 01/08/2023 00:38, Thomas Petazzoni wrote: > Hello Christian, > > Thanks for the patch. See below some review. > > On Thu, 11 May 2023 18:40:56 -0700 > Christian Stewart via buildroot <buildroot@buildroot.org> wrote: > >> package/Config.in | 1 + >> package/crio/Config.in | 54 ++++++++++++++++++++++++++++ >> package/crio/crio.hash | 3 ++ >> package/crio/crio.mk | 82 >> ++++++++++++++++++++++++++++++++++++++++++ >> 4 files changed, 140 insertions(+) > > Entry in DEVELOPERS file missing. > >> diff --git a/package/crio/Config.in b/package/crio/Config.in >> new file mode 100644 >> index 0000000000..35a38c587e >> --- /dev/null >> +++ b/package/crio/Config.in >> @@ -0,0 +1,54 @@ >> +config BR2_PACKAGE_CRIO >> + bool "crio" >> + depends on BR2_PACKAGE_HOST_GO_TARGET_ARCH_SUPPORTS >> + depends on BR2_PACKAGE_HOST_GO_TARGET_CGO_LINKING_SUPPORTS >> + depends on BR2_TOOLCHAIN_HAS_THREADS >> + depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_4 # iproute2, >> __kernel_{u,}long_t >> + depends on !BR2_TOOLCHAIN_USES_UCLIBC # no fexecve > > uClibc has fexecve() now > >> + depends on BR2_USE_MMU # libgpgme, iproute2, fork() > > depends on BR2_PACKAGE_LIBGPG_ERROR_ARCH_SUPPORTS # libgpgme > > is missing > >> + select BR2_PACKAGE_IPROUTE2 >> + select BR2_PACKAGE_IPTABLES > > Neither of these are referenced in the .mk file. Are these runtime > dependencies? If so, please indicate this via a comment. > > For this kind of package, a runtime test in support/testing/ would be > very good. You can ask Julien Olivain for help here :-) I will be glad to help! For a good runtime test, we need: 1. a simple reference Buildroot configuration that compiles the package, and 2. a simple reference "known working" sequence that is sufficiently stable in time (to require minimum maintenance after package updates). I was thinking about this CRI-O tutorial: https://github.com/cri-o/cri-o/blob/main/tutorials/crictl.md But it requires crictl, from the cri-tools package: https://github.com/kubernetes-sigs/cri-tools/tree/master Christian: Are you planning to add this package later? Or do you know a better/simpler way to start a small container? Moreover, when I quickly tried this patch, starting from qemu_aarch64_virt_defconfig. (without systemd) I observed the following: I had a build failure due to a missing libseccomp. Maybe a build dependency is missing? Once built, the service failed to start (by running "crio" manually), due to missing runtime dependencies to: conmon, runc, nsenter (from util-linux). Could you check those are mandatory (and add those as runtime dependency), or if they can be disabled by configuration? Once installed, I finally got an error due to the missing /var/lib/crio directory on the target filesystem. Maybe it is missing in CRIO_INSTALL_TARGET_CMDS? Please also check if specific permissions are needed, and define a CRIO_PERMISSIONS if needed. See: https://nightly.buildroot.org/manual.html#generic-package-reference Best regards, Julien.
diff --git a/package/Config.in b/package/Config.in index 420ebaa370..0b9d87470e 100644 --- a/package/Config.in +++ b/package/Config.in @@ -2635,6 +2635,7 @@ menu "System tools" source "package/coreutils/Config.in" source "package/cpulimit/Config.in" source "package/cpuload/Config.in" + source "package/crio/Config.in" source "package/crun/Config.in" source "package/daemon/Config.in" source "package/dc3dd/Config.in" diff --git a/package/crio/Config.in b/package/crio/Config.in new file mode 100644 index 0000000000..35a38c587e --- /dev/null +++ b/package/crio/Config.in @@ -0,0 +1,54 @@ +config BR2_PACKAGE_CRIO + bool "crio" + depends on BR2_PACKAGE_HOST_GO_TARGET_ARCH_SUPPORTS + depends on BR2_PACKAGE_HOST_GO_TARGET_CGO_LINKING_SUPPORTS + depends on BR2_TOOLCHAIN_HAS_THREADS + depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_4 # iproute2, __kernel_{u,}long_t + depends on !BR2_TOOLCHAIN_USES_UCLIBC # no fexecve + depends on BR2_USE_MMU # libgpgme, iproute2, fork() + select BR2_PACKAGE_IPROUTE2 + select BR2_PACKAGE_IPTABLES + select BR2_PACKAGE_LIBGPGME + help + crio implements the Kubelet Container Runtime Interface (CRI) + using OCI conformant runtimes like runc or podman. + + https://github.com/cri-o/cri-o/ + +if BR2_PACKAGE_CRIO + +config BR2_PACKAGE_CRIO_DRIVER_BTRFS + bool "btrfs filesystem driver" + depends on BR2_USE_MMU # btrfs-progs + depends on BR2_TOOLCHAIN_HAS_THREADS # btrfs-progs + select BR2_PACKAGE_BTRFS_PROGS + help + Build the btrfs filesystem driver. + +config BR2_PACKAGE_CRIO_DRIVER_DEVICEMAPPER + bool "devicemapper filesystem driver" + depends on BR2_TOOLCHAIN_HAS_THREADS # lvm2 + depends on BR2_USE_MMU # lvm2 + depends on !BR2_STATIC_LIBS # lvm2 + select BR2_PACKAGE_LVM2 + help + Build the devicemapper filesystem driver. + +config BR2_PACKAGE_CRIO_DRIVER_OSTREE + bool "ostree storage driver" + depends on BR2_PACKAGE_LIBGPG_ERROR_ARCH_SUPPORTS # libostree, libgpgme, libgpg-error + depends on BR2_TOOLCHAIN_HAS_THREADS # libostree, libglib2 + depends on BR2_USE_WCHAR # libostree, libglib2 + depends on BR2_USE_MMU # libostree, e2fsprogs, libglib2, libgpgme + # doesn't build with musl due to lack of TEMP_FAILURE_RETRY() + depends on !BR2_TOOLCHAIN_USES_MUSL # libostree + select BR2_PACKAGE_LIBOSTREE + help + Build the ostree storage driver. + +endif + +comment "crio needs a glibc or musl toolchain w/ threads" + depends on BR2_PACKAGE_HOST_GO_TARGET_ARCH_SUPPORTS && \ + BR2_PACKAGE_HOST_GO_TARGET_CGO_LINKING_SUPPORTS + depends on !BR2_TOOLCHAIN_HAS_THREADS || BR2_TOOLCHAIN_USES_UCLIBC diff --git a/package/crio/crio.hash b/package/crio/crio.hash new file mode 100644 index 0000000000..2cad8f8bb8 --- /dev/null +++ b/package/crio/crio.hash @@ -0,0 +1,3 @@ +# Locally computed +sha256 8fd7c84ff74eff0a65e090bb9619f36508e461f6925f92a5b8886b759f2347d9 crio-1.27.0.tar.gz +sha256 b40930bbcf80744c86c46a12bc9da056641d722716c378f5659b9e555ef833e1 LICENSE diff --git a/package/crio/crio.mk b/package/crio/crio.mk new file mode 100644 index 0000000000..042c369799 --- /dev/null +++ b/package/crio/crio.mk @@ -0,0 +1,82 @@ +################################################################################ +# +# crio +# +################################################################################ + +CRIO_VERSION = 1.27.0 +CRIO_SITE = $(call github,cri-o,cri-o,v$(CRIO_VERSION)) +CRIO_LICENSE = Apache-2.0 +CRIO_LICENSE_FILES = LICENSE + +CRIO_CPE_ID_VENDOR = kubernetes +CRIO_CPE_ID_PRODUCT = cri-o + +CRIO_BUILD_TARGETS = cmd/crio cmd/crio-status +CRIO_DEPENDENCIES += libgpgme +CRIO_LDFLAGS = \ + -X $(CRIO_GOMOD)/internal/version.Version=$(CRIO_VERSION) +CRIO_TAGS = exclude_graphdriver_zfs + +ifeq ($(BR2_INIT_SYSTEMD),y) +CRIO_TAGS += systemd +endif + +ifeq ($(BR2_PACKAGE_LIBAPPARMOR),y) +CRIO_DEPENDENCIES += libapparmor +CRIO_TAGS += apparmor +endif + +ifeq ($(BR2_PACKAGE_LIBSECCOMP),y) +CRIO_TAGS += seccomp +CRIO_DEPENDENCIES += libseccomp host-pkgconf +endif + +ifeq ($(BR2_PACKAGE_LIBSELINUX),y) +CRIO_TAGS += selinux +CRIO_DEPENDENCIES += libselinux +endif + +ifeq ($(BR2_PACKAGE_CRIO_DRIVER_BTRFS),y) +CRIO_DEPENDENCIES += btrfs-progs +else +CRIO_TAGS += exclude_graphdriver_btrfs +endif + +ifeq ($(BR2_PACKAGE_CRIO_DRIVER_DEVICEMAPPER),y) +CRIO_DEPENDENCIES += lvm2 +else +CRIO_TAGS += exclude_graphdriver_devicemapper +endif + +ifeq ($(BR2_PACKAGE_CRIO_DRIVER_OSTREE),y) +CRIO_DEPENDENCIES += libostree +CRIO_TAGS += ostree +endif + +define CRIO_BUILD_PINNS + $(TARGET_MAKE_ENV) $(MAKE) CC="$(TARGET_CC)" CFLAGS="$(TARGET_CFLAGS)" \ + LDFLAGS="$(TARGET_LDFLAGS)" STRIP="$(TARGET_STRIP)" \ + -C $(@D)/pinns ../bin/pinns +endef +CRIO_POST_BUILD_HOOKS += CRIO_BUILD_PINNS + +define CRIO_INSTALL_TARGET_CMDS + $(HOST_GO_COMMON_ENV) $(TARGET_MAKE_ENV) $(MAKE) -C $(@D) \ + DESTDIR=$(TARGET_DIR) PREFIX=$(TARGET_DIR)/usr ETCDIR=$(TARGET_DIR)/etc \ + OPT_CNI_BIN_DIR=$(TARGET_DIR)/opt/cni/bin install.bin-nobuild + $(INSTALL) -d -m 700 $(TARGET_DIR)/etc/cni + $(INSTALL) -d -m 700 $(TARGET_DIR)/etc/cni/net.d + $(INSTALL) -D -m 644 $(@D)/contrib/cni/10-crio-bridge.conflist \ + $(TARGET_DIR)/etc/cni/net.d/10-crio-bridge.conflist +endef + +define CRIO_INSTALL_INIT_SYSTEMD + $(HOST_GO_COMMON_ENV) $(TARGET_MAKE_ENV) $(MAKE) -C $(@D) \ + DESTDIR=$(TARGET_DIR) PREFIX=$(TARGET_DIR)/usr ETCDIR=$(TARGET_DIR)/etc \ + OPT_CNI_BIN_DIR=$(TARGET_DIR)/opt/cni/bin install.systemd + $(SED) 's,/usr/local/bin,/usr/bin,g' \ + $(TARGET_DIR)/usr/lib/systemd/system/{crio,crio-wipe}.service +endef + +$(eval $(golang-package))
crio implements the Kubelet Container Runtime Interface (CRI) using OCI conformant runtimes like runc or podman. https://github.com/cri-o/cri-o/ Signed-off-by: Christian Stewart <christian@aperture.us> --- package/Config.in | 1 + package/crio/Config.in | 54 ++++++++++++++++++++++++++++ package/crio/crio.hash | 3 ++ package/crio/crio.mk | 82 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 140 insertions(+) create mode 100644 package/crio/Config.in create mode 100644 package/crio/crio.hash create mode 100644 package/crio/crio.mk