Eliminate half-baked multi-architecture support #35124
Conversation
k8s-github-robot
added
size/M
|
@luxas you're the ARM ninja around, care to test? |
|
Jenkins GCI GKE smoke e2e failed for commit 4e121e6. Full PR test history. The magic incantation to run this job again is |
jbeda
added
release-note
| } | ||
| } | ||
|
|
||
| func MonoArchitectureNodeAffinity() api.NodeSelectorRequirement { |
There was a problem hiding this comment.
The term "Mono" here makes me think of Mono and windows and such. Perhaps just "ArchitectureNodeAffinity"? If we add multi-arch support in the future we can rename/rework this.
There was a problem hiding this comment.
Haha, happy to rename. What about NativeArchitectureNodeAffinity()?
There was a problem hiding this comment.
NativeArchitectureNodeAffinity SGTM
| @@ -206,14 +207,10 @@ func SetMasterTaintTolerations(meta *api.ObjectMeta) { | |||
| meta.Annotations[api.TolerationsAnnotationKey] = string(tolerationsAnnotation) | |||
| } | |||
|
|
|||
| func SetMasterNodeAffinity(meta *api.ObjectMeta) { | |||
| func SetNodeAffinity(meta *api.ObjectMeta, expr ...api.NodeSelectorRequirement) { | |||
There was a problem hiding this comment.
We don't have documentation on public functions here and we should probably start. Can you start documenting some of this stuff as you touch it?
| @@ -223,6 +220,18 @@ func SetMasterNodeAffinity(meta *api.ObjectMeta) { | |||
| meta.Annotations[api.AffinityAnnotationKey] = string(affinityAnnotation) | |||
| } | |||
|
|
|||
| func MasterNodeAffinity() api.NodeSelectorRequirement { | |||
| } | ||
| } | ||
|
|
||
| func MonoArchitectureNodeAffinity() api.NodeSelectorRequirement { |
…ty architecture-agnostic (fix kubernetes#33916)
errordeveloper
force-pushed
the
fix-33916
branch
from
4e121e6
to
9703df3
Compare
|
@jbeda I've addressed your comments and rebased. |
|
Thanks! LGTM |
jbeda
added
the
lgtm
|
@k8s-bot test this [submit-queue is verifying that this PR is safe to merge] |
1 similar comment
|
@k8s-bot test this [submit-queue is verifying that this PR is safe to merge] |
|
Automatic merge from submit-queue |
What this PR does / why we need it:
We have release
kubeadmwith half-baked support for clusters with nodes of different CPU architectures. The problem with the code as it stand is that user will notice pending daemonsets ofkube-proxyfor machines with architectures that they don't have. At the same time, the code as it stand did not pick up correct images for architectures it wanted to allow. Additionally, it only treatedkube-proxyin such a way, but didn't do anything aboutkube-dns. This removes multiple daemonesets, but ensures that whichever resources we deploy have node affinity set to the architecture native to the master. Users wishing to use mixed architectures can still create extra daemonsets via the API.Which issue this PR fixes: fixes #33916
Release note:
This change is