Skip to content

Commit

Permalink
fix(hairpin): rely on CNI hairpin mode
Browse files Browse the repository at this point in the history
Instead of using the hairping controller that I created, instead rely
upon the `hairpinMode` option of the bridge CNI that does this job
better. See https://www.kube-router.io/docs/user-guide/#hairpin-mode for
more information.
  • Loading branch information
aauren committed Apr 27, 2024
1 parent 9d9b796 commit 317c754
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 10 deletions.
6 changes: 6 additions & 0 deletions pkg/controllers/proxy/hairpin_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@ import (
"k8s.io/klog/v2"
)

// !!!! IMPORTANT !!!! - This code is not currently used
// Not creating the hairpin controller for now because this should be handled at the CNI level. The CNI bridge
// plugin ensures that hairpin mode is set much more reliably than we do. However, as a lot of work was put into
// the hairpin controller, and so that it is around to reference in the future if needed, I'm leaving the code
// for now.

type hairpinController struct {
epC <-chan string
nsc *NetworkServicesController
Expand Down
18 changes: 13 additions & 5 deletions pkg/controllers/proxy/network_services_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,8 +247,10 @@ func (nsc *NetworkServicesController) Run(healthChan chan<- *healthcheck.Control
}

// Run the hairpin controller
wg.Add(1)
go nsc.hpc.Run(stopCh, wg, healthChan)
if nsc.hpc != nil {
wg.Add(1)
go nsc.hpc.Run(stopCh, wg, healthChan)
}

// enable masquerade rule
err = nsc.ensureMasqueradeIptablesRule()
Expand Down Expand Up @@ -1324,7 +1326,9 @@ func (nsc *NetworkServicesController) syncHairpinIptablesRules() error {
//
// Without this change, the return traffic from a client to a service within the same pod will never
// make it back into the pod's namespace
nsc.hpEndpointReceiver <- ep.ip
if nsc.hpc != nil {
nsc.hpEndpointReceiver <- ep.ip
}

// Handle ClusterIP Service
hairpinRuleFrom(familyClusterIPs, ep.ip, family, svcInfo.port, rulesMap)
Expand Down Expand Up @@ -2114,8 +2118,12 @@ func NewNetworkServicesController(clientset kubernetes.Interface,
nsc.epSliceLister = epSliceInformer.GetIndexer()
nsc.EndpointSliceEventHandler = nsc.newEndpointSliceEventHandler()

nsc.hpEndpointReceiver = make(chan string)
nsc.hpc = NewHairpinController(&nsc, nsc.hpEndpointReceiver)
// Not creating the hairpin controller for now because this should be handled at the CNI level. The CNI bridge
// plugin ensures that hairpin mode is set much more reliably than we do. However, as a lot of work was put into
// the hairpin controller, and so that it is around to reference in the future if needed, I'm leaving the code
// for now.
// nsc.hpEndpointReceiver = make(chan string)
// nsc.hpc = NewHairpinController(&nsc, nsc.hpEndpointReceiver)

nsc.nphc = NewNodePortHealthCheck()

Expand Down
10 changes: 5 additions & 5 deletions pkg/healthcheck/health_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,11 +166,11 @@ func (hc *HealthController) CheckHealth() bool {
klog.Error("NetworkService Controller heartbeat missed")
health = false
}
if time.Since(hc.Status.HairpinControllerAlive) >
HPCSyncPeriod+hc.Status.HairpinControllerAliveTTL+graceTime {
klog.Error("Hairpin Controller heartbeat missed")
health = false
}
// if time.Since(hc.Status.HairpinControllerAlive) >
// HPCSyncPeriod+hc.Status.HairpinControllerAliveTTL+graceTime {
// klog.Error("Hairpin Controller heartbeat missed")
// health = false
// }
}

if hc.Config.MetricsEnabled {
Expand Down

0 comments on commit 317c754

Please sign in to comment.