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 identification of control-plane Node in e2e test framework #3728

Conversation

antoninbas
Copy link
Contributor

We use the node-role.kubernetes.io/<ROLE> label to identify the
control-plane Node. The <ROLE> we use needs to depend on the K8s
version because:

  • before K8s v1.20, the label is node-role.kubernetes.io/master
  • starting with K8s v1.24, the label is node-role.kubernetes.io/control-plane.
  • in between, both labels are used by K8s

However, the function used to determine the correct label to use based
on the K8s version was called before the K8s server version was
actually determined, so it was always returning
node-role.kubernetes.io/master. This was causing tests to fail for K8s
v1.24 (which has just been released), since the label no longer exists
in that version.

Signed-off-by: Antonin Bas abas@vmware.com

We use the `node-role.kubernetes.io/<ROLE>` label to identify the
control-plane Node. The `<ROLE>` we use needs to depend on the K8s
version because:
- before K8s v1.20, the label is `node-role.kubernetes.io/master`
- starting with K8s v1.24, the label is `node-role.kubernetes.io/control-plane`.
- in between, both labels are used by K8s

However, the function used to determine the correct label to use based
on the K8s version was called *before* the K8s server version was
actually determined, so it was always returning
`node-role.kubernetes.io/master`. This was causing tests to fail for K8s
v1.24 (which has just been released), since the label no longer exists
in that version.

Signed-off-by: Antonin Bas <abas@vmware.com>
@codecov-commenter
Copy link

codecov-commenter commented May 4, 2022

Codecov Report

Merging #3728 (d478bf8) into main (8f451f7) will decrease coverage by 7.80%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3728      +/-   ##
==========================================
- Coverage   64.44%   56.63%   -7.81%     
==========================================
  Files         278      392     +114     
  Lines       39513    55091   +15578     
==========================================
+ Hits        25463    31201    +5738     
- Misses      12068    21459    +9391     
- Partials     1982     2431     +449     
Flag Coverage Δ
integration-tests 38.05% <ø> (?)
kind-e2e-tests 51.95% <ø> (-0.40%) ⬇️
unit-tests 43.82% <ø> (-0.03%) ⬇️

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/agent/util/net.go 29.33% <0.00%> (-22.23%) ⬇️
pkg/agent/route/route_linux.go 30.27% <0.00%> (-19.53%) ⬇️
pkg/agent/util/ipset/ipset.go 59.45% <0.00%> (-8.11%) ⬇️
pkg/agent/openflow/service.go 78.16% <0.00%> (-5.75%) ⬇️
pkg/util/k8s/client.go 35.71% <0.00%> (-5.36%) ⬇️
pkg/controller/egress/validate.go 62.29% <0.00%> (-4.92%) ⬇️
pkg/agent/flowexporter/utils.go 72.34% <0.00%> (-4.26%) ⬇️
...agent/flowexporter/connections/deny_connections.go 80.45% <0.00%> (-3.45%) ⬇️
...g/controller/networkpolicy/store/appliedtogroup.go 87.61% <0.00%> (-2.86%) ⬇️
... and 125 more

@tnqn
Copy link
Member

tnqn commented May 5, 2022

/test-e2e
/skip-networkpolicy
/skip-conformance

@XinShuYang
Copy link
Contributor

/test-e2e

@XinShuYang
Copy link
Contributor

/test-ipv6-e2e
/test-ipv6-only-e2e

@antoninbas antoninbas merged commit 2065919 into antrea-io:main May 5, 2022
@antoninbas antoninbas deleted the fix-identification-of-control-plane-node-in-e2e-framework branch May 5, 2022 22:24
@tnqn
Copy link
Member

tnqn commented May 6, 2022

@antoninbas do you think this PR and #3731 should be backported to make antrea 1.5 and 1.6 work with K8s 1.24 better?

@antoninbas
Copy link
Contributor Author

Yes we can backport. I'll open the PRs for this one. #3731 is more important since it affects controller scheduling.

@antoninbas antoninbas added the kind/cherry-pick Categorizes issue or PR as related to the cherry-pick of a bug fix from the main branch to a release label May 6, 2022
@antoninbas antoninbas added action/backport Indicates a PR that requires backports. and removed kind/cherry-pick Categorizes issue or PR as related to the cherry-pick of a bug fix from the main branch to a release labels May 6, 2022
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants