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

Filter out the hostNetwork Pods locally on Linux #7012

Merged
merged 1 commit into from
Feb 21, 2025

Conversation

wenyingd
Copy link
Contributor

@wenyingd wenyingd commented Feb 21, 2025

This change is to resolve the issue that "spec.hostNetwork" is not supported as Pod's field selector since K8s v1.28, so we may hit issues if antrea run on a cluster with version [1.19, 1.27] .

The fix is to remove the field selector "spec.hostNetwork" in the Pod list options, and locally filters out the hostNetwork Pod on Linux.

@wenyingd wenyingd force-pushed the issue_7002_local_filter branch 3 times, most recently from e73cf12 to d9e0df8 Compare February 21, 2025 03:53
@wenyingd wenyingd requested review from antoninbas and tnqn February 21, 2025 03:54
func(options *metav1.ListOptions) {
options.FieldSelector = fields.OneTermEqualSelector("spec.hostNetwork", "false").String()
},
nil,
Copy link
Contributor

Choose a reason for hiding this comment

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

the informer no longer needs to be filtered, you can revert back to the old version of the code since this was introduced recently

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, updated.

@luolanzone luolanzone added this to the Antrea v2.3 release milestone Feb 21, 2025
@wenyingd wenyingd force-pushed the issue_7002_local_filter branch from d9e0df8 to aca6432 Compare February 21, 2025 05:29
@luolanzone
Copy link
Contributor

/test-all

@luolanzone
Copy link
Contributor

@wenyingd can you check the failure of the kind e2e tests?

@wenyingd wenyingd changed the title [CNIServer] Filter out the hostNetwork Pods locally on Linux Filter out the hostNetwork Pods locally on Linux Feb 21, 2025
@wenyingd
Copy link
Contributor Author

@wenyingd can you check the failure of the kind e2e tests?

It looks the failed cases are not related, all cases are passed after I re-run them.

@wenyingd
Copy link
Contributor Author

/test-e2e

@wenyingd
Copy link
Contributor Author

/test-windows-all

tnqn
tnqn previously approved these changes Feb 21, 2025
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, minor comments about readability

This change is to resolve the issue that "spec.hostNetwork" is not supported as
Pod's field selector since K8s v1.28, so we may hit issues if antrea run on a
cluster with version [1.19, 1.27] .

The fix is to remove the field selector "spec.hostNetwork" in the Pod list
options, and locally filter out the hostNetwork Pods on Linux. This fix includes
changes in both CNIServer and flow-aggregator.

Signed-off-by: Wenying Dong <wenyingd@vmware.com>
@wenyingd
Copy link
Contributor Author

/test-all
/test-windows-all

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

@luolanzone
Copy link
Contributor

Integration test is failed, I rerun it, I noticed the same failure in the release PR, not sure if there is a bug caused the flaky failure.

@wenyingd
Copy link
Contributor Author

Integration test is failed, I rerun it, I noticed the same failure in the release PR, not sure if there is a bug caused the flaky failure.

I also hit the integration test failure in another CI related change (without any function code change).

@luolanzone luolanzone merged commit 2fbb1b9 into antrea-io:main Feb 21, 2025
58 of 60 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants