Skip to content
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

Jump to bottom

Dual-stack: fix the bug that Service clusterIP does not respect specified ipFamily #89612

Merged
merged 1 commit into from Apr 3, 2020
Merged

Dual-stack: fix the bug that Service clusterIP does not respect specified ipFamily #89612

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions pkg/registry/core/service/storage/rest.go
View file
Open in desktop
Expand Up @@ -559,7 +559,7 @@ func (r *REST) getAllocatorByClusterIP(service *api.Service) ipallocator.Interfa
}

secondaryAllocatorCIDR := r.secondaryServiceIPs.CIDR()
if netutil.IsIPv6String(service.Spec.ClusterIP) && netutil.IsIPv6CIDR(&secondaryAllocatorCIDR) {
if netutil.IsIPv6String(service.Spec.ClusterIP) == netutil.IsIPv6CIDR(&secondaryAllocatorCIDR) {
return r.secondaryServiceIPs
}

Expand All @@ -574,7 +574,7 @@ func (r *REST) getAllocatorBySpec(service *api.Service) ipallocator.Interface {
}

secondaryAllocatorCIDR := r.secondaryServiceIPs.CIDR()
if *(service.Spec.IPFamily) == api.IPv6Protocol && netutil.IsIPv6CIDR(&secondaryAllocatorCIDR) {
if (*(service.Spec.IPFamily) == api.IPv6Protocol) == netutil.IsIPv6CIDR(&secondaryAllocatorCIDR) {
return r.secondaryServiceIPs
}

Expand Down
124 changes: 119 additions & 5 deletions pkg/registry/core/service/storage/rest_test.go
View file
Open in desktop
Expand Up @@ -2512,15 +2512,17 @@ func TestAllocGetters(t *testing.T) {
testCases := []struct {
name string

isIPv6Primary bool
enableDualStack bool
specExpctPrimary bool
clusterIPExpectPrimary bool

svc *api.Service
}{
{
name: "spec:v4 ip:v4 dualstack:off",
name: "spec:v4 ip:v4 dualstack:off isIPv6Primary:false",

isIPv6Primary: false,
specExpctPrimary: true,
clusterIPExpectPrimary: true,
enableDualStack: false,
Expand All @@ -2536,8 +2538,9 @@ func TestAllocGetters(t *testing.T) {
},
},
{
name: "spec:v4 ip:v4 dualstack:on",
name: "spec:v4 ip:v4 dualstack:on isIPv6Primary:false",

isIPv6Primary: false,
specExpctPrimary: true,
clusterIPExpectPrimary: true,
enableDualStack: true,
Expand All @@ -2554,8 +2557,9 @@ func TestAllocGetters(t *testing.T) {
},

{
name: "spec:v4 ip:v6 dualstack:on",
name: "spec:v4 ip:v6 dualstack:on isIPv6Primary:false",

isIPv6Primary: false,
specExpctPrimary: true,
clusterIPExpectPrimary: false,
enableDualStack: true,
Expand All @@ -2572,8 +2576,9 @@ func TestAllocGetters(t *testing.T) {
},

{
name: "spec:v6 ip:v6 dualstack:on",
name: "spec:v6 ip:v6 dualstack:on isIPv6Primary:false",

isIPv6Primary: false,
specExpctPrimary: false,
clusterIPExpectPrimary: false,
enableDualStack: true,
Expand All @@ -2590,8 +2595,9 @@ func TestAllocGetters(t *testing.T) {
},

{
name: "spec:v6 ip:v4 dualstack:on",
name: "spec:v6 ip:v4 dualstack:on isIPv6Primary:false",

isIPv6Primary: false,
specExpctPrimary: false,
clusterIPExpectPrimary: true,
enableDualStack: true,
Expand All @@ -2606,6 +2612,99 @@ func TestAllocGetters(t *testing.T) {
},
},
},
{
name: "spec:v6 ip:v6 dualstack:off isIPv6Primary:true",

isIPv6Primary: true,
specExpctPrimary: true,
clusterIPExpectPrimary: true,
enableDualStack: false,

svc: &api.Service{
ObjectMeta: metav1.ObjectMeta{Name: "foo", ResourceVersion: "1"},
Spec: api.ServiceSpec{
Selector: map[string]string{"bar": "baz"},
Type: api.ServiceTypeClusterIP,
IPFamily: &ipv6Service,
ClusterIP: "2000::1",
},
},
},
{
name: "spec:v6 ip:v6 dualstack:on isIPv6Primary:true",

isIPv6Primary: true,
specExpctPrimary: true,
clusterIPExpectPrimary: true,
enableDualStack: true,

svc: &api.Service{
ObjectMeta: metav1.ObjectMeta{Name: "foo", ResourceVersion: "1"},
Spec: api.ServiceSpec{
Selector: map[string]string{"bar": "baz"},
Type: api.ServiceTypeClusterIP,
IPFamily: &ipv6Service,
ClusterIP: "2000::1",
},
},
},

{
name: "spec:v6 ip:v4 dualstack:on isIPv6Primary:true",

isIPv6Primary: true,
specExpctPrimary: true,
clusterIPExpectPrimary: false,
enableDualStack: true,

svc: &api.Service{
ObjectMeta: metav1.ObjectMeta{Name: "foo", ResourceVersion: "1"},
Spec: api.ServiceSpec{
Selector: map[string]string{"bar": "baz"},
Type: api.ServiceTypeClusterIP,
IPFamily: &ipv6Service,
ClusterIP: "10.0.0.1",
},
},
},

{
name: "spec:v4 ip:v4 dualstack:on isIPv6Primary:true",

isIPv6Primary: true,
specExpctPrimary: false,
clusterIPExpectPrimary: false,
enableDualStack: true,

svc: &api.Service{
ObjectMeta: metav1.ObjectMeta{Name: "foo", ResourceVersion: "1"},
Spec: api.ServiceSpec{
Selector: map[string]string{"bar": "baz"},
Type: api.ServiceTypeClusterIP,
IPFamily: &ipv4Service,
ClusterIP: "10.0.0.1",
},
},
},

{
name: "spec:v4 ip:v6 dualstack:on isIPv6Primary:true",

isIPv6Primary: true,
specExpctPrimary: false,
clusterIPExpectPrimary: true,
enableDualStack: true,

svc: &api.Service{
ObjectMeta: metav1.ObjectMeta{Name: "foo", ResourceVersion: "1"},
Spec: api.ServiceSpec{
Selector: map[string]string{"bar": "baz"},
Type: api.ServiceTypeClusterIP,
IPFamily: &ipv4Service,
ClusterIP: "2000::1",
},
},
},
}

for _, tc := range testCases {
Expand All @@ -2614,6 +2713,21 @@ func TestAllocGetters(t *testing.T) {
storage, _, server := NewTestREST(t, nil, tc.enableDualStack)
defer server.Terminate(t)

if tc.isIPv6Primary {
if storage.secondaryServiceIPs != nil {
storage.serviceIPs, storage.secondaryServiceIPs = storage.secondaryServiceIPs, storage.serviceIPs
} else {
r, err := ipallocator.NewCIDRRange(makeIPNet6(t))
if err != nil {
t.Fatalf("cannot create CIDR Range(primary) %v", err)
}
if tc.enableDualStack {
storage.secondaryServiceIPs = storage.serviceIPs
}
storage.serviceIPs = r
}
}

if tc.enableDualStack && storage.secondaryServiceIPs == nil {
t.Errorf("storage must allocate secondary ServiceIPs allocator for dual stack")
return
Expand Down