🌱 Add toleration for 'node.cloudprovider.kubernetes.io/uninitialized'#41098
Conversation
7e856c2 to
c9ebcae
Compare
|
There was a network issue:
Unfortunately I see no way to run the test again. |
gandro
left a comment
There was a problem hiding this comment.
Thanks for the PR and context! Looks good to me!
Only organization members can run/restart CI. I'll restart CI for you! |
|
I think you also have to update the auto-generated Helm value documentation. Run the following commands to achieve that: checkpatch is also complaining about the long subject in the commit message, but this is an optional workflow an can be ignored. The subject is fine as is. |
|
c9ebcae to
4df5566
Compare
When Kubelet gets started with --cloud-provider=external, then new Nodes get that annotation. CCM picks these new nodes and sets then ProviderID. But before CCM can start on the first control-plane of a new cluster, the CNI must be running. This means Cilum Operator needs a toleration for that taint. Related: https://app.slack.com/client/T1MATJ4SZ/C53TG4J4R In Cilium v1.17 the Cilium Operator had a toleration for all taints. This was changed in that PR: cilium#40475 This PR extends the list of tolerations. Fixes: aa9a24c (Change the default taints that Cilium tolerates to avoid deploying to a drained node) Signed-off-by: Thomas Guettler <thomas.guettler@syself.com>
4df5566 to
39f4ff7
Compare
Thank you for the hint. I only run I pushed after running your command. Please re-check
I trimmed the git subject line. |
|
@parlakisik @gandro The generation of the values.schema.json looks strange. There is "tolerations": {
"items": {
"anyOf": [
{
"properties": {
"key": {
"type": "string"
},
"operator": {
"type": "string"
}
}
},
{
"properties": {
"key": {
"type": "string"
},
"operator": {
"type": "string"
}
}
},
{
"properties": {
"key": {
"type": "string"
},
"operator": {
"type": "string"
}
}
},
{
"properties": {
"key": {
"type": "string"
},
"operator": {
"type": "string"
}
}
}
] |
|
I think there is a bug in the schema generation. That we should investigate. that is normal for now . |
|
Looks good to me! |
gandro
left a comment
There was a problem hiding this comment.
Thanks!
Yeah, both our helm docs and schema generators sometimes do strange things. We've noticed the strange schema in earlier PRs as well, but it doesn't prevent anyone from setting custom tolerations (I've tested that).
|
/test |
I created an issue. Maybe someone wants to fix that: values.schema.json contain duplicate data · Issue #41099 · cilium/cilium |
When Kubelet gets started with --cloud-provider=external, then new Nodes get the Taint 'node.cloudprovider.kubernetes.io/uninitialized'. CCM picks these new nodes and sets then ProviderID. But before CCM can start on the first control-plane of a new cluster, the CNI must be running. This means Cilium Operator needs a toleration for that taint. Otherwise, the first node (control-plane) in a cluster fails to start.
Related: https://app.slack.com/client/T1MATJ4SZ/C53TG4J4R
In Cilium v1.17 the Cilium Operator had a toleration for all taints. This was changed in that PR: #40475 This PR extends the list of tolerations.
Fixes: aa9a24c (Change the default taints that Cilium tolerates to avoid deploying to a drained node)
Please ensure your pull request adheres to the following guidelines:
For first time contributors, read Submitting a pull request
All code is covered by unit and/or runtime tests where feasible.
Yes, a test with
--cloud-provider=external(Kubelet argument) would be good. But not doable for me at the moment. I can assist if you want to. Maybe this could be done with Cluster-API and InfraProvider Docker.description and a
Fixes: #XXXline if the commit addresses a particularGitHub issue.
Needed?
Fixes: <commit-id>tag, thenplease add the commit author[s] as reviewer[s] to this issue.
I tried, but I guess I do not have permission to set a reviewer. I wrote him via Slack.