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

NETOBSERV-994 FLP multiarch consistency #426

Merged
merged 1 commit into from
May 11, 2023

Conversation

jpinsonneau
Copy link
Collaborator

@jpinsonneau jpinsonneau commented Apr 20, 2023

These changes brings consistency between netobserv components

  • added shortcuts
  • use the same target for multi arch / single arch relying on MULTIARCH_TARGETS array
    • The default is currently amd64 arm64 ppc64le but I'm fine to set only amd64 if you prefer
  • renamed variables and targets so every repo can run the same targets such as:
    • IMAGE_ORG=jpinsonn VERSION=multiarch make images to push all images and manifest
    • MULTIARCH_TARGETS=amd64 make image-build image-push to push a single amd64 image to quay.io/netobserv/flowlogs-pipeline:latest-adm64
$ make help

Usage:
  make <target>

General
  help                  Display this help.
  vendors               Check go vendors

Develop
  lint                  Lint the code
  build                 Build flowlogs-pipeline executable and update the docs
  docs                  Update flowlogs-pipeline documentation
  clean                 Clean
  tests-unit            Unit tests
  tests-fast            Fast unit tests (no race tests / coverage)
  tests-e2e             End-to-end tests
  tests-all             All tests
  benchmarks            Benchmark
  run                   Run

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
  build-ci-images       Build CI images
  push-ci-images        Push CI images

kubernetes
  deploy                Deploy the image
  undeploy              Undeploy the image
  deploy-loki           Deploy loki
  undeploy-loki         Undeploy loki
  deploy-prometheus     Deploy prometheus
  undeploy-prometheus   Undeploy prometheus
  deploy-grafana        Deploy grafana
  undeploy-grafana      Undeploy grafana
  deploy-netflow-simulator  Deploy netflow simulator
  undeploy-netflow-simulator  Undeploy netflow simulator

kind
  create-kind-cluster   Create cluster
  delete-kind-cluster   Delete cluster
  kind-load-image       Load image to kind

metrics
  generate-configuration  Generate metrics configuration

End2End
  local-deploy          Deploy locally on kind (with simulated flowlogs)
  local-cleanup         Undeploy from local kind
  local-redeploy        Redeploy locally (on current kind)
  ocp-deploy            Deploy to OCP
  ocp-cleanup           Undeploy from OCP
  dev-local-deploy      Deploy locally with simulated netflows

shortcuts helpers
  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
  ci-images-build       Build CI images
  ci-images-push        Push CI images
  ci-images             Build and push CI images

Related PRs:

@codecov
Copy link

codecov bot commented Apr 20, 2023

Codecov Report

Merging #426 (57df16d) into main (4aee357) will increase coverage by 0.03%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #426      +/-   ##
==========================================
+ Coverage   64.72%   64.75%   +0.03%     
==========================================
  Files          94       94              
  Lines        6645     6645              
==========================================
+ Hits         4301     4303       +2     
+ Misses       2100     2099       -1     
+ Partials      244      243       -1     
Flag Coverage Δ
unittests 64.75% <ø> (+0.03%) ⬆️

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

see 1 file with indirect coverage changes

@jotak
Copy link
Member

jotak commented Apr 20, 2023

@jpinsonneau did you see this issue: #423 ? I was wondering if that could also impact what you're doing (I don't think so, but haven't had time to look closely)

@jpinsonneau
Copy link
Collaborator Author

@jpinsonneau did you see this issue: #423 ? I was wondering if that could also impact what you're doing (I don't think so, but haven't had time to look closely)

@jotak I suggest to address this in a separate PR
We need to create our own docker image, save it to netobserv quay (I can't push as maintainer 😿 ) and then update the builder images in all of our components passing --build-arg GO_VERSION=1.19.8 in build_target functions

.PHONY: image-push
image-push: ## Push MULTIARCH_TARGETS images
trap 'exit' INT; \
$(foreach target,$(MULTIARCH_TARGETS),$(call push_target,$(target)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how long it takes before and after this change to build and push images
I am curious if that takes much longer time we could have multi-target build option and just leave the current options for the default target ?
depends on how frequently we need to build for all targets do we do that when release new version or on every PR ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's mainly 3 times longer as MULTIARCH_TARGETS contains amd64 arm64 ppc64le currently. The manifest creation is super fast and can even be skipped if you point a single arch image.
I'm fine setting amd64 only as default and letting a comment with example to build the 3 archs.

Copy link
Contributor

@OlivierCazade OlivierCazade left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jpinsonneau
Copy link
Collaborator Author

/approve

@openshift-ci
Copy link

openshift-ci bot commented May 11, 2023

[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:

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

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

Successfully merging this pull request may close these issues.

5 participants