Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR updates k8s.io dependencies to newer versions and removes the dependency on k8s.io/kubernetes/pkg/kubelet. As part of this change, the NetworkNotReadyErrorMsg constant is defined locally in the codebase rather than importing it from the kubelet package.
- Updated k8s.io dependencies to v0.34.1 and removed k8s.io/kubernetes dependency
- Defined
NetworkNotReadyErrorMsgconstant locally in affected modules - Updated various indirect dependencies to compatible versions
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| go.mod | Updated k8s.io dependencies to v0.34.1 and removed k8s.io/kubernetes, updated various indirect dependencies |
| network/secondary_endpoint_client_linux.go | Added local NetworkNotReadyErrorMsg constant and updated error usage |
| network/secondary_endpoint_linux_test.go | Removed kubelet import and updated error message reference |
| cns/middlewares/k8sSwiftV2.go | Added local NetworkNotReadyErrorMsg constant but kept old kubelet references in error variables |
| cns/middlewares/k8sSwiftV2_linux_test.go | Removed kubelet import and updated test assertions |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
/azp run Azure Container Networking PR |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Paul Yu <129891899+paulyufan2@users.noreply.github.com>
…ure-container-networking into updateK8sIODependencies
|
/azp run Azure Container Networking PR |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run Azure Container Networking PR |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run Azure Container Networking PR |
|
Azure Pipelines successfully started running 1 pipeline(s). |
cns/multitenantcontroller/multitenantoperator/multitenantcrdreconciler_test.go
Show resolved
Hide resolved
|
/azp run Azure Container Networking PR |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Hi @rbtr , what version of k8s is this meant for? |
|
@pjohnst5 the main branch is currently the 1.7.x release train and deploys to AKS from 1.33-1.35 |
|
Okay thanks @rbtr , I will be looking into if without these, k8s v1.34 would break for example, unless you know that already (were these necessary for 1.34-5? Or just good hygiene?) |
* Chore: update k8s.io dependencies * Update cns/middlewares/k8sSwiftV2.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Paul Yu <129891899+paulyufan2@users.noreply.github.com> * controller-runtime needs to be updated * fix controller-runtime v0.22.1 Apply method missing * fix UT * fix linter issues * fix linter issue * fix healthz issues * fix linter issues * add new error var * upgrade apiserver to v0.34.1 --------- Signed-off-by: Paul Yu <129891899+paulyufan2@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Reason for Change:
Since sigs.k8s.io/controller-runtime v0.18.x, the default RESTMapper is discovery-based. Because the NewHealthzHandlerWithChecks() builds a client with only Scheme: cli, err := client.New(cfg, client.Options{ Scheme: scheme })
controller-runtime now hits the API discovery endpoints to map GVK→GVR before doing cli.List(...).
Our mock API server from configureLocalAPIServer() doesn’t implement discovery (and likely doesn’t return a decodable body for the list), so mapping/decoding fails → health check returns error → test expects healthy (200) but gets unhealthy.
2.Change 2: k8s.io/kubernetes/pkg/kubelet
To solve this issue after we upgrade k8s packages:
go: github.com/Azure/azure-container-networking/cns/middlewares imports
k8s.io/kubernetes/pkg/kubelet imports
k8s.io/cri-client/pkg: reading k8s.io/cri-client/go.mod at revision v0.0.0: unknown revision v0.0.0
go: github.com/Azure/azure-container-networking/cns/middlewares imports
k8s.io/kubernetes/pkg/kubelet imports
k8s.io/kubernetes/pkg/kubelet/apis/podresources imports
k8s.io/cri-client/pkg/util: reading k8s.io/cri-client/go.mod at revision v0.0.0: unknown revision v0.0.0
This error is the reason why we shouldn’t import k8s.io/kubernetes/pkg/kubelet. The k8s.io/kubernetes monorepo has local replace rules for its staging deps (like k8s.io/cri-client) that don’t work outside the tree, so Go tries to resolve a fake v0.0.0 and fails. Instead we should using the staged modules to fix this.
kubelet.NetworkNotReadyErrorMsg == NetworkNotReadyErrorMsg = "network is not ready"
so make this as a constant variable makes more sense
Issue Fixed:
This PR is to update k8s.io dependencies along with dependencies changes
Requirements:
Notes: