-
Notifications
You must be signed in to change notification settings - Fork 260
chore: remove legacy kube-init and default to CNI state #3383
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
Conversation
|
/azp run Azure Container Networking PR |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
This pull request is stale because it has been open for 2 weeks with no activity. Remove stale label or comment or this will be closed in 7 days |
|
Pull request closed due to inactivity. |
c942654 to
3b9b475
Compare
|
/azp run Azure Container Networking PR |
|
Azure Pipelines successfully started running 1 pipeline(s). |
3b9b475 to
793abc9
Compare
|
/azp run Azure Container Networking PR |
|
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Overview
This PR removes legacy Kubernetes initialization code and updates the system to default to using the CNI state for pod information. Key changes include:
- Introducing a new pod info provider in the cni package.
- Renaming and consolidating packages from “cnireconciler” to “cni” and “cns” where applicable.
- Removing legacy version-checking functions and associated tests.
Reviewed Changes
| File | Description |
|---|---|
| cns/stateprovider/cni/podinfoprovider.go | New implementation of the CNI pod info provider. |
| cns/stateprovider/cns/podinfoprovider.go | Updates to use endpoint state from the CNS store instead of legacy CNI state migration. |
| cns/NetworkContainerContract.go | Changes to the key schema for PodInfo, defaulting to PodInterfaceID. |
| cns/stateprovider/cni/statefile_test.go | Package rename from cnireconciler to cni. |
| cns/stateprovider/cni/podinfoprovider_test.go | Test updates matching package renames and function call changes. |
| cns/stateprovider/cni/statefile.go | Package rename from cnireconciler to cni. |
| cns/service/main.go | Updated calls to use the new cni and cns pod info providers and removal of legacy CNI version checks. |
| cns/restserver/internalapi_test.go | Minor test adjustments due to GlobalPodInfoScheme changes. |
| cns/nodesubnet/initialization_test.go | Test updates to use the new CNS pod info provider. |
| cns/NetworkContainerContract_test.go | Removal of deferred reset for GlobalPodInfoScheme. |
| cns/restserver/nodesubnet_test.go | Package and function call updates to use the new CNS and CNI pod info providers. |
| cns/cnireconciler/version*.go | Legacy version checking functions and tests removed. |
Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (2)
cns/NetworkContainerContract.go:289
- Ensure that using p.PodInterfaceID as the default key is the intended behavior, and that the PodInterfaceID is guaranteed to be unique. If uniqueness cannot be guaranteed, consider using a composite key or revisiting the key schema.
return p.PodInterfaceID
cns/NetworkContainerContract_test.go:60
- Consider restoring GlobalPodInfoScheme to its original value at the end of tests (or using proper test isolation) to prevent side effects on subsequent tests.
GlobalPodInfoScheme = InterfaceIDPodInfoScheme
793abc9 to
6b5f07e
Compare
|
/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). |
|
This pull request is stale because it has been open for 2 weeks with no activity. Remove stale label or comment or this will be closed in 7 days |
|
This pull request is stale because it has been open for 2 weeks with no activity. Remove stale label or comment or this will be closed in 7 days |
|
Pull request closed due to inactivity. |
|
This pull request is stale because it has been open for 2 weeks with no activity. Remove stale label or comment or this will be closed in 7 days |
Signed-off-by: Evan Baker <[email protected]>
Signed-off-by: Evan Baker <[email protected]>
Signed-off-by: GitHub <[email protected]>
143ff7c to
e6a9160
Compare
|
/azp run Azure Container Networking PR |
|
Azure Pipelines successfully started running 1 pipeline(s). |
* chore: remove legacy kube-init and default to CNI state Signed-off-by: Evan Baker <[email protected]> * refactor podinfoproviders Signed-off-by: Evan Baker <[email protected]> * fix lints Signed-off-by: GitHub <[email protected]> --------- Signed-off-by: Evan Baker <[email protected]> Signed-off-by: GitHub <[email protected]>
Reason for Change:
Removes the legacy Kubernetes init and makes CNI init the code default. 🚮
Reconcile initial state from CNI has been the default (via config) since CNI 1.4.7 (so at least AKS 1.27). It's been in use for so long that we're starting to roll off of it, also, to the new CNS managed state.
Issue Fixed:
Requirements:
Notes: