Add more configuration flexibility to Egress for user #7041

jainpulkit22 · 2025-03-03T09:38:12Z

Added an immutable FailurePolicy field to the Egress spec. If this field is set to "NodeSNAT" then if the egress node
is not available due to any reason then the packet transfer or we can say the traffic will go via normal Node SNAT, while in the second case where this field is set to "Drop", and if egress node is not available packet won't go out and traffic will be stuck because of unavailability of egress node.

Fixes #6988.

antrea-bot · 2025-03-03T09:38:15Z

Can one of the admins verify this patch?

jainpulkit22 · 2025-03-04T06:11:14Z

Points to note:

In case of dynamic egress, if there is no node available then egress IP is also not assigned to the egress, and so no flows are installed and traffic goes through normal Node SNAT, so its not possible to add configurable parameter for dynamic egress because in case of no node available we don't have any egress, and we don't have any deciding condition on basis of which we can drop the traffic if we want to enable strict enforcement for dynamic egress.
In case of static egress what happens is the Egress has an IP, but node is not guaranteed so we have two choices now that when egress node is not available for static egress we can either allow the traffic to go through normal Node SNAT(just similar to the working of dynamic egress), or we can drop the traffic.

My current implementation gives user the flexibility to decide whether to allow the traffic(best effort), or not(strict enforcement of egress policy).

But In ideal case won't this be too confusing for the end user that static egress is configurable and dynamic is not, so should we keep the behaviour of both the egress consistent and make sure that when there is no node available for static egress then instead of dropping the traffic/packet we should allow the traffic to pass through the normal Node SNAT, same as in the case of dynamic egress.

what's your opinion on this @tnqn @antoninbas

jainpulkit22 · 2025-03-04T06:17:10Z

Points to note:

In case of dynamic egress, if there is no node available then egress IP is also not assigned to the egress, and so no flows are installed and traffic goes through normal Node SNAT, so its not possible to add configurable parameter for dynamic egress because in case of no node available we don't have any egress, and we don't have any deciding condition on basis of which we can drop the traffic if we want to enable strict enforcement for dynamic egress.

In case of static egress what happens is the Egress has an IP, but node is not guaranteed so we have two choices now that when egress node is not available for static egress we can either allow the traffic to go through normal Node SNAT(just similar to the working of dynamic egress), or we can drop the traffic.

My current implementation gives user the flexibility to decide whether to allow the traffic(best effort), or not(strict enforcement of egress policy).

But In ideal case won't this be too confusing for the end user that static egress is configurable and dynamic is not, so should we keep the behaviour of both the egress consistent and make sure that when there is no node available for static egress then instead of dropping the traffic/packet we should allow the traffic to pass through the normal Node SNAT, same as in the case of dynamic egress.

what's your opinion on this @tnqn @antoninbas

jainpulkit22 · 2025-03-20T07:00:50Z

@antoninbas @tnqn I have updated the code making both dynamic and static egress configurable, based on the set failurePolicy which is an enum field in Egress Spec and supports two values 1) "Drop" 2) "NodeSNAT", if the field is set to 1 then if the egress node is not available the packet will be dropped in both the kind of egresses, while if the field is set to 2 then if the egress node is not available the packet will be forwarded via local NodeSNAT.
The above field is currently set to be immutable, also if the egress node is available then the above field will not take any effect and the packet will go through the egress node.

tnqn · 2025-03-31T15:51:00Z

pkg/apis/crd/v1beta1/types.go

@@ -1004,9 +1004,17 @@ type EgressSpec struct {
 	// Cannot be set with ExternalIPPool.
 	ExternalIPPools []string `json:"externalIPPools,omitempty"`
 	// Bandwidth specifies the rate limit of north-south egress traffic of this Egress.
-	Bandwidth *Bandwidth `json:"bandwidth,omitempty"`
+	Bandwidth     *Bandwidth        `json:"bandwidth,omitempty"`
+	FailurePolicy FailurePolicyType `json:"failurePolicy"`


Shouldn't it be a pointer given existing egress won't have it set?

And what's the default value? Please add proper comment for this new field for better understanding.

Default would be NodeSNAT

added comments

pkg/apis/crd/v1beta1/types.go

pkg/controller/egress/validate.go

pkg/agent/controller/egress/egress_controller.go

tnqn · 2025-03-31T15:58:59Z

pkg/agent/controller/egress/egress_controller.go

-		eState.pods.Insert(pod)
-		stalePods.Delete(pod)
+	failurePolicy := egress.Spec.FailurePolicy
+	if failurePolicy == crdv1b1.FailurePolicyDrop || egressNodeAvailability(egress) {


This doesn't seem dropping the traffic but still sending it to the egress IP. This is more like Ignore instead of Drop.

yes we are kind of Ignoring the packet

Added a FailurePolicy field to the Egress spec. If this field is set to "NodeSNAT" then if the egress node is not available due to any reason then the packet transfer or we can say the traffic will go via normal Node SNAT, while in the second case where this field is set to "Drop", and if egress node is not available packet won't go out and traffic will be stuck because of unavailability of egress node. Signed-off-by: Pulkit Jain <[email protected]>

jainpulkit22 mentioned this pull request Mar 3, 2025

Allow Node SNAT for Static Egress case #6831

Closed

jainpulkit22 force-pushed the configurable-snat-for-egress branch from 83f9094 to e25f867 Compare March 3, 2025 09:44

jainpulkit22 requested review from antoninbas and tnqn March 7, 2025 10:42

jianjuns self-requested a review March 11, 2025 05:28

jainpulkit22 force-pushed the configurable-snat-for-egress branch from e25f867 to a4d3382 Compare March 20, 2025 06:48

jainpulkit22 requested review from jianjuns and removed request for jianjuns March 20, 2025 07:01

tnqn reviewed Mar 31, 2025

View reviewed changes

jainpulkit22 force-pushed the configurable-snat-for-egress branch from a4d3382 to 366789e Compare April 2, 2025 10:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add more configuration flexibility to Egress for user #7041

Add more configuration flexibility to Egress for user #7041

jainpulkit22 commented Mar 3, 2025 •

edited

Loading

antrea-bot commented Mar 3, 2025

jainpulkit22 commented Mar 4, 2025 •

edited

Loading

jainpulkit22 commented Mar 4, 2025 •

edited

Loading

jainpulkit22 commented Mar 20, 2025

tnqn Mar 31, 2025

jainpulkit22 Apr 2, 2025

jainpulkit22 Apr 2, 2025 •

edited

Loading

tnqn Mar 31, 2025

jainpulkit22 Apr 2, 2025

Add more configuration flexibility to Egress for user #7041

Are you sure you want to change the base?

Add more configuration flexibility to Egress for user #7041

Conversation

jainpulkit22 commented Mar 3, 2025 • edited Loading

antrea-bot commented Mar 3, 2025

jainpulkit22 commented Mar 4, 2025 • edited Loading

jainpulkit22 commented Mar 4, 2025 • edited Loading

jainpulkit22 commented Mar 20, 2025

tnqn Mar 31, 2025

Choose a reason for hiding this comment

jainpulkit22 Apr 2, 2025

Choose a reason for hiding this comment

jainpulkit22 Apr 2, 2025 • edited Loading

Choose a reason for hiding this comment

tnqn Mar 31, 2025

Choose a reason for hiding this comment

jainpulkit22 Apr 2, 2025

Choose a reason for hiding this comment

jainpulkit22 commented Mar 3, 2025 •

edited

Loading

jainpulkit22 commented Mar 4, 2025 •

edited

Loading

jainpulkit22 commented Mar 4, 2025 •

edited

Loading

jainpulkit22 Apr 2, 2025 •

edited

Loading