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-1595: generate BPF from CI #309

Merged
merged 5 commits into from
Apr 5, 2024

Conversation

jotak
Copy link
Member

@jotak jotak commented Apr 2, 2024

  • Add BPF generation steps in the main Dockerfile
  • Split make generate in 2 (one for BPF another for protoc ; we don't need to build protobuf from CI as this is still code)
  • Add a pull-request check to verify that the provided BPF bin match the code

Copy link

openshift-ci bot commented Apr 2, 2024

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@jotak jotak added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Apr 3, 2024
@codecov-commenter
Copy link

codecov-commenter commented Apr 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 33.84%. Comparing base (1cc8125) to head (2ce74a7).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #309   +/-   ##
=======================================
  Coverage   33.84%   33.84%           
=======================================
  Files          47       47           
  Lines        3865     3865           
=======================================
  Hits         1308     1308           
  Misses       2469     2469           
  Partials       88       88           
Flag Coverage Δ
unittests 33.84% <100.00%> (ø)

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

github-actions bot commented Apr 3, 2024

New image:
quay.io/netobserv/netobserv-ebpf-agent:3ba1248

It will expire after two weeks.

To deploy this build, run from the operator repo, assuming the operator is running:

USER=netobserv VERSION=3ba1248 make set-agent-image

@jotak jotak changed the title test non-modified binaries NETOBSERV-1595: generate BPF from CI Apr 3, 2024
@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented Apr 3, 2024

@jotak: This pull request references NETOBSERV-1595 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.16.0" version, but no target version was set.

In response to this:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented Apr 3, 2024

@jotak: This pull request references NETOBSERV-1595 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.16.0" version, but no target version was set.

In response to this:

  • Add BPF generation steps in the main Dockerfile
  • Split make generate in 2 (one for BPF another for protoc ; we don't need to build protobuf from CI as this is still code)
  • Add a pull-request check to verify that the provided BPF bin match the code

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@jotak
Copy link
Member Author

jotak commented Apr 3, 2024

I intentionally removed the .o files from source and generated a build just to make sure it uses CI-generated .o
I'll put back .o in git so that they are still available for local runs.

@github-actions github-actions bot removed the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Apr 3, 2024
@jotak jotak marked this pull request as ready for review April 3, 2024 07:28
Dockerfile Outdated
FROM --platform=$BUILDPLATFORM docker.io/library/golang:1.21 as builder

# Build layer
FROM --platform=$TARGETPLATFORM fedora:38 as builder
Copy link
Contributor

Choose a reason for hiding this comment

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

Pls use fc39 as in the generator your pr should have no .o changes

Makefile Outdated
@echo "### Generating BPF Go bindings"
test -f $(shell go env GOPATH)/bin/bpf2go || go install github.com/cilium/ebpf/cmd/bpf2go@${CILIUM_EBPF_VERSION}
Copy link
Contributor

Choose a reason for hiding this comment

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

U can do preqs here instead of repeating install

@msherif1234
Copy link
Contributor

/LGTM

@msherif1234
Copy link
Contributor

/test images

1 similar comment
@msherif1234
Copy link
Contributor

/test images

@jotak jotak added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Apr 5, 2024
Copy link

github-actions bot commented Apr 5, 2024

New image:
quay.io/netobserv/netobserv-ebpf-agent:9a271c2

It will expire after two weeks.

To deploy this build, run from the operator repo, assuming the operator is running:

USER=netobserv VERSION=9a271c2 make set-agent-image

@jotak
Copy link
Member Author

jotak commented Apr 5, 2024

/approve

@msherif1234 one downside of this is increased image build time, that can perhaps be painful when you build locally. If this ends up too painful we could create a separate dockerfile for local builds, that would diverge from CI dockerfile.

@jotak jotak added the approved label Apr 5, 2024
Copy link

openshift-ci bot commented Apr 5, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jotak

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

@openshift-merge-bot openshift-merge-bot bot merged commit cb2cf53 into netobserv:main Apr 5, 2024
10 checks passed
msherif1234 added a commit to msherif1234/netobserv-ebpf-agent that referenced this pull request Apr 15, 2024
Signed-off-by: Mohamed Mahmoud <mmahmoud@redhat.com>
openshift-merge-bot bot pushed a commit that referenced this pull request Apr 15, 2024
Signed-off-by: Mohamed Mahmoud <mmahmoud@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved jira/valid-reference lgtm ok-to-test To set manually when a PR is safe to test. Triggers image build on PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants