Merge pull request #96636 from Nordix/disable-nodeport-2

service.spec.AllocateLoadBalancerNodePorts followup
This commit is contained in:
Kubernetes Prow Robot 2020-11-24 06:59:01 -08:00 committed by GitHub
commit c652ffbe4a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 73 additions and 4 deletions

View File

@ -49,6 +49,7 @@ go_test(
"//staging/src/k8s.io/apiserver/pkg/registry/rest:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",
"//staging/src/k8s.io/component-base/featuregate/testing:go_default_library",
"//vendor/k8s.io/utils/pointer:go_default_library",
],
)

View File

@ -184,7 +184,9 @@ func dropServiceDisabledFields(newSvc *api.Service, oldSvc *api.Service) {
// Clear AllocateLoadBalancerNodePorts if ServiceLBNodePortControl if not enabled
if !utilfeature.DefaultFeatureGate.Enabled(features.ServiceLBNodePortControl) {
newSvc.Spec.AllocateLoadBalancerNodePorts = nil
if !allocateLoadBalancerNodePortsInUse(oldSvc) {
newSvc.Spec.AllocateLoadBalancerNodePorts = nil
}
}
if !utilfeature.DefaultFeatureGate.Enabled(features.MixedProtocolLBService) {
@ -199,6 +201,14 @@ func dropServiceDisabledFields(newSvc *api.Service, oldSvc *api.Service) {
}
}
// returns true if svc.Spec.AllocateLoadBalancerNodePorts field is in use
func allocateLoadBalancerNodePortsInUse(svc *api.Service) bool {
if svc == nil {
return false
}
return svc.Spec.AllocateLoadBalancerNodePorts != nil
}
// returns true if svc.Spec.ServiceIPFamily field is in use
func serviceDualStackFieldsInUse(svc *api.Service) bool {
if svc == nil {
@ -396,9 +406,14 @@ func dropTypeDependentFields(newSvc *api.Service, oldSvc *api.Service) {
newSvc.Spec.HealthCheckNodePort = 0
}
// AllocateLoadBalancerNodePorts may only be set for type LoadBalancer
if newSvc.Spec.Type != api.ServiceTypeLoadBalancer {
newSvc.Spec.AllocateLoadBalancerNodePorts = nil
// If a user is switching to a type that doesn't need allocatedLoadBalancerNodePorts AND they did not change
// this field, it is safe to drop it.
if oldSvc.Spec.Type == api.ServiceTypeLoadBalancer && newSvc.Spec.Type != api.ServiceTypeLoadBalancer {
if newSvc.Spec.AllocateLoadBalancerNodePorts != nil && oldSvc.Spec.AllocateLoadBalancerNodePorts != nil {
if *oldSvc.Spec.AllocateLoadBalancerNodePorts == *newSvc.Spec.AllocateLoadBalancerNodePorts {
newSvc.Spec.AllocateLoadBalancerNodePorts = nil
}
}
}
// NOTE: there are other fields like `selector` which we could wipe.

View File

@ -34,6 +34,7 @@ import (
utilfeature "k8s.io/apiserver/pkg/util/feature"
featuregatetesting "k8s.io/component-base/featuregate/testing"
"k8s.io/kubernetes/pkg/features"
utilpointer "k8s.io/utils/pointer"
)
func newStrategy(cidr string, hasSecondary bool) (testStrategy Strategy, testStatusStrategy Strategy) {
@ -935,6 +936,15 @@ func TestDropTypeDependentFields(t *testing.T) {
}
}
}
setAllocateLoadBalancerNodePortsTrue := func(svc *api.Service) {
svc.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true)
}
setAllocateLoadBalancerNodePortsFalse := func(svc *api.Service) {
svc.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(false)
}
clearAllocateLoadBalancerNodePorts := func(svc *api.Service) {
svc.Spec.AllocateLoadBalancerNodePorts = nil
}
testCases := []struct {
name string
@ -1022,6 +1032,46 @@ func TestDropTypeDependentFields(t *testing.T) {
svc: makeValidServiceCustom(setTypeLoadBalancer, setHCNodePort),
patch: patches(setTypeClusterIP, changeHCNodePort),
expect: makeValidServiceCustom(setHCNodePort, changeHCNodePort),
}, { // allocatedLoadBalancerNodePorts cases
name: "clear allocatedLoadBalancerNodePorts true -> true",
svc: makeValidServiceCustom(setTypeLoadBalancer, setAllocateLoadBalancerNodePortsTrue),
patch: setTypeNodePort,
expect: makeValidServiceCustom(setTypeNodePort),
}, {
name: "clear allocatedLoadBalancerNodePorts false -> false",
svc: makeValidServiceCustom(setTypeLoadBalancer, setAllocateLoadBalancerNodePortsFalse),
patch: setTypeNodePort,
expect: makeValidServiceCustom(setTypeNodePort),
}, {
name: "set allocatedLoadBalancerNodePorts nil -> true",
svc: makeValidServiceCustom(setTypeLoadBalancer),
patch: patches(setTypeNodePort, setAllocateLoadBalancerNodePortsTrue),
expect: makeValidServiceCustom(setTypeNodePort, setAllocateLoadBalancerNodePortsTrue),
}, {
name: "set allocatedLoadBalancerNodePorts nil -> false",
svc: makeValidServiceCustom(setTypeLoadBalancer),
patch: patches(setTypeNodePort, setAllocateLoadBalancerNodePortsFalse),
expect: makeValidServiceCustom(setTypeNodePort, setAllocateLoadBalancerNodePortsFalse),
}, {
name: "set allocatedLoadBalancerNodePorts true -> nil",
svc: makeValidServiceCustom(setTypeLoadBalancer, setAllocateLoadBalancerNodePortsTrue),
patch: patches(setTypeNodePort, clearAllocateLoadBalancerNodePorts),
expect: makeValidServiceCustom(setTypeNodePort),
}, {
name: "set allocatedLoadBalancerNodePorts false -> nil",
svc: makeValidServiceCustom(setTypeLoadBalancer, setAllocateLoadBalancerNodePortsFalse),
patch: patches(setTypeNodePort, clearAllocateLoadBalancerNodePorts),
expect: makeValidServiceCustom(setTypeNodePort),
}, {
name: "set allocatedLoadBalancerNodePorts true -> false",
svc: makeValidServiceCustom(setTypeLoadBalancer, setAllocateLoadBalancerNodePortsTrue),
patch: patches(setTypeNodePort, setAllocateLoadBalancerNodePortsFalse),
expect: makeValidServiceCustom(setTypeNodePort, setAllocateLoadBalancerNodePortsFalse),
}, {
name: "set allocatedLoadBalancerNodePorts false -> true",
svc: makeValidServiceCustom(setTypeLoadBalancer, setAllocateLoadBalancerNodePortsFalse),
patch: patches(setTypeNodePort, setAllocateLoadBalancerNodePortsTrue),
expect: makeValidServiceCustom(setTypeNodePort, setAllocateLoadBalancerNodePortsTrue),
}}
for _, tc := range testCases {
@ -1053,6 +1103,9 @@ func TestDropTypeDependentFields(t *testing.T) {
if result.Spec.HealthCheckNodePort != tc.expect.Spec.HealthCheckNodePort {
t.Errorf("failed %q: expected healthCheckNodePort %d, got %d", tc.name, tc.expect.Spec.HealthCheckNodePort, result.Spec.HealthCheckNodePort)
}
if !reflect.DeepEqual(result.Spec.AllocateLoadBalancerNodePorts, tc.expect.Spec.AllocateLoadBalancerNodePorts) {
t.Errorf("failed %q: expected AllocateLoadBalancerNodePorts %v, got %v", tc.name, tc.expect.Spec.AllocateLoadBalancerNodePorts, result.Spec.AllocateLoadBalancerNodePorts)
}
})
}
}