-
Notifications
You must be signed in to change notification settings - Fork 260
[backport] - (#3776)(#3768) perf: update error message for MTPNC not found and not ready in CNS #3775
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
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.
Pull Request Overview
This PR updates the network readiness error message so that kubelet will recognize it and retry faster, aligning with upstream expectations.
- Prefixes the existing mtpnc-not-ready error with “network is not ready”
- Adds an inline comment linking to the kubelet source that checks for this string
- (Unit tests and documentation updates included in the cherry-picked PR)
Comments suppressed due to low confidence (2)
cns/middlewares/k8sSwiftV2.go:22
- Ensure there is a unit test that specifically asserts this error message starts with "network is not ready", so kubelet will detect and retry as intended.
errMTPNCNotReady = errors.New("network is not ready - mtpnc is not ready")
cns/middlewares/k8sSwiftV2.go:22
- [nitpick] The variable name
errMTPNCNotReadynow covers network readiness; consider renaming it toerrNetworkNotReadyor similar to better reflect its scope.
errMTPNCNotReady = errors.New("network is not ready - mtpnc is not ready")
|
@isaac-dasan I can see that this PR is a cherry-pick and the diff is tiny, but we have been talking about having all changes being under UT coverage, could we please add one? One approach that works pretty well for all fixes is to add an assertion that fails without your fix, but passes with the fix. |
santhoshmprabhu
left a comment
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.
UT will be added when we cherry pick #3776 to 1.6.
00d66ee
|
/azp run Azure Container Networking PR |
|
Azure Pipelines successfully started running 1 pipeline(s). |
update network error msg to match kubelet expectations Signed-off-by: GitHub <[email protected]>
…3776) squash commit to prevent github CLA issue Signed-off-by: GitHub <[email protected]>
00d66ee to
6da6ad6
Compare
|
/azp run Azure Container Networking PR |
|
Azure Pipelines successfully started running 1 pipeline(s). |
update network error msg to match kubelet expectations
cherry pick from master of this
Requirements:
Notes: