Skip to content

Add more configuration flexibility to Egress for user #7041

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add more configuration flexibility to Egress for user #7041

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions build/charts/antrea/crds/egress.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ spec:
type: object
required:
- appliedTo
- failurePolicy
oneOf:
- anyOf:
- required:
Expand Down Expand Up @@ -118,6 +119,9 @@ spec:
type: string
burst:
type: string
failurePolicy:
type: string
enum: ['Drop', 'NodeSNAT']
status:
type: object
properties:
Expand Down
4 changes: 4 additions & 0 deletions build/yamls/antrea-aks.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1414,6 +1414,7 @@ spec:
type: object
required:
- appliedTo
- failurePolicy
oneOf:
- anyOf:
- required:
Expand Down Expand Up @@ -1512,6 +1513,9 @@ spec:
type: string
burst:
type: string
failurePolicy:
type: string
enum: ['Drop', 'NodeSNAT']
status:
type: object
properties:
Expand Down
4 changes: 4 additions & 0 deletions build/yamls/antrea-crds.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1403,6 +1403,7 @@ spec:
type: object
required:
- appliedTo
- failurePolicy
oneOf:
- anyOf:
- required:
Expand Down Expand Up @@ -1501,6 +1502,9 @@ spec:
type: string
burst:
type: string
failurePolicy:
type: string
enum: ['Drop', 'NodeSNAT']
status:
type: object
properties:
Expand Down
4 changes: 4 additions & 0 deletions build/yamls/antrea-eks.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1414,6 +1414,7 @@ spec:
type: object
required:
- appliedTo
- failurePolicy
oneOf:
- anyOf:
- required:
Expand Down Expand Up @@ -1512,6 +1513,9 @@ spec:
type: string
burst:
type: string
failurePolicy:
type: string
enum: ['Drop', 'NodeSNAT']
status:
type: object
properties:
Expand Down
4 changes: 4 additions & 0 deletions build/yamls/antrea-gke.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1414,6 +1414,7 @@ spec:
type: object
required:
- appliedTo
- failurePolicy
oneOf:
- anyOf:
- required:
Expand Down Expand Up @@ -1512,6 +1513,9 @@ spec:
type: string
burst:
type: string
failurePolicy:
type: string
enum: ['Drop', 'NodeSNAT']
status:
type: object
properties:
Expand Down
4 changes: 4 additions & 0 deletions build/yamls/antrea-ipsec.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1414,6 +1414,7 @@ spec:
type: object
required:
- appliedTo
- failurePolicy
oneOf:
- anyOf:
- required:
Expand Down Expand Up @@ -1512,6 +1513,9 @@ spec:
type: string
burst:
type: string
failurePolicy:
type: string
enum: ['Drop', 'NodeSNAT']
status:
type: object
properties:
Expand Down
4 changes: 4 additions & 0 deletions build/yamls/antrea.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1414,6 +1414,7 @@ spec:
type: object
required:
- appliedTo
- failurePolicy
oneOf:
- anyOf:
- required:
Expand Down Expand Up @@ -1512,6 +1513,9 @@ spec:
type: string
burst:
type: string
failurePolicy:
type: string
enum: ['Drop', 'NodeSNAT']
status:
type: object
properties:
Expand Down
69 changes: 43 additions & 26 deletions pkg/agent/controller/egress/egress_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ var emptyWatch = watch.NewEmptyWatch()

var newIPAssigner = ipassigner.NewIPAssigner

var egressNodeAvailability = hasEgressNode

// egressState keeps the actual state of an Egress that has been realized.
type egressState struct {
// The actual egress IP of the Egress. If it's different from the desired IP, there is an update to EgressIP, and we
Expand Down Expand Up @@ -989,6 +991,13 @@ func (c *EgressController) updateEgressStatus(egress *crdv1b1.Egress, egressIP s
return nil
}

func hasEgressNode(egress *crdv1b1.Egress) bool {
if egress.Status.EgressNode == "" {
return false
}
return true
}

func (c *EgressController) syncEgress(egressName string) error {
startTime := time.Now()
defer func() {
Expand Down Expand Up @@ -1024,6 +1033,7 @@ func (c *EgressController) syncEgress(egressName string) error {
desiredNode = egressNode
} else {
scheduleErr = err
desiredEgressIP = egress.Spec.EgressIP
}
} else {
desiredEgressIP = egress.Spec.EgressIP
Expand Down Expand Up @@ -1118,39 +1128,46 @@ func (c *EgressController) syncEgress(egressName string) error {
}()

egressIP := net.ParseIP(eState.egressIP)
// Install SNAT flows for desired Pods.
for pod := range pods {
eState.pods.Insert(pod)
stalePods.Delete(pod)
failurePolicy := egress.Spec.FailurePolicy
if failurePolicy == crdv1b1.FailurePolicyDrop || egressNodeAvailability(egress) {
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes we are kind of Ignoring the packet

// Install SNAT flows for desired Pods.
for pod := range pods {
eState.pods.Insert(pod)
stalePods.Delete(pod)

// If the Egress is not the effective one for the Pod, do nothing.
if !c.bindPodEgress(pod, egressName) {
continue
}

// If the Egress is not the effective one for the Pod, do nothing.
if !c.bindPodEgress(pod, egressName) {
continue
}
// Get the Pod's openflow port.
parts := strings.Split(pod, "/")
podNamespace, podName := parts[0], parts[1]
ifaces := c.ifaceStore.GetContainerInterfacesByPod(podName, podNamespace)
if len(ifaces) == 0 {
klog.Infof("Interfaces of Pod %s/%s not found", podNamespace, podName)
continue
}

// Get the Pod's openflow port.
parts := strings.Split(pod, "/")
podNamespace, podName := parts[0], parts[1]
ifaces := c.ifaceStore.GetContainerInterfacesByPod(podName, podNamespace)
if len(ifaces) == 0 {
klog.Infof("Interfaces of Pod %s/%s not found", podNamespace, podName)
continue
ofPort := ifaces[0].OFPort
if eState.ofPorts.Has(ofPort) {
staleOFPorts.Delete(ofPort)
continue
}
if err := c.ofClient.InstallPodSNATFlows(uint32(ofPort), egressIP, mark); err != nil {
return err
}
eState.ofPorts.Insert(ofPort)
}

ofPort := ifaces[0].OFPort
if eState.ofPorts.Has(ofPort) {
staleOFPorts.Delete(ofPort)
continue
// Uninstall SNAT flows for stale Pods.
if err := c.uninstallPodFlows(egressName, eState, staleOFPorts, stalePods); err != nil {
return err
}
if err := c.ofClient.InstallPodSNATFlows(uint32(ofPort), egressIP, mark); err != nil {
} else if failurePolicy == crdv1b1.FailurePolicyNodeSNAT {
if err := c.uninstallPodFlows(egressName, eState, eState.ofPorts, eState.pods); err != nil {
return err
}
eState.ofPorts.Insert(ofPort)
}

// Uninstall SNAT flows for stale Pods.
if err := c.uninstallPodFlows(egressName, eState, staleOFPorts, stalePods); err != nil {
return err
}
return nil
}
Expand Down
27 changes: 27 additions & 0 deletions pkg/agent/controller/egress/egress_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1101,6 +1101,15 @@ func TestSyncEgress(t *testing.T) {
},
},
}

egressNodeAvailability = func(egress *crdv1b1.Egress) bool {
return true
}

defer func() {
egressNodeAvailability = hasEgressNode
}()

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
initObjects := []runtime.Object{tt.existingEgress}
Expand Down Expand Up @@ -1195,6 +1204,15 @@ func TestPodUpdateShouldSyncEgress(t *testing.T) {
{Pod: &cpv1b2.PodReference{Name: "pendingPod", Namespace: "ns1"}},
},
}

egressNodeAvailability = func(egress *crdv1b1.Egress) bool {
return true
}

defer func() {
egressNodeAvailability = hasEgressNode
}()

c := newFakeController(t, []runtime.Object{egress})
stopCh := make(chan struct{})
defer close(stopCh)
Expand Down Expand Up @@ -1327,6 +1345,15 @@ func TestSyncOverlappingEgress(t *testing.T) {
{Pod: &cpv1b2.PodReference{Name: "pod4", Namespace: "ns4"}},
},
}

egressNodeAvailability = func(egress *crdv1b1.Egress) bool {
return true
}

defer func() {
egressNodeAvailability = hasEgressNode
}()

c := newFakeController(t, []runtime.Object{egress1, egress2, egress3})
stopCh := make(chan struct{})
defer close(stopCh)
Expand Down
14 changes: 14 additions & 0 deletions pkg/apis/crd/v1beta1/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1005,8 +1005,22 @@ type EgressSpec struct {
ExternalIPPools []string `json:"externalIPPools,omitempty"`
// Bandwidth specifies the rate limit of north-south egress traffic of this Egress.
Bandwidth *Bandwidth `json:"bandwidth,omitempty"`
// FailurePolicy specifies the behaviour of Egress in cases when Egress Node is not available,
// it supports two kind of behaviours "Drop" and "NodeSNAT", it is a required field.
FailurePolicy FailurePolicyType `json:"failurePolicy"`
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Default would be NodeSNAT

Copy link
Contributor Author

@jainpulkit22 jainpulkit22 Apr 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

added comments

}

type FailurePolicyType string

const (
// FailurePolicyDrop defines the behaviour of Egress in situations when Egress Node is not available.
// Here, Drop signifies that if Egress Node is not available then the packet should be dropped.
FailurePolicyDrop FailurePolicyType = "Drop"
// FailurePolicyNodeSNAT defines the behaviour of Egress in situations when Egress Node is not available.
// Here, NodeSNAT signifies that if Egress Node is not available then the packet should be sent via local NodeSNAT.
FailurePolicyNodeSNAT FailurePolicyType = "NodeSNAT"
)

type Bandwidth struct {
// Rate specifies the maximum traffic rate. e.g. 300k, 10M
Rate string `json:"rate"`
Expand Down
11 changes: 9 additions & 2 deletions pkg/apiserver/openapi/zz_generated.openapi.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading