DPDK CI discussions
 help / color / mirror / Atom feed
From: Aaron Conole <aconole@redhat.com>
To: Adam Hassick <ahassick@iol.unh.edu>
Cc: ci@dpdk.org,  Owen Hilyard <ohilyard@iol.unh.edu>,  alialnu@nvidia.com
Subject: Re: [PATCH v5 01/11] containers/docs: Add container builder start
Date: Thu, 25 May 2023 09:48:26 -0400	[thread overview]
Message-ID: <f7tcz2omsid.fsf@redhat.com> (raw)
In-Reply-To: <20230523170413.812922-2-ahassick@iol.unh.edu> (Adam Hassick's message of "Tue, 23 May 2023 13:04:03 -0400")

Adam Hassick <ahassick@iol.unh.edu> writes:

> From: Owen Hilyard <ohilyard@iol.unh.edu>
>
> * Add README file for containers
> * Add pyproject file with required dependencies for building containers
>
> This module allows anyone to build the containers used in DPDK CI, and
> allows the community to contribute container definitions back to DPDK
> CI. Please read the README for more information, since some
> functionality is opt-in due to resource requirements.
>
> Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
> ---

I don't like the structure of this series.

For example, you introduce README.md in this patch, and then later on
rewrite sections of it.  Please don't do that.  There is no reason to
introduce wrong information in one patch and remove it in another.

In general, make an introduced change to be the complete change.  The
exception is when you are introducing features which modify existing
functionality, and therefore need to split the final form of a change
over a few discreet steps.

>  containers/README.md                      | 138 ++++++++++++++++++++++
>  containers/template_engine/pyproject.toml |  21 ++++
>  2 files changed, 159 insertions(+)
>  create mode 100644 containers/README.md
>  create mode 100644 containers/template_engine/pyproject.toml
>
> diff --git a/containers/README.md b/containers/README.md
> new file mode 100644
> index 0000000..5d01caf
> --- /dev/null
> +++ b/containers/README.md
> @@ -0,0 +1,138 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright (c) 2022 University of New Hampshire
> +
> +DPDK CI Container Build System
> +==============================
> +
> +## Rational
> +
> +There are a few important factors for why a custom build system was created for
> +the containers instead of using an existing one. The first was that podman was
> +nearly mandatory for this task.
> +
> +### Why Podman
> +
> +1. Licensed RHEL containers need podman
> +
> +The build system MUST be able to handle creating properly licensed RHEL
> +containers, so that the RHEL CI testing is as accurate as possible.
> +
> +2. "Developer Laptop Friendliness"
> +
> +Another goal of the build system was to enable anyone to easily build the
> +containers. Not all developers are able to use Linux as the main OS on their
> +main development machine. Podman runs on MacOS via podman-machine and Windows
> +either by podman-machine or WSL.
> +
> +3. OCI Containers
> +
> +OCI containers are more portable than some other container solutions. Much of
> +the progress on getting containers running on top of FreeBSD jails targets OCI
> +containers specifically. The tracking issue for this is
> +[https://reviews.freebsd.org/D21570](https://reviews.freebsd.org/D21570).
> +Once upstream support happens, there should be a relatively simple path to
> +supporting containers in FreeBSD once podman/docker APIs are better supported.
> +At the moment, lack up upstream support means no support in this project for
> +FreeBSD.
> +
> +### Python and Makefiles instead of Buildah as a library
> +
> +The next question someone might have is why a combination of Python and
> +makefiles were used instead of using buildah as a library. The largest
> +reason is that every DPDK developer is going to need to have some
> +level of familiarity with Python due to DTS. Buildah is only available
> +as a library via Go, and would tie DPDK to a particular container
> +implementation. Go, while not difficult to learn, is a compiled language,
> +meaning that the build system would require a build system.
> +
> +The other reason is that most of the logic that needs to be performed is very
> +simple, and python has a few libraries that do most of the work. If it weren't
> +for the desire to have an inventory file (inventory.yaml) with a schema
> +(inventory_schema.json), this probably could have been an AWK script. After the
> +container images are produced, it is very easy to use the same template
> +engine to produce a makefile that can be used to both build and push the
> +containers. This makefile can be run with multiple jobs for parallel building
> +of containers, something not supported by all compose implementations.
> +
> +Meson was considered instead of Makefiles, however, Meson does not handle new
> +Meson being generated during the build very well, and Meson wants most commands
> +to have an output file, which is not true of many of the commands. Meson is
> +also more difficult to generate using a templating library than Makefile
> +targets.
> +
> +## Building
> +
> +### Environment Variables
> +
> +All environment variables are namespaced to DPDK_CI_CONTAINERS to avoid any
> +issues.
> +
> +| Variable                   | Description                                     | Default | Valid Values |
> +| -------------------------- | ----------------------------------------------- | ------- | ------------ |
> +DPDK_CI_CONTAINERS_ON_RHEL | Whether you are building on licensed RHEL. RHEL containers must be built on licensed RHEL, this can be used to forcibly enable/disable RHEL containers if automatic detection fails. | (grep -q 'Red Hat Enterprise Linux' /etc/redhat-release && echo 'Y') \|\| echo 'N' | 'Y' or 'N'
> +DPDK_CI_CONTAINERS_FAIL_ON_UNBUILDABLE | Fail during dockerfile generation if any container in the inventory is not buildable. Currently will cause a failure if you are not on RHEL and try to build RHEL containers. | 'N' | 'Y' or 'N'
> +DPDK_CI_CONTAINERS_BUILD_ABI | Whether to bake ABI images into the containers. | 'N' | 'Y' or 'N'
> +DPDK_CI_CONTAINERS_BUILD_LIBABIGAIL | Whether to build libabigail from source on distros that do not package it. DPDK_CI_CONTAINERS_BUILD_ABI=Y overrides this to 'Y' | 'N' | 'Y' or 'N'
> +DPDK_CI_CONTAINER_BUILDER_PROGRAM | What container builder program to use. | 'podman' | Any container builder that exposes the same interface and provides the same behavior as podman.
> +DPDK_CI_CONTAINERS_LIBABIGAIL_CLONE_URL | What URL to clone libabigail from, since some distros need to compile it from source. | 'git://sourceware.org/git/libabigail.git' | A repository containing libabigail which shares history with the main repository.
> +DPDK_CI_CONTAINERS_DPDK_CLONE_URL | What URL to clone DPDK from. | 'https://dpdk.org/git/dpdk' | Any DPDK mirror.
> +DPDK_CI_CONTAINERS_CONTAINER_BUILDER_TAG | What tag to give to the container which creates the dockerfiles. The default should be fine unless you have issues with collisions. | 'dpdk_ci_container_builder' | Any valid OCI container tag (A valid C function name will work)
> +DPDK_CI_CONTAINERS_EXTRA_PUSH_ARGS | Extra arguments to add to the push command, can be used for credentials if 'podman login' won't work. | '' | [https://docs.podman.io/en/latest/markdown/podman-push.1.html#options](https://docs.podman.io/en/latest/markdown/podman-push.1.html#options)
> +DPDK_CI_CONTAINERS_REGISTRY_HOSTNAME | The hostname of the registry to push to. | 'localhost' | The hostname of any system exposing an OCI container registry or localhost to push to local storage.
> +DPDK_CI_CONTAINERS_EXTRA_SCRIPTS_PATH | The path to a directory to copy into all of the containers at /scripts | unset | The path to any local file directory.
> +DPDK_CI_CONTAINERS_CONTEXT_DIRECTORY | Set the directory to build the containers in. All generated files will be placed in this directory or one of it's children | '$(CURDIR)/container_context' | Any absolute directory path
> +
> +### Builder System Requirements
> +
> +#### Required Programs
> +
> +* GNU make (POSIX make may work, but is not supported)
> +* git
> +* find
> +* posix utilities (GNU coreutils will work)
> +* bash
> +* podman >= 4.0.0 (docker or other container builder programs may work, but are
> +unsupported)
> +    * podman 4.0.0 allows run mounts, which allow mounting a directory into the build context of a container. This is used to  persist ccache directories for each container.
> +* qemu-$ARCH-static for any non-native architecture/revision you want to build for.
> +
> +#### Hardware
> +
> +| Hardware Type | Requirement                  | Reason |
> +| ------------- | ---------------------------- | ----------------------------------- |
> +| Disk space    | 5 GB of disk space per image | Some images are 4 GB at the moment, and as DPDK's API grows, so will the ABI references.
> +Memory | Either 1.5x or 2x the memory needed to compile DPDK per makefile job | 1.5x is enough for the container overhead and caching when compiling natively, 2x is for builds under emulation (ARM container on x86, etc).
> +
> +
> +#### RHEL containers
> +
> +RHEL container images must be built on RHEL.
> +
> +### Build containers locally
> +
> +```bash
> +make build
> +```
> +
> +### Push containers to registry
> +
> +This will probably involve following prompts in your terminal, but if you have
> +other authentication set up, (LDAP, Kerberos, etc), it may not prompt you.
> +Logging into a registry is what allows you to upload containers to a remote
> +system for others to pull down.
> +
> +If you are working alone, you probably can ignore this and keep the containers
> +locally. If you are in an enterprise setting, ask your DevOps or Systems
> +Administration team where the preferred location for hosting containers is.
> +
> +Since these images take so long to build, it is recommended to use a container
> +registry and have any CI systems pull from that registry.
> +
> +Redhat guide to setting up a podman container registry:
> +[https://www.redhat.com/sysadmin/simple-container-registry](https://www.redhat.com/sysadmin/simple-container-registry)
> +
> +```bash
> +$DPDK_CI_CONTAINER_BUILDER_PROGRAM login $DPDK_CI_CONTAINERS_REGISTRY_HOSTNAME
> +# < Complete login process >
> +make push
> +```
> \ No newline at end of file
> diff --git a/containers/template_engine/pyproject.toml b/containers/template_engine/pyproject.toml
> new file mode 100644
> index 0000000..f5611ce
> --- /dev/null
> +++ b/containers/template_engine/pyproject.toml
> @@ -0,0 +1,21 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright (c) 2022 University of New Hampshire
> +
> +[tool.poetry]
> +name = "dpdk_ci_containers"
> +version = "0.1.0"
> +description = ""
> +authors = ["Owen Hilyard <ohilyard@iol.unh.edu>"]
> +license = "BSD-3-Clause"
> +
> +[tool.poetry.dependencies]
> +python = "^3.8"
> +Jinja2 = "^3.1.2"
> +jsonschema = "^4.10.0"
> +PyYAML = "^6.0"
> +
> +[tool.poetry.dev-dependencies]
> +
> +[build-system]
> +requires = ["poetry-core>=1.0.0"]
> +build-backend = "poetry.core.masonry.api"


  reply	other threads:[~2023-05-25 13:48 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-23 17:04 [PATCH v5 00/11] Community Lab Containers and Builder Engine Adam Hassick
2023-05-23 17:04 ` [PATCH v5 01/11] containers/docs: Add container builder start Adam Hassick
2023-05-25 13:48   ` Aaron Conole [this message]
2023-05-23 17:04 ` [PATCH v5 02/11] containers/inventory: Add inventory for container builder Adam Hassick
2023-05-23 17:04 ` [PATCH v5 03/11] containers/builder: Dockerfile creation script Adam Hassick
2023-05-23 17:04 ` [PATCH v5 04/11] containers/templates: Templates for Dockerfiles Adam Hassick
2023-05-23 17:04 ` [PATCH v5 05/11] containers/container_builder: Container for python scripts Adam Hassick
2023-05-23 17:04 ` [PATCH v5 06/11] containers/Makefile: Makefile to automate builds Adam Hassick
2023-05-23 17:04 ` [PATCH v5 07/11] containers/inventory: Add ABI rev and coverity attribute Adam Hassick
2023-05-23 17:04 ` [PATCH v5 08/11] containers/builder: Add arguments to templating script Adam Hassick
2023-05-23 17:04 ` [PATCH v5 09/11] containers/templates: Update templates Adam Hassick
2023-05-23 17:04 ` [PATCH v5 10/11] containers/Makefile: Add new variables and target changes Adam Hassick
2023-05-23 17:04 ` [PATCH v5 11/11] containers/docs: Update README Adam Hassick
2023-05-25 13:45   ` Aaron Conole
2023-06-05 20:09     ` Adam Hassick

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f7tcz2omsid.fsf@redhat.com \
    --to=aconole@redhat.com \
    --cc=ahassick@iol.unh.edu \
    --cc=alialnu@nvidia.com \
    --cc=ci@dpdk.org \
    --cc=ohilyard@iol.unh.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).