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

fix build for s390x and ppc64le #151

Merged
merged 1 commit into from
Jul 18, 2023
Merged

fix build for s390x and ppc64le #151

merged 1 commit into from
Jul 18, 2023

Conversation

msherif1234
Copy link
Contributor

@msherif1234 msherif1234 commented Jul 17, 2023

ppc64le and s390x for some reason they use uint8 instead of int8 for this type
https://pkg.go.dev/syscall#Utsname
leading to this build error

6.662 pkg/utils/utils.go:80:54: cannot use buf.Release[:] (value of type []uint8) as []int8 value in argument to utsnameStr

Fix to create two instances of utils package based on arch check

  • unit-tests
 docker images
REPOSITORY                               TAG            IMAGE ID       CREATED          SIZE
quay.io/mmahmoud/netobserv-ebpf-agent    main-s390x     bd818f928693   24 seconds ago   117MB
quay.io/mmahmoud/netobserv-ebpf-agent    main-amd64     1b01b143841e   5 minutes ago    118MB
quay.io/netobserv/netobserv-ebpf-agent   main-amd64     1b01b143841e   5 minutes ago    118MB
quay.io/mmahmoud/netobserv-ebpf-agent    main-ppc64le   b01ba81f8e67   6 minutes ago    141MB

@codecov
Copy link

codecov bot commented Jul 17, 2023

Codecov Report

Merging #151 (5858f39) into main (4a182aa) will not change coverage.
The diff coverage is 0.00%.

@@           Coverage Diff           @@
##             main     #151   +/-   ##
=======================================
  Coverage   39.25%   39.25%           
=======================================
  Files          31       31           
  Lines        2214     2214           
=======================================
  Hits          869      869           
  Misses       1296     1296           
  Partials       49       49           
Flag Coverage Δ
unittests 39.25% <0.00%> (ø)

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

Impacted Files Coverage Δ
pkg/utils/utils.go 46.42% <0.00%> (ø)

@memodi
Copy link
Contributor

memodi commented Jul 17, 2023

/ok-to-test

@openshift-ci openshift-ci bot added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Jul 17, 2023
@github-actions
Copy link

New image: quay.io/netobserv/netobserv-ebpf-agent:96e75c6. It will expire after two weeks.

@stleerh
Copy link

stleerh commented Jul 17, 2023

There's no need to create a duplicate file to handle the different architectures. Just use generics instead. Change:
func utsnameStr(in []int8) string {

to:
func utsnameStr[T int8 | uint8](in []T) string {

Signed-off-by: msherif1234 <mmahmoud@redhat.com>
@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 Jul 17, 2023
@msherif1234
Copy link
Contributor Author

/ok-to-test

@openshift-ci openshift-ci bot added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Jul 17, 2023
@github-actions
Copy link

New image:
quay.io/netobserv/netobserv-ebpf-agent:dd3ced3

It will expire after two weeks.

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

USER=netobserv VERSION=dd3ced3 make set-agent-image

@stleerh
Copy link

stleerh commented Jul 17, 2023

/lgtm but might want to add a comment

@msherif1234
Copy link
Contributor Author

/approve

@openshift-ci
Copy link

openshift-ci bot commented Jul 18, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: msherif1234

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-robot openshift-merge-robot merged commit df09c4e into netobserv:main Jul 18, 2023
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved 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