Skip to content

Commit

Permalink
feat(NSC): honor service-proxy-name label
Browse files Browse the repository at this point in the history
Abide the service.kubernetes.io/service-proxy-name label as defined by
the upstream standard here:
https://github.com/kubernetes-sigs/kpng/blob/master/doc/service-proxy.md#ignored-servicesendpoints

Resolves the failing e2e test:
should implement service.kubernetes.io/service-proxy-name

Fixes: #979
  • Loading branch information
aauren committed Jan 5, 2024
1 parent ced5102 commit a0fe844
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 0 deletions.
12 changes: 12 additions & 0 deletions pkg/controllers/proxy/network_services_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ const (
customDSRRouteTableName = "kube-router-dsr"
externalIPRouteTableID = "79"
externalIPRouteTableName = "external_ip"
kubeRouterProxyName = "kube-router"

// Taken from https://github.com/torvalds/linux/blob/master/include/uapi/linux/ip_vs.h#L21
ipvsPersistentFlagHex = 0x0001
Expand All @@ -64,6 +65,7 @@ const (
arpAnnounceUseBestLocalAddress = 2
arpIgnoreReplyOnlyIfTargetIPIsLocal = 1

// kube-router custom labels / annotations
svcDSRAnnotation = "kube-router.io/service.dsr"
svcSchedulerAnnotation = "kube-router.io/service.scheduler"
svcHairpinAnnotation = "kube-router.io/service.hairpin"
Expand All @@ -72,6 +74,9 @@ const (
svcSkipLbIpsAnnotation = "kube-router.io/service.skiplbips"
svcSchedFlagsAnnotation = "kube-router.io/service.schedflags"

// kubernetes standard labels / annotations
svcProxyNameLabel = "service.kubernetes.io/service-proxy-name"

// All IPSET names need to be less than 31 characters in order for the Kernel to accept them. Keep in mind that the
// actual formulation for this may be inet6:<setNameBase> depending on ip family, plus when we change ipsets we use
// a swap operation that adds a hyphen to the end, so that means that these base names actually need to be less than
Expand Down Expand Up @@ -892,6 +897,13 @@ func (nsc *NetworkServicesController) buildServicesInfo() serviceInfoMap {
continue
}

proxyName, err := getLabelFromMap(svcProxyNameLabel, svc.Labels)
if err == nil && proxyName != kubeRouterProxyName {
klog.V(2).Infof("Skipping service name:%s namespace:%s due to service-proxy-name label not being one "+
"that belongs to kube-router", svc.Name, svc.Namespace)
continue
}

for _, port := range svc.Spec.Ports {
svcInfo := serviceInfo{
clusterIP: net.ParseIP(svc.Spec.ClusterIP),
Expand Down
12 changes: 12 additions & 0 deletions pkg/controllers/proxy/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -530,3 +530,15 @@ func runIPCommandsWithArgs(ipArgs []string, additionalArgs ...string) *exec.Cmd
allArgs = append(allArgs, additionalArgs...)
return exec.Command("ip", allArgs...)
}

// getLabelFromMap checks the list of passed labels for the service.kubernetes.io/service-proxy-name
// label and if it exists, returns it otherwise returns an error
func getLabelFromMap(label string, labels map[string]string) (string, error) {
for lbl, val := range labels {
if lbl == label {
return val, nil
}
}

return "", fmt.Errorf("label doesn't exist in map")
}
34 changes: 34 additions & 0 deletions pkg/controllers/proxy/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,3 +131,37 @@ func TestNetworkServicesController_lookupServiceByFWMark(t *testing.T) {
assert.Zero(t, foundPort, "port should be zero on error")
})
}

func TestNetworkServicesController_getLabelFromMap(t *testing.T) {
labels := map[string]string{
"service.kubernetes.io": "foo",
"kube-router.io": "bar",
}
copyLabels := func(srcLbls map[string]string) map[string]string {
dstLbls := map[string]string{}
for k, v := range srcLbls {
dstLbls[k] = v
}
return dstLbls
}
t.Run("return blank when passed labels don't contain service-proxy-name label", func(t *testing.T) {
lbl, err := getLabelFromMap(svcProxyNameLabel, labels)
assert.Empty(t, lbl, "should return blank for a list of labels that don't contain a service-proxy-name label")
assert.Error(t, err, "should return an error when the label doesn't exist")
})

t.Run("return blank when empty label map is passed", func(t *testing.T) {
lbls := map[string]string{}
lbl, err := getLabelFromMap(svcProxyNameLabel, lbls)
assert.Empty(t, lbl, "should return blank for a map with no elements")
assert.Error(t, err, "should return an error when the map doesn't contain any elements")
})

t.Run("return value when an labels contains service-proxy-name label", func(t *testing.T) {
lbls := copyLabels(labels)
lbls[svcProxyNameLabel] = "foo"
lbl, err := getLabelFromMap(svcProxyNameLabel, lbls)
assert.Equal(t, "foo", lbl, "should return value when service-proxy-name passed")
assert.Nil(t, err, "error should be nil when the label exists")
})
}

0 comments on commit a0fe844

Please sign in to comment.