Svc REST: clean up defaultOnRead to be consistent

Headless+selectorless -> RequireDualStack

Headless+selector -> SingleStack

Add test cases to cover this and ExternalName and dual-stack init (which
I think can never trigger, but best to be safe).
This commit is contained in:
Tim Hockin 2021-09-13 17:08:03 -07:00
parent 6a49ed41ea
commit 52f54ce90d
3 changed files with 119 additions and 70 deletions

View File

@ -117,7 +117,7 @@ func MakeServicePort(name string, port int, tgtPort intstr.IntOrString, proto ap
// SetHeadless sets the service as headless and clears other fields.
func SetHeadless(svc *api.Service) {
SetTypeClusterIP(svc)
svc.Spec.ClusterIP = api.ClusterIPNone
SetClusterIPs(api.ClusterIPNone)(svc)
}
// SetSelector sets the service selector.

View File

@ -47,9 +47,8 @@ import (
svcreg "k8s.io/kubernetes/pkg/registry/core/service"
"k8s.io/kubernetes/pkg/registry/core/service/ipallocator"
"k8s.io/kubernetes/pkg/registry/core/service/portallocator"
"sigs.k8s.io/structured-merge-diff/v4/fieldpath"
netutil "k8s.io/utils/net"
"sigs.k8s.io/structured-merge-diff/v4/fieldpath"
)
type EndpointsStorage interface {
@ -248,65 +247,59 @@ func (r *REST) defaultOnReadService(service *api.Service) {
return
}
// Set ipFamilies and ipFamilyPolicy if needed.
r.defaultOnReadIPFamilies(service)
}
func (r *REST) defaultOnReadIPFamilies(service *api.Service) {
// ExternalName does not need this.
if !needsClusterIP(service) {
return
}
// If IPFamilies is set, we assume IPFamilyPolicy is also set (it should
// not be possible to have one and not the other), and therefore we don't
// need further defaulting. Likewise, if IPFamilies is *not* set, we
// assume IPFamilyPolicy can't be set either.
if len(service.Spec.IPFamilies) > 0 {
return // already defaulted
return
}
// set clusterIPs based on ClusterIP
if len(service.Spec.ClusterIPs) == 0 {
if len(service.Spec.ClusterIP) > 0 {
service.Spec.ClusterIPs = []string{service.Spec.ClusterIP}
}
}
requireDualStack := api.IPFamilyPolicyRequireDualStack
singleStack := api.IPFamilyPolicySingleStack
preferDualStack := api.IPFamilyPolicyPreferDualStack
// headless services
if len(service.Spec.ClusterIPs) == 1 && service.Spec.ClusterIPs[0] == api.ClusterIPNone {
service.Spec.IPFamilies = []api.IPFamily{r.primaryIPFamily}
requireDualStack := api.IPFamilyPolicyRequireDualStack
// headless+selectorless
// headless+selectorless takes both families. Why?
// at this stage we don't know what kind of endpoints (specifically their IPFamilies) the
// user has assigned to this selectorless service. We assume it has dualstack and we default
// it to PreferDualStack on any cluster (single or dualstack configured).
if service.Spec.ClusterIP == api.ClusterIPNone {
// Headless.
if len(service.Spec.Selector) == 0 {
service.Spec.IPFamilyPolicy = &preferDualStack
if r.primaryIPFamily == api.IPv4Protocol {
service.Spec.IPFamilies = append(service.Spec.IPFamilies, api.IPv6Protocol)
} else {
service.Spec.IPFamilies = append(service.Spec.IPFamilies, api.IPv4Protocol)
}
// Headless + selectorless is a special-case.
//
// At this stage we don't know what kind of endpoints (specifically
// their IPFamilies) the user has assigned to this selectorless
// service. We assume it has dual-stack and we default it to
// RequireDualStack on any cluster (single- or dual-stack
// configured).
service.Spec.IPFamilyPolicy = &requireDualStack
service.Spec.IPFamilies = []api.IPFamily{r.primaryIPFamily, otherFamily(r.primaryIPFamily)}
} else {
// headless w/ selector
// this service type follows cluster configuration. this service (selector based) uses a
// selector and will have to follow how the cluster is configured. If the cluster is
// configured to dual stack then the service defaults to PreferDualStack. Otherwise we
// default it to SingleStack.
if r.secondaryIPFamily != "" {
service.Spec.IPFamilies = append(service.Spec.IPFamilies, r.secondaryIPFamily)
service.Spec.IPFamilyPolicy = &preferDualStack
} else {
service.Spec.IPFamilyPolicy = &singleStack
}
// Headless + selector - default to single.
service.Spec.IPFamilyPolicy = &singleStack
service.Spec.IPFamilies = []api.IPFamily{r.primaryIPFamily}
}
} else {
// headful
// make sure a slice exists to receive the families
service.Spec.IPFamilies = make([]api.IPFamily, len(service.Spec.ClusterIPs), len(service.Spec.ClusterIPs))
// Headful: init ipFamilies from clusterIPs.
service.Spec.IPFamilies = make([]api.IPFamily, len(service.Spec.ClusterIPs))
for idx, ip := range service.Spec.ClusterIPs {
if netutil.IsIPv6String(ip) {
service.Spec.IPFamilies[idx] = api.IPv6Protocol
} else {
service.Spec.IPFamilies[idx] = api.IPv4Protocol
}
if len(service.Spec.IPFamilies) == 1 {
service.Spec.IPFamilyPolicy = &singleStack
} else if len(service.Spec.IPFamilies) == 2 {
service.Spec.IPFamilyPolicy = &requireDualStack
}
}
if len(service.Spec.IPFamilies) == 1 {
service.Spec.IPFamilyPolicy = &singleStack
} else if len(service.Spec.IPFamilies) == 2 {
// It shouldn't be possible to get here, but just in case.
service.Spec.IPFamilyPolicy = &requireDualStack
}
}
}

View File

@ -561,29 +561,79 @@ func TestServiceDefaultOnRead(t *testing.T) {
input runtime.Object
expect runtime.Object
}{{
name: "no change v4",
input: svctest.MakeService("foo", svctest.SetClusterIPs("10.0.0.1")),
expect: svctest.MakeService("foo", svctest.SetClusterIPs("10.0.0.1")),
name: "single v4",
input: svctest.MakeService("foo", svctest.SetClusterIPs("10.0.0.1")),
expect: svctest.MakeService("foo", svctest.SetClusterIPs("10.0.0.1"),
svctest.SetIPFamilyPolicy(api.IPFamilyPolicySingleStack),
svctest.SetIPFamilies(api.IPv4Protocol)),
}, {
name: "missing clusterIPs v4",
input: svctest.MakeService("foo", svctest.SetClusterIP("10.0.0.1")),
expect: svctest.MakeService("foo", svctest.SetClusterIPs("10.0.0.1")),
name: "single v6",
input: svctest.MakeService("foo", svctest.SetClusterIPs("2000::1")),
expect: svctest.MakeService("foo", svctest.SetClusterIPs("2000::1"),
svctest.SetIPFamilyPolicy(api.IPFamilyPolicySingleStack),
svctest.SetIPFamilies(api.IPv6Protocol)),
}, {
name: "no change v6",
input: svctest.MakeService("foo", svctest.SetClusterIPs("2000::1")),
expect: svctest.MakeService("foo", svctest.SetClusterIPs("2000::1")),
name: "missing clusterIPs v4",
input: svctest.MakeService("foo", svctest.SetClusterIP("10.0.0.1")),
expect: svctest.MakeService("foo", svctest.SetClusterIPs("10.0.0.1"),
svctest.SetIPFamilyPolicy(api.IPFamilyPolicySingleStack),
svctest.SetIPFamilies(api.IPv4Protocol)),
}, {
name: "missing clusterIPs v6",
input: svctest.MakeService("foo", svctest.SetClusterIP("2000::1")),
expect: svctest.MakeService("foo", svctest.SetClusterIPs("2000::1")),
name: "missing clusterIPs v6",
input: svctest.MakeService("foo", svctest.SetClusterIP("2000::1")),
expect: svctest.MakeService("foo", svctest.SetClusterIPs("2000::1"),
svctest.SetIPFamilyPolicy(api.IPFamilyPolicySingleStack),
svctest.SetIPFamilies(api.IPv6Protocol)),
}, {
name: "list, no change v4",
input: makeServiceList(svctest.SetClusterIPs("10.0.0.1")),
expect: makeServiceList(svctest.SetClusterIPs("10.0.0.1")),
name: "list v4",
input: makeServiceList(svctest.SetClusterIPs("10.0.0.1")),
expect: makeServiceList(svctest.SetClusterIPs("10.0.0.1"),
svctest.SetIPFamilyPolicy(api.IPFamilyPolicySingleStack),
svctest.SetIPFamilies(api.IPv4Protocol)),
}, {
name: "list, missing clusterIPs v4",
input: makeServiceList(svctest.SetClusterIP("10.0.0.1")),
expect: makeServiceList(svctest.SetClusterIPs("10.0.0.1")),
name: "list missing clusterIPs v4",
input: makeServiceList(svctest.SetClusterIP("10.0.0.1")),
expect: makeServiceList(svctest.SetClusterIPs("10.0.0.1"),
svctest.SetIPFamilyPolicy(api.IPFamilyPolicySingleStack),
svctest.SetIPFamilies(api.IPv4Protocol)),
}, {
name: "external name",
input: makeServiceList(svctest.SetTypeExternalName),
expect: makeServiceList(svctest.SetTypeExternalName),
}, {
name: "dual v4v6",
input: svctest.MakeService("foo", svctest.SetClusterIPs("10.0.0.1", "2000::1")),
expect: svctest.MakeService("foo", svctest.SetClusterIPs("10.0.0.1", "2000::1"),
svctest.SetIPFamilyPolicy(api.IPFamilyPolicyRequireDualStack),
svctest.SetIPFamilies(api.IPv4Protocol, api.IPv6Protocol)),
}, {
name: "dual v6v4",
input: svctest.MakeService("foo", svctest.SetClusterIPs("2000::1", "10.0.0.1")),
expect: svctest.MakeService("foo", svctest.SetClusterIPs("2000::1", "10.0.0.1"),
svctest.SetIPFamilyPolicy(api.IPFamilyPolicyRequireDualStack),
svctest.SetIPFamilies(api.IPv6Protocol, api.IPv4Protocol)),
}, {
name: "headless",
input: svctest.MakeService("foo", svctest.SetHeadless),
expect: svctest.MakeService("foo", svctest.SetHeadless,
svctest.SetIPFamilyPolicy(api.IPFamilyPolicySingleStack),
svctest.SetIPFamilies(api.IPv4Protocol)),
}, {
name: "headless selectorless",
input: svctest.MakeService("foo", svctest.SetHeadless,
svctest.SetSelector(map[string]string{})),
expect: svctest.MakeService("foo", svctest.SetHeadless,
svctest.SetIPFamilyPolicy(api.IPFamilyPolicyRequireDualStack),
svctest.SetIPFamilies(api.IPv4Protocol, api.IPv6Protocol)),
}, {
name: "headless selectorless pre-set",
input: svctest.MakeService("foo", svctest.SetHeadless,
svctest.SetSelector(map[string]string{}),
svctest.SetIPFamilyPolicy(api.IPFamilyPolicySingleStack),
svctest.SetIPFamilies(api.IPv6Protocol)),
expect: svctest.MakeService("foo", svctest.SetHeadless,
svctest.SetIPFamilyPolicy(api.IPFamilyPolicySingleStack),
svctest.SetIPFamilies(api.IPv6Protocol)),
}, {
name: "not Service or ServiceList",
input: &api.Pod{},
@ -593,7 +643,7 @@ func TestServiceDefaultOnRead(t *testing.T) {
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
storage, _, server := newStorage(t, []api.IPFamily{api.IPv4Protocol})
storage, _, server := newStorage(t, []api.IPFamily{api.IPv4Protocol, api.IPv6Protocol})
defer server.Terminate(t)
defer storage.Store.DestroyFunc()
@ -619,11 +669,17 @@ func TestServiceDefaultOnRead(t *testing.T) {
}
// Verify fields we know are affected
if svc.Spec.ClusterIP != exp.Spec.ClusterIP {
t.Errorf("clusterIP: expected %v, got %v", exp.Spec.ClusterIP, svc.Spec.ClusterIP)
if want, got := exp.Spec.ClusterIP, svc.Spec.ClusterIP; want != got {
t.Errorf("clusterIP: expected %v, got %v", want, got)
}
if !reflect.DeepEqual(svc.Spec.ClusterIPs, exp.Spec.ClusterIPs) {
t.Errorf("clusterIPs: expected %v, got %v", exp.Spec.ClusterIPs, svc.Spec.ClusterIPs)
if want, got := exp.Spec.ClusterIPs, svc.Spec.ClusterIPs; !reflect.DeepEqual(want, got) {
t.Errorf("clusterIPs: expected %v, got %v", want, got)
}
if want, got := fmtIPFamilyPolicy(exp.Spec.IPFamilyPolicy), fmtIPFamilyPolicy(svc.Spec.IPFamilyPolicy); want != got {
t.Errorf("ipFamilyPolicy: expected %v, got %v", want, got)
}
if want, got := exp.Spec.IPFamilies, svc.Spec.IPFamilies; !reflect.DeepEqual(want, got) {
t.Errorf("ipFamilies: expected %v, got %v", want, got)
}
})
}