Skip to content
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

Fix tolerations for Kubernetes 1.24 #3731

Merged
merged 1 commit into from
May 6, 2022

Conversation

xliuxu
Copy link
Contributor

@xliuxu xliuxu commented May 5, 2022

The taints and labels for control-plane Nodes will be changed for
cluster version >= 1.24.
kubeadm v1.24.0 will add both taints for control-plane Nodes and
it seems to be a bug. But adding both tolerations has no side effects.

Signed-off-by: Xu Liu [email protected]

tnqn
tnqn previously approved these changes May 5, 2022
Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

kubeadm v1.24.0 will add both taints for control-plane Nodes and it seems to be a bug. But adding both tolerations has no side effects.

It seems an intended change: https://github.com/kubernetes/kubernetes/blame/master/CHANGELOG/CHANGELOG-1.24.md#L330

For new clusters, both the old taint node-role.kubernetes.io/master:NoSchedule and new taint node-role.kubernetes.io/control-plane:NoSchedule will be added to control plane nodes.

@codecov-commenter
Copy link

codecov-commenter commented May 5, 2022

Codecov Report

Merging #3731 (4399c2b) into main (2065919) will decrease coverage by 0.64%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3731      +/-   ##
==========================================
- Coverage   64.60%   63.95%   -0.65%     
==========================================
  Files         278      278              
  Lines       39640    39753     +113     
==========================================
- Hits        25608    25426     -182     
- Misses      12043    12370     +327     
+ Partials     1989     1957      -32     
Flag Coverage Δ
e2e-tests 46.16% <ø> (?)
kind-e2e-tests 50.13% <ø> (-2.90%) ⬇️
unit-tests 43.69% <ø> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/controller/egress/store/egressgroup.go 1.72% <0.00%> (-54.32%) ⬇️
pkg/util/k8s/name.go 8.00% <0.00%> (-24.00%) ⬇️
pkg/agent/route/route_linux.go 30.17% <0.00%> (-19.63%) ⬇️
pkg/agent/util/net.go 32.44% <0.00%> (-19.12%) ⬇️
pkg/agent/openflow/service.go 66.66% <0.00%> (-17.25%) ⬇️
pkg/agent/controller/networkpolicy/reject.go 76.47% <0.00%> (-9.42%) ⬇️
pkg/agent/util/ipset/ipset.go 59.45% <0.00%> (-8.11%) ⬇️
pkg/agent/multicast/mcast_controller.go 61.38% <0.00%> (-6.07%) ⬇️
pkg/util/k8s/client.go 35.71% <0.00%> (-5.36%) ⬇️
pkg/controller/egress/validate.go 62.29% <0.00%> (-4.92%) ⬇️
... and 27 more

@xliuxu
Copy link
Contributor Author

xliuxu commented May 5, 2022

Thanks @tnqn. I noticed this PR also includes the fix in #3728. Please help to merge this PR after that.

@tnqn
Copy link
Member

tnqn commented May 6, 2022

@xliuxu could you resolve conflicts?

@xliuxu xliuxu force-pushed the 1.24_e2e_label_taints_fix branch 2 times, most recently from 55165d1 to 9384991 Compare May 6, 2022 03:11
@xliuxu xliuxu changed the title Fix e2e tests for Kubernetes 1.24 Fix tolerations in e2e tests for Kubernetes 1.24 May 6, 2022
@tnqn
Copy link
Member

tnqn commented May 6, 2022

@xliuxu could you also update tolerations of antrea-controller?

# Allow it to schedule onto master nodes.
- key: node-role.kubernetes.io/master
effect: NoSchedule

Otherwise antrea-controller will never be scheduled to control-plane node. The title may need adjustment after that.

@xliuxu xliuxu force-pushed the 1.24_e2e_label_taints_fix branch 3 times, most recently from 9384991 to db794a1 Compare May 6, 2022 03:40
@xliuxu xliuxu changed the title Fix tolerations in e2e tests for Kubernetes 1.24 Fix tolerations for Kubernetes 1.24 May 6, 2022
@xliuxu
Copy link
Contributor Author

xliuxu commented May 6, 2022

@tnqn Done. Thanks!

@@ -203,6 +203,9 @@ controller:
# Allow it to schedule onto master nodes.
- key: node-role.kubernetes.io/master
effect: NoSchedule
# Control-plane taint since Kubernetes >= 1.24.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Control-plane taint since Kubernetes >= 1.24.
# Control-plane taint since Kubernetes 1.24.

or

Suggested change
# Control-plane taint since Kubernetes >= 1.24.
# Control-plane taint for Kubernetes >= 1.24.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@xliuxu xliuxu force-pushed the 1.24_e2e_label_taints_fix branch from db794a1 to 38523b7 Compare May 6, 2022 04:12
tnqn
tnqn previously approved these changes May 6, 2022
Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tnqn
Copy link
Member

tnqn commented May 6, 2022

/test-all

The taints for control-plane Nodes are changed for cluster version
>= 1.24. Add a new toleration for Pods running on control-plane
Nodes to make sure they can be scheduled.

Signed-off-by: Xu Liu <[email protected]>
@xliuxu
Copy link
Contributor Author

xliuxu commented May 6, 2022

/test-all

@tnqn tnqn added action/release-note Indicates a PR that should be included in release notes. action/backport Indicates a PR that requires backports. labels May 6, 2022
@tnqn tnqn merged commit 7673d42 into antrea-io:main May 6, 2022
@tnqn
Copy link
Member

tnqn commented May 7, 2022

@xliuxu could you please backport this PR to release 1.5 and 1.6?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action/backport Indicates a PR that requires backports. action/release-note Indicates a PR that should be included in release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants