🌱 api: relax validation for Machine .status.addresses to maximum of 256 instead of 128 items#13395
Conversation
… instead of 128 items
|
Welcome @MadJlzz! |
|
Hi @MadJlzz. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Tip We noticed you've done this a few times! Consider joining the org to skip this step and gain Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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-sigs/prow repository. |
|
/hold Let's try to get an answer why all Pod IPs are added as Node IPs, that sounds just wrong to me |
|
Okay let's go ahead with this. But to be clear. I think it should be investigated if this pattern of setting Pod IPs as Machine IPs is really correct. In general I'm -1 on keep increasing the number of supported Machine IPs if someone wants to run more Pods on Nodes and CAPZ keeps adding Pod IPs as Machine IPs (except if we have a clear story why this makes sense and is semantically correct) /ok-to-test |
|
LGTM label has been added. DetailsGit tree hash: ccf0aa3312b262f1e85fd78bbfc5f6034ffa0379 |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sbueringer The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/cherry-pick release-1.12 |
|
/cherry-pick release-1.11 |
|
@sbueringer: once the present PR merges, I will cherry-pick it on top of DetailsIn response to this:
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-sigs/prow repository. |
|
@sbueringer: once the present PR merges, I will cherry-pick it on top of DetailsIn response to this:
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-sigs/prow repository. |
|
@sbueringer: new pull request created: #13399 DetailsIn response to this:
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-sigs/prow repository. |
|
@sbueringer: new pull request created: #13400 DetailsIn response to this:
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-sigs/prow repository. |
…
What this PR does / why we need it:
This PR relaxes even more the validation which was added in v1.11 to allow more entries in the Machine's .status.addresses slice.
I've encountered this validation error while trying to upgrade a cluster from
1.32to1.34. I did upgrade capi providers which introduced this initial validation inv1.11I am using Azure's CNI as per my network provider with a
networkInterfaces.privateIPConfigsof 180 and amax-podsof 180 in the kubeletExtraArgs opts.In fact, the following error:
was blocking any kind of other patches ~ in my case, the nodeRef value allowing capi to understand a node has successfully rolled out. (with a side effect of never beeing able fully upgrade a cluster)
I am still not sure why they're putting all pods IPs inside the machine's
status.addressesthough...This PR is a follow up of #13060
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #
/area api