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

Add CONFIG_BPF_SYSCALL=y to aarch64 config #15164

Merged
merged 2 commits into from Jun 14, 2023
Merged

Conversation

brancz
Copy link
Member

@brancz brancz commented Oct 18, 2022

This is a rebased version of #14675.

Supersedes #14675.

cc @eiffel-fl

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Oct 18, 2022
@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Oct 18, 2022
@minikube-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@brancz
Copy link
Member Author

brancz commented Oct 18, 2022

/assign @medyagh @eiffel-fl

@k8s-ci-robot
Copy link
Contributor

@brancz: GitHub didn't allow me to assign the following users: eiffel-fl.

Note that only kubernetes members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

In response to this:

/assign @medyagh @eiffel-fl

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 kubernetes/test-infra repository.

@eiffel-fl
Copy link
Contributor

Hi.

Thank you for the rebase!
I tested the image I built on top of main and it works fine:

 $ ./out/minikube start --driver=qemu --iso-url=$(pwd)minikube-arm64.iso
😄  minikube v1.27.0 on Darwin 12.6 (arm64)
✨  Using the qemu2 (experimental) driver based on user configuration
❗  The default network for QEMU will change from 'user' to 'socket_vmnet' in a future release
❗  You are using the QEMU driver without a dedicated network, which doesn't support `minikube service` & `minikube tunnel` commands.
To try the experimental dedicated network see: https://minikube.sigs.k8s.io/docs/drivers/qemu/#networking
💿  Downloading VM boot image ...
🎉  minikube 1.27.1 is available! Download it: https://github.com/kubernetes/minikube/releases/tag/v1.27.1
👍  Starting control plane node minikube in cluster minikube
💡  To disable this notice, run: 'minikube config set WantUpdateNotification false'

🔥  Creating qemu2 VM (CPUs=2, Memory=4000MB, Disk=20000MB) ...
🐳  Preparing Kubernetes v1.25.1 on Docker 20.10.18 ...
    ▪ Generating certificates and keys ...
    ▪ Booting up control plane ...
    ▪ Configuring RBAC rules ...
🔎  Verifying Kubernetes components...
    ▪ Using image gcr.io/k8s-minikube/storage-provisioner:v5
🌟  Enabled addons: storage-provisioner, default-storageclass
🏄  Done! kubectl is now configured to use "minikube" cluster and "default" namespace by default
$ ./out/minikube start
                         _             _            
            _         _ ( )           ( )           
  ___ ___  (_)  ___  (_)| |/')  _   _ | |_      __  
/' _ ` _ `\| |/' _ `\| || , <  ( ) ( )| '_`\  /'__`\
| ( ) ( ) || || ( ) || || |\`\ | (_) || |_) )(  ___/
(_) (_) (_)(_)(_) (_)(_)(_) (_)`\___/'(_,__/'`\____)

$ zgrep DEBUG_INFO_BTF /proc/config.gz 
CONFIG_DEBUG_INFO_BTF=y
$ ls /sys/kernel/btf/vmlinux 
/sys/kernel/btf/vmlinux

So, your image can be built in the CI!
I will just test it once built by the CI by I think we can merge it without troubles :)!

Best regards.

@brancz
Copy link
Member Author

brancz commented Oct 20, 2022

That would be awesome! 🥳

@spowelljr
Copy link
Member

ok-to-build-iso

@minikube-bot
Copy link
Collaborator

Hi @brancz, we have updated your PR with the reference to newly built ISO. Pull the changes locally if you want to test with them or update your PR further.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 21, 2022
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 21, 2022
@spowelljr
Copy link
Member

ok-to-build-iso

@minikube-bot
Copy link
Collaborator

Hi @brancz, building a new ISO failed.
See the logs at: https://storage.cloud.google.com/minikube-builds/logs/15164/39a7865/iso_build.txt

@spowelljr
Copy link
Member

Getting the same error as #14675 (comment)

@eiffel-fl
Copy link
Contributor

Getting the same error as #14675 (comment)

Is this bug known? I do not really understand why adding a CONFIG_ to the kernel makes libocispec failed to build.
I will take a deeper look at it though.

@kakkoyun
Copy link
Contributor

Hey @eiffel-fl, any chance you had a look at this?
Time to time I realize we need this :)

@eiffel-fl
Copy link
Contributor

eiffel-fl commented Dec 15, 2022

Hey @eiffel-fl, any chance you had a look at this?

Hi!

This is still on my TODO list, I will take a look before the end of the year!

Time to time I realize we need this :)

Do you have an ISO with which you can play? Meanwhile, I can craft one for you so you can run eBPF related stuff on arm64 CPU.

@brancz
Copy link
Member Author

brancz commented Dec 15, 2022

We've been using the first ISO you shared and it has been working flawlessly for months.

@eiffel-fl
Copy link
Contributor

We've been using the first ISO you shared and it has been working flawlessly for months.

OK, this is a lesser evil but I do totally agree this change should be upstreamed!
I will take a look at it but I am bit concerned about not being able to reproduce it locally :s.
I will share my findings as soon as I work on it!

@eiffel-fl
Copy link
Contributor

I compiled again the image with this patch locally and was successful doing it.
Regarding the problem which happens in the CI during the image build, it could be related to containers/libocispec#70 but it was normally fixed by containers/libocispec#75 which is included in version of crun used in minikube (i.e. 1.2).

Sadly, I was not able to trigger this race condition locally, is it possible to reproduce the CI environment to try to reproduce the race condition locally? It would ease finding a proper solution.

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Mark this PR as fresh with /remove-lifecycle stale
  • Close this PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Mar 21, 2023
@eiffel-fl
Copy link
Contributor

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Mar 22, 2023
@brancz
Copy link
Member Author

brancz commented Mar 23, 2023

If there is absolutely anything we can do to help this along let us know and we'll happily do it! :)

@spowelljr
Copy link
Member

ok-to-build-iso

@minikube-bot
Copy link
Collaborator

Hi @brancz, we have updated your PR with the reference to newly built ISO. Pull the changes locally if you want to test with them or update your PR further.

@spowelljr
Copy link
Member

I'm going to build it again to see if it's flaky

@spowelljr
Copy link
Member

ok-to-build-iso

@minikube-bot
Copy link
Collaborator

Hi @brancz, we have updated your PR with the reference to newly built ISO. Pull the changes locally if you want to test with them or update your PR further.

@k8s-ci-robot k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Mar 28, 2023
@spowelljr
Copy link
Member

ok-to-build-iso

@minikube-bot
Copy link
Collaborator

Hi @brancz, we have updated your PR with the reference to newly built ISO. Pull the changes locally if you want to test with them or update your PR further.

@brancz
Copy link
Member Author

brancz commented Mar 29, 2023

Pulled and tested, and it looks good!

@spowelljr
Copy link
Member

/ok-to-test

@k8s-ci-robot k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Mar 29, 2023
@minikube-pr-bot

This comment has been minimized.

@minikube-pr-bot

This comment has been minimized.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 30, 2023
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 13, 2023
@spowelljr
Copy link
Member

ok-to-build-iso

@minikube-bot
Copy link
Collaborator

Hi @brancz, we have updated your PR with the reference to newly built ISO. Pull the changes locally if you want to test with them or update your PR further.

@minikube-pr-bot
Copy link

kvm2 driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 15164) |
+----------------+----------+---------------------+
| minikube start | 51.2s    | 51.3s               |
| enable ingress | 28.0s    | 27.2s               |
+----------------+----------+---------------------+

Times for minikube (PR 15164) start: 51.7s 50.8s 52.4s 52.2s 49.3s
Times for minikube start: 53.7s 50.7s 49.9s 50.5s 51.4s

Times for minikube ingress: 29.3s 29.2s 26.8s 27.2s 27.7s
Times for minikube (PR 15164) ingress: 27.3s 29.2s 27.7s 24.2s 27.8s

docker driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 15164) |
+----------------+----------+---------------------+
| minikube start | 24.4s    | 22.8s               |
| enable ingress | 20.5s    | 21.4s               |
+----------------+----------+---------------------+

Times for minikube start: 24.7s 26.4s 24.8s 22.0s 24.2s
Times for minikube (PR 15164) start: 24.3s 21.3s 25.0s 21.8s 21.8s

Times for minikube ingress: 20.8s 19.8s 20.3s 20.8s 20.8s
Times for minikube (PR 15164) ingress: 21.8s 20.8s 20.8s 22.8s 20.8s

docker driver with containerd runtime

+-------------------+----------+---------------------+
|      COMMAND      | MINIKUBE | MINIKUBE (PR 15164) |
+-------------------+----------+---------------------+
| minikube start    | 21.6s    | 23.0s               |
| ⚠️  enable ingress | 26.3s    | 34.1s ⚠️             |
+-------------------+----------+---------------------+

Times for minikube ingress: 19.3s 31.3s 31.4s 18.3s 31.3s
Times for minikube (PR 15164) ingress: 31.3s 46.3s 30.3s 31.3s 31.3s

Times for minikube start: 20.6s 20.7s 23.3s 20.4s 23.2s
Times for minikube (PR 15164) start: 23.8s 23.1s 21.9s 22.5s 23.4s

@minikube-pr-bot
Copy link

These are the flake rates of all failed tests.

Environment Failed Tests Flake Rate (%)
Docker_Windows TestStartStop/group/default-k8s-diff-port/serial/SecondStart (gopogh) 0.00 (chart)
KVM_Linux_containerd TestRunningBinaryUpgrade (gopogh) 4.79 (chart)

To see the flake rates of all tests by environment, click here.

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: brancz, spowelljr

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 14, 2023
@spowelljr spowelljr merged commit abd35a5 into kubernetes:master Jun 14, 2023
27 of 39 checks passed
@eiffel-fl
Copy link
Contributor

Amazing! Thank you for the merge!

@brancz brancz deleted the patch-1 branch February 22, 2024 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants