Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge pull request #1084 from tssurya/bug-2083116
[release-4.10] Bug 2083116: delete SNAT2NIP if pod.node == egressNodeServingPod
  • Loading branch information
openshift-merge-robot committed May 12, 2022
2 parents fc0ebe3 + 4a3554b commit 18bd7e1
Show file tree
Hide file tree
Showing 4 changed files with 895 additions and 18 deletions.
1 change: 0 additions & 1 deletion go-controller/pkg/kube/kube.go
Expand Up @@ -241,7 +241,6 @@ func (k *Kube) UpdateEgressIP(eIP *egressipv1.EgressIP) error {
}

func (k *Kube) PatchEgressIP(name string, patchData []byte) error {
klog.Infof("Patching status on EgressIP %s", name)
_, err := k.EIPClient.K8sV1().EgressIPs().Patch(context.TODO(), name, types.JSONPatchType, patchData, metav1.PatchOptions{})
return err
}
Expand Down
30 changes: 15 additions & 15 deletions go-controller/pkg/ovn/egressip.go
Expand Up @@ -865,9 +865,6 @@ func (oc *Controller) addPodEgressIPAssignments(name string, statusAssignments [
podIPs: logicalPort.ips,
}
oc.eIPC.podAssignment[podKey] = podState
if err := oc.eIPC.deletePerPodGRSNAT(pod, logicalPort.ips); err != nil {
return err
}
} else {
for _, status := range statusAssignments {
if _, exists := podState.egressStatuses[status]; !exists {
Expand Down Expand Up @@ -912,13 +909,11 @@ func (oc *Controller) deleteEgressIPAssignments(name string, statusesToRemove []
}
for podKey, podStatus := range oc.eIPC.podAssignment {
delete(podStatus.egressStatuses, statusToRemove)
if len(podStatus.egressStatuses) == 0 {
podNamespace, podName := getPodNamespaceAndNameFromKey(podKey)
if err := oc.eIPC.addPerPodGRSNAT(podNamespace, podName, podStatus.podIPs); err != nil {
return err
}
delete(oc.eIPC.podAssignment, podKey)
podNamespace, podName := getPodNamespaceAndNameFromKey(podKey)
if err := oc.eIPC.addPerPodGRSNAT(podNamespace, podName, podStatus.podIPs); err != nil {
return err
}
delete(oc.eIPC.podAssignment, podKey)
}
}
return nil
Expand Down Expand Up @@ -962,9 +957,6 @@ func (oc *Controller) deletePodEgressIPAssignments(name string, statusesToRemove
}
delete(podStatus.egressStatuses, statusToRemove)
}
if len(podStatus.egressStatuses) > 0 {
return nil
}
if err := oc.eIPC.addPerPodGRSNAT(pod.Namespace, pod.Name, podStatus.podIPs); err != nil {
return err
}
Expand Down Expand Up @@ -1243,6 +1235,7 @@ type EgressIPPatchStatus struct {
// object update which risks resetting the EgressIP object's fields to the state
// they had when we started processing the change.
func (oc *Controller) patchReplaceEgressIPStatus(name string, statusItems []egressipv1.EgressIPStatusItem) error {
klog.Infof("Patching status on EgressIP %s: %v", name, statusItems)
return retry.RetryOnConflict(retry.DefaultRetry, func() error {
t := []EgressIPPatchStatus{
{
Expand Down Expand Up @@ -1680,6 +1673,9 @@ type egressIPController struct {
// (routing pod traffic to the egress node) and NAT objects on the egress node
// (SNAT-ing to the egress IP).
func (e *egressIPController) addPodEgressIPAssignment(egressIPName string, status egressipv1.EgressIPStatusItem, pod *kapi.Pod, podIPs []*net.IPNet) (err error) {
if err := e.deletePerPodGRSNAT(pod, podIPs, status); err != nil {
return err
}
if err := e.handleEgressReroutePolicy(podIPs, status, egressIPName, e.createEgressReroutePolicy); err != nil {
return fmt.Errorf("unable to create logical router policy, err: %v", err)
}
Expand Down Expand Up @@ -1733,9 +1729,10 @@ func (e *egressIPController) addPerPodGRSNAT(podNamespace, podName string, podIP
return nil
}

func (e *egressIPController) deletePerPodGRSNAT(pod *kapi.Pod, podIPs []*net.IPNet) error {
if config.Gateway.DisableSNATMultipleGWs {
// remove snats to->nodeIP (from the node where pod exists) for these podIPs before adding the snat to->egressIP
func (e *egressIPController) deletePerPodGRSNAT(pod *kapi.Pod, podIPs []*net.IPNet, status egressipv1.EgressIPStatusItem) error {
if config.Gateway.DisableSNATMultipleGWs && status.Node == pod.Spec.NodeName {
// remove snats to->nodeIP (from the node where pod exists if that node is also serving
// as an egress node for this pod) for these podIPs before adding the snat to->egressIP
extIPs, err := getExternalIPsGRSNAT(e.watchFactory, pod.Spec.NodeName)
if err != nil {
return err
Expand All @@ -1744,6 +1741,9 @@ func (e *egressIPController) deletePerPodGRSNAT(pod *kapi.Pod, podIPs []*net.IPN
if err != nil {
return err
}
} else if config.Gateway.DisableSNATMultipleGWs {
// it means the node on which the pod is is different from the egressNode that is managing the pod
klog.V(5).Infof("Not deleting SNAT on %s since egress node managing %s/%s is %s", pod.Spec.NodeName, pod.Namespace, pod.Name, status.Node)
}
return nil
}
Expand Down

0 comments on commit 18bd7e1

Please sign in to comment.