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

Update to buildroot 2023.02.9 & Golang 1.21.6 #18020

Merged
merged 25 commits into from
Feb 28, 2024

Conversation

travier
Copy link
Contributor

@travier travier commented Jan 22, 2024

Heavily inspired by #17093

Building and launching an instance using the KVM2 driver & cri-o runtime works.

Did not do any further tests so far.

Fixes: #15841

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 22, 2024
@k8s-ci-robot
Copy link
Contributor

Hi @travier. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@k8s-ci-robot k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jan 22, 2024
@minikube-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@afbjorklund
Copy link
Collaborator

You might want to separate the ISO updates from the CI updates

@travier
Copy link
Contributor Author

travier commented Jan 22, 2024

They were all together in the parent PRs. A lot of the ISO updates are removing packages to use the one from the new buildroot version.

Could you clarify which changes do you think I should split?

@afbjorklund
Copy link
Collaborator

Could you clarify which changes do you think I should split?

I just meant the deploy/iso vs. the hack/update, but if they were together before it might be easier for @spowelljr

@spowelljr
Copy link
Member

spowelljr commented Jan 22, 2024

The hack/update change is because we were managing the CNI plugins package ourself, but the new Buildroot version has it included, so I was removing our CI from auto updating the CNI plugins in ISO since the files no longer exist, so I think it makes sense to include it

@spowelljr
Copy link
Member

ok-to-build-iso

@travier
Copy link
Contributor Author

travier commented Jan 28, 2024

Documenting missing bits / issues as I find them:

  • cni-plugins from buildroot does not include the host-local plugin (breaks cri-o)
  • crun version in buildroot is only 1.7.2

@spowelljr
Copy link
Member

Just a heads up that we're resolving issues with our CI infrastructure so won't be able to build the ISO right now, but bear with us and we'll try building this as soon as we can

@medyagh
Copy link
Member

medyagh commented Jan 30, 2024

@travier thank you very much for this contribution, glad to find another developer touching this code, this part of the code requires special experties, our CI machine is currently having issue, as soon as we fix that we can build the ISO again

@travier
Copy link
Contributor Author

travier commented Jan 30, 2024

How would you prefer to go for crun & cni-plugins? Should we keep the updated/custom versions in minikube or should we patch the buildroot ones?

From experiments so far, we can not have external packages with the same names as the ones in buildroot so we would need to rename them to keep them in the minikube tree. Otherwise this will need "large" patches for the buildroot tree.

@spowelljr
Copy link
Member

From a maintenance standpoint I think having a custom module with a different name would be ideal. Unless there is an easy to to update the patches. I'm just thinking if Buildroot updates crun they we have to remove the patches that are now included, and a new version comes out we have to add new patches, seems like more work than the custom package.

@travier
Copy link
Contributor Author

travier commented Jan 31, 2024

So, with this update, this still runs crun 1.7.2 (not sure why it's not pulling the latest yet) but it uses the right cni-plugins and thus the pods can run.

I pulled in some other package updates. Let me know if you prefer to have them split from this PR.

@spowelljr
Copy link
Member

Awesome thanks! I've made significant progress in resolving the CI infrastructure issues. The machines are running again, but there are 1,000 test jobs in the backlog, I'm working on resolving the login issue and then I can clear the backlog and we can try building this.

@spowelljr
Copy link
Member

spowelljr commented Feb 13, 2024

Sorry for the delay, just finished resolving the CI issues, let's try building this finally

@spowelljr
Copy link
Member

/ok-to-build-iso

@spowelljr
Copy link
Member

I pulled in some other package updates. Let me know if you prefer to have them split from this PR.

Due to the backup in the whole CI I'm happy to merge this with all the package updates as long as the build is successful

@travier
Copy link
Contributor Author

travier commented Feb 14, 2024

For the GitHub workflow, this fails on:

---------------- 1 Failures :( ----------------------------
[
  "TestFunctional/parallel/DashboardCmd"
]

I don't have access to the ISO PR Build Push logs.

Edit: I've tried enabling the dashboard and "it worked" as far as I could see. I've not tried running the test yet.

@spowelljr
Copy link
Member

spowelljr commented Feb 14, 2024

The DashboardCmd failure you can ignore, the test is flaky and needs to be fixed.

The ISO build failed with VCS stamping cri-dockerd

16:44:34 mkdir -p build/linux/cri-dockerd
16:44:34 cd /home/jenkins/go/src/k8s.io/minikube/out/buildroot/output-aarch64/build/cri-dockerd-aarch64-b58acf8f78f9d7bce1241d1cddb0932e7101f278 && env CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -trimpath -ldflags "-s -w -buildid=b58acf8 -X github.com/Mirantis/cri-dockerd/cmd/version.Version=0.3.3 -X github.com/Mirantis/cri-dockerd/cmd/version.PreRelease=`grep -q dev <<< "0.3.3" && echo "pre" || echo ""` -X github.com/Mirantis/cri-dockerd/cmd/version.GitCommit=b58acf8" -o cri-dockerd
16:44:35 error obtaining VCS status: exit status 128
16:44:35 	Use -buildvcs=false to disable VCS stamping.

Also, looking at the log we're in an aarch64 (output-aarch64/build/cri-dockerd-aarch64) directory, but the build command is using amd64 (GOARCH=amd64 go build).

Edit: Looks like you added the disable VCS for x86 but forgot to add it to aarch64, so that should be a simple fix.

Edit 2: It's using amd64 because we're using the make command in cri-dockerd's Makefile, which is:

GOARCH=$(ARCH) go build -trimpath $(CRI_DOCKERD_LDFLAGS) -o $@

And since the machine building the ISO is amd64 it's setting GOARCH to amd64.

@travier travier force-pushed the buildroot-update branch 2 times, most recently from fe5dab7 to 9b496d8 Compare February 15, 2024 09:52
@spowelljr
Copy link
Member

/ok-to-build-iso

@travier
Copy link
Contributor Author

travier commented Feb 15, 2024

Updated to fix the issues above. It builds on x86_64 for me. Not tested.

@travier
Copy link
Contributor Author

travier commented Feb 22, 2024

Thanks! Updated.

@spowelljr
Copy link
Member

ok-to-build-iso

@minikube-bot
Copy link
Collaborator

Hi @travier, 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

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Feb 23, 2024
@minikube-pr-bot
Copy link

kvm2 driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 18020) |
+----------------+----------+---------------------+
| minikube start | 51.9s    | 50.5s               |
| enable ingress | 27.8s    | 24.9s               |
+----------------+----------+---------------------+

Times for minikube ingress: 27.6s 27.0s 28.6s 28.1s 27.6s
Times for minikube (PR 18020) ingress: 23.6s 26.7s 24.1s 23.1s 27.2s

Times for minikube start: 52.5s 49.7s 52.4s 49.7s 55.1s
Times for minikube (PR 18020) start: 49.4s 50.4s 51.4s 49.9s 51.7s

docker driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 18020) |
+----------------+----------+---------------------+
| minikube start | 24.4s    | 23.2s               |
| enable ingress | 20.8s    | 21.3s               |
+----------------+----------+---------------------+

Times for minikube ingress: 20.8s 20.8s 20.8s 20.8s 20.8s
Times for minikube (PR 18020) ingress: 20.9s 21.8s 20.9s 21.8s 20.9s

Times for minikube start: 25.6s 25.9s 24.5s 21.7s 24.5s
Times for minikube (PR 18020) start: 21.7s 24.8s 21.8s 25.2s 22.7s

docker driver with containerd runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 18020) |
+----------------+----------+---------------------+
| minikube start | 21.7s    | 22.8s               |
| enable ingress | 31.1s    | 34.5s               |
+----------------+----------+---------------------+

Times for minikube start: 20.8s 20.4s 20.2s 23.6s 23.6s
Times for minikube (PR 18020) start: 23.9s 20.9s 20.6s 23.6s 25.0s

Times for minikube ingress: 31.3s 31.4s 31.3s 31.3s 30.3s
Times for minikube (PR 18020) ingress: 31.3s 31.3s 31.4s 31.4s 47.4s

@minikube-pr-bot
Copy link

These are the flake rates of all failed tests.

Environment Failed Tests Flake Rate (%)
Docker_Linux_containerd TestDownloadOnly/v1.29.0-rc.2/json-events (gopogh) 0.00 (chart)
Docker_Linux_crio TestDownloadOnly/v1.29.0-rc.2/json-events (gopogh) 0.00 (chart)
Hyperkit_macOS TestIngressAddonLegacy/serial/ValidateIngressDNSAddonActivation (gopogh) 0.00 (chart)
Hyper-V_Windows TestDockerFlags (gopogh) 0.00 (chart)
Hyper-V_Windows TestIngressAddonLegacy/serial/ValidateIngressAddonActivation (gopogh) 0.00 (chart)
Hyper-V_Windows TestIngressAddonLegacy/StartLegacyK8sCluster (gopogh) 0.00 (chart)
Hyper-V_Windows TestNetworkPlugins/group/flannel/NetCatPod (gopogh) 0.00 (chart)
KVM_Linux_containerd TestIngressAddonLegacy/serial/ValidateIngressAddonActivation (gopogh) 0.00 (chart)
KVM_Linux_containerd TestIngressAddonLegacy/serial/ValidateIngressAddons (gopogh) 0.00 (chart)
KVM_Linux_containerd TestIngressAddonLegacy/serial/ValidateIngressDNSAddonActivation (gopogh) 0.00 (chart)
KVM_Linux_containerd TestIngressAddonLegacy/StartLegacyK8sCluster (gopogh) 0.00 (chart)
KVM_Linux_containerd TestStartStop/group/old-k8s-version/serial/AddonExistsAfterStop (gopogh) 0.00 (chart)
KVM_Linux_containerd TestStartStop/group/old-k8s-version/serial/DeployApp (gopogh) 0.00 (chart)
KVM_Linux_containerd TestStartStop/group/old-k8s-version/serial/EnableAddonWhileActive (gopogh) 0.00 (chart)
KVM_Linux_containerd TestStartStop/group/old-k8s-version/serial/FirstStart (gopogh) 0.00 (chart)
KVM_Linux_containerd TestStartStop/group/old-k8s-version/serial/SecondStart (gopogh) 0.00 (chart)
KVM_Linux_containerd TestStartStop/group/old-k8s-version/serial/UserAppExistsAfterStop (gopogh) 0.00 (chart)
KVM_Linux_crio TestIngressAddonLegacy/serial/ValidateIngressAddonActivation (gopogh) 0.00 (chart)
KVM_Linux_crio TestIngressAddonLegacy/serial/ValidateIngressDNSAddonActivation (gopogh) 0.00 (chart)
KVM_Linux_crio TestIngressAddonLegacy/StartLegacyK8sCluster (gopogh) 0.00 (chart)
KVM_Linux_crio TestKubernetesUpgrade (gopogh) 0.00 (chart)
KVM_Linux_crio TestStartStop/group/old-k8s-version/serial/DeployApp (gopogh) 0.00 (chart)
KVM_Linux_crio TestStartStop/group/old-k8s-version/serial/EnableAddonWhileActive (gopogh) 0.00 (chart)
KVM_Linux TestIngressAddonLegacy/serial/ValidateIngressAddonActivation (gopogh) 0.00 (chart)
KVM_Linux TestIngressAddonLegacy/serial/ValidateIngressAddons (gopogh) 0.00 (chart)
KVM_Linux TestIngressAddonLegacy/serial/ValidateIngressDNSAddonActivation (gopogh) 0.00 (chart)
KVM_Linux TestIngressAddonLegacy/StartLegacyK8sCluster (gopogh) 0.00 (chart)
KVM_Linux TestKubernetesUpgrade (gopogh) 0.00 (chart)
KVM_Linux TestStartStop/group/old-k8s-version/serial/AddonExistsAfterStop (gopogh) 0.00 (chart)
KVM_Linux TestStartStop/group/old-k8s-version/serial/DeployApp (gopogh) 0.00 (chart)
More tests... Continued...

Too many tests failed - See test logs for more details.

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

@travier
Copy link
Contributor Author

travier commented Feb 24, 2024

I've started looking at TestStartStop/group/old-k8s-version/serial/DeployApp and it's trying to run Kubernetes 1.16 with the latest cri-o, which is likely not going to work as cri-o is "version"-locked with kubernetes.

I have not looked at the other failures yet.

@spowelljr
Copy link
Member

We talked about it in office hours today, basically as of 1.28(?) the cri-o is built for the matching version of Kubernetes and there is no backwards compatibility guaranteed. So the updated cri-o is not working with the old version of Kubernetes we're testing with (1.16). As long as it works with a newer version say 1.24 then I'm happy to bump the "old" version we use for testing.

@spowelljr
Copy link
Member

This worked down to K8s v1.20 for me, so looks good to me

@spowelljr spowelljr merged commit d4e0a20 into kubernetes:master Feb 28, 2024
22 of 41 checks passed
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

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

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 Feb 28, 2024
@spowelljr
Copy link
Member

Thank you very much for your continued pursuit of this PR @travier, happy to be back on the current version of Buildroot, and unblocks using Go 1.21 and will allow us to install new packages that were blocked like Docker buildx

@travier travier deleted the buildroot-update branch February 29, 2024 14:26
@travier
Copy link
Contributor Author

travier commented Feb 29, 2024

@spowelljr Thanks for the help and reviews! I'll likely be looking at crun and other components updates now.

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/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bump ISO Go version back up to 1.20 after buildroot fix
7 participants