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

ignore host network pods in all endpoints #89

Closed
wants to merge 1 commit into from
Closed

Conversation

haouc
Copy link
Contributor

@haouc haouc commented Mar 15, 2024

What type of PR is this?

bug
Which issue does this PR fix:
We shouldn't populate pods in host network to PolicyEndpoint's endpoints.

What does this PR do / Why do we need it:
The endpoints from host network pods in ingress/egress/podselector can become redundant data in nodes and CRs.

If an issue # is not available please add steps to reproduce and the controller logs:

Testing done on this change:

Tested in local dev cluster and cyclonus tests were passed.

Automation added to e2e:

Will this PR introduce any new dependencies?:

Will this break upgrades or downgrades. Has updating a running cluster been tested?:

Does this PR introduce any user-facing change?:


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@haouc haouc requested a review from a team as a code owner March 15, 2024 22:05
@haouc haouc force-pushed the main branch 7 times, most recently from 75db615 to 6ff6d2d Compare March 16, 2024 00:04
@achevuru
Copy link
Contributor

Let's add a simple test result with host networking pods matching ingress, egress and pod selector endpoint selectors..

@@ -491,3 +497,22 @@ func dedupPorts(policyPorts []policyinfo.Port) []policyinfo.Port {
}
return nil
}

func (r *defaultEndpointsResolver) getPodList(ctx context.Context, opts client.ListOption) ([]corev1.Pod, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe to make this more clear, it can be getNonHostNetworkingPodList ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated the name.

@haouc haouc force-pushed the main branch 2 times, most recently from 96aae91 to 35c65f7 Compare March 21, 2024 19:37
@haouc haouc requested a review from jayanthvn March 22, 2024 20:15
@haouc
Copy link
Contributor Author

haouc commented Mar 22, 2024

Let's add a simple test result with host networking pods matching ingress, egress and pod selector endpoint selectors..

The results doc has been shared offline. Please let me know if we want to add the result here as well.

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.

3 participants