NETOBSERV-848 eBPF agent multi-arch builds (upstream)

[jpinsonneau]

Implements multi arch builds and add documentation and consistency

$ make help

Usage:
  make <target>

General
  help                  Display this help.
  vendors               Check go vendors
  prereqs               Check if prerequisites are met, and install missing dependencies

Develop
  fmt                   Run go fmt against code.
  lint                  Lint the code
  generate              Generate artifacts of the code repo (pkg/ebpf and pkg/proto packages)
  docker-generate       Create the container that generates the eBPF binaries
  compile               Compile ebpf agent project
  test                  Test code using go test
  coverage-report       Generate coverage report
  coverage-report-html  Generate HTML coverage report
  tests-e2e             Run e2e tests

Images
  image-build           Build MULTIARCH_TARGETS images
  image-push            Push MULTIARCH_TARGETS images
  manifest-build        Build MULTIARCH_TARGETS manifest
  manifest-push         Push MULTIARCH_TARGETS manifest
  ci-images-build       Build CI images
  ci-images-push        Push CI images

shortcuts helpers
  build                 Test and Build ebpf agent
  build-image           Build MULTIARCH_TARGETS images
  push-image            Push MULTIARCH_TARGETS images
  build-manifest        Build MULTIARCH_TARGETS manifest
  push-manifest         Push MULTIARCH_TARGETS manifest
  images                Build and push MULTIARCH_TARGETS images and related manifest
  build-ci-images       Build CI images
  push-ci-images        Push CI images
  ci-images             Build and push CI images

Please use netobserv/flowlogs-pipeline#426 for main discussion and this PR only for specific items

[msherif1234]

why we didn't pass in the new build-arg here and make the platform env var and user can pass in the arch it needs to build for instead of having the new targets

[jpinsonneau]

so the var will update it's value one after the other for multiarch build ? you will loose the ability to run in parallel like make -j multiarch-manifest-build 🤔

[msherif1234]

I was thinking of two make invocation in case of amd and ppc tomorrow could be another platform for ibm or windows

[jpinsonneau]

I didn't found a way to make multiarch-manifest-build dependencies passing environment variables

The trick here is to concat a simple string that build the dependencies using $(foreach T,$(MULTIARCH_TARGETS),image-push/$T )

Does that work for you ?

[jpinsonneau]

another alternative would be to always rely on MULTIARCH_TARGETS for both image-build and image-push and totally get rid of multiarch-manifest-build and multiarch-manifest-push commands.

the manifest can either always be created or created only when MULTIARCH_TARGETS contains at least 2 items.

@OlivierCazade @jotak WDYT ?

[jpinsonneau]

d54d221

[jotak]

sry I'd like to wait for Olivier's input here - he's been the one who has most explored the various options

[stleerh]

• If podman doesn't exist, for example, it will display an error message.  This should be suppressed. 
• It should also check that one of podman or docker exists and quit if that is not the case.
• Use := instead of =.

Suggest:

OCI_BIN_PATH := $(shell which podman 2> /dev/null || which docker 2> /dev/null)
ifeq ($(OCI_BIN_PATH),)
$(error Requires podman or docker in PATH)
endif

[jpinsonneau]

Yes, I relied on FLP Makefile for such changes. Let's all agree on how to manage that and apply it everywhere !

@jotak @OlivierCazade @msherif1234

[jotak]

How about a slight variation:

OCI_BIN_PATH ?= $(shell which podman 2> /dev/null || which docker 2> /dev/null)
ifeq ($(OCI_BIN_PATH),)
$(error Requires OCI-compliant CLI such as podman or docker in PATH)
endif

?= allows for overriding it, e.g. using a different installation path or another OCI compliant tool

[stleerh]

The only problem is that in other parts of the Makefile, it generally uses OCI_BIN, which is the basename of OCI_BIN_PATH (either 'podman' or 'docker') instead of OCI_BIN_PATH which has the absolute path.  Therefore, it's possible if you override OCI_BIN_PATH, OCI_BIN could fail or even use a different version depending on PATH.

You could change all OCI_BIN usage to OCI_BIN_PATH.  There is at least one case where it's using OCI_BIN to determine if you are running docker or podman.

[jpinsonneau]

your suggestions doesn't work with docker.io/library/golang:1.19 image + it's common to simply use $(shell which podman || which docker) without further checks

I'll keep this as is for now

[codecov]

Codecov Report

Merging #100 (072e9e8) into main (beb50b3) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #100   +/-   ##
=======================================
  Coverage   41.80%   41.80%           
=======================================
  Files          30       30           
  Lines        2050     2050           
=======================================
  Hits          857      857           
  Misses       1155     1155           
  Partials       38       38           

Flag	Coverage Δ
unittests	41.80% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

[msherif1234]

/LGTM

[jpinsonneau]

/approve

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jpinsonneau

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [jpinsonneau]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment