Svc: Move ETP clearing to dropTypeDependentFields

I  am not sure why ExternalTrafficPolicy was different, but this is more
consistent with other field clearing logic.
This commit is contained in:
Tim Hockin 2021-07-12 12:34:05 -07:00
parent 6b21e064be
commit d13c920606
2 changed files with 15 additions and 22 deletions

View File

@ -346,25 +346,6 @@ func shouldAllocateNodePorts(service *api.Service) bool {
return false
}
// externalTrafficPolicyUpdate adjusts ExternalTrafficPolicy during service update if needed.
// It is necessary because we default ExternalTrafficPolicy field to different values.
// (NodePort / LoadBalancer: default is Global; Other types: default is empty.)
func externalTrafficPolicyUpdate(oldService, service *api.Service) {
var neededExternalTraffic, needsExternalTraffic bool
if oldService.Spec.Type == api.ServiceTypeNodePort ||
oldService.Spec.Type == api.ServiceTypeLoadBalancer {
neededExternalTraffic = true
}
if service.Spec.Type == api.ServiceTypeNodePort ||
service.Spec.Type == api.ServiceTypeLoadBalancer {
needsExternalTraffic = true
}
if neededExternalTraffic && !needsExternalTraffic {
// Clear ExternalTrafficPolicy to prevent confusion from ineffective field.
service.Spec.ExternalTrafficPolicy = api.ServiceExternalTrafficPolicyType("")
}
}
// healthCheckNodePortUpdate handles HealthCheckNodePort allocation/release
// and adjusts HealthCheckNodePort during service update if needed.
func (rs *REST) healthCheckNodePortUpdate(oldService, service *api.Service, nodePortOp *portallocator.PortAllocationOperation) (bool, error) {
@ -390,13 +371,12 @@ func (rs *REST) healthCheckNodePortUpdate(oldService, service *api.Service, node
klog.Infof("Transition to non LoadBalancer type service or LoadBalancer type service with ExternalTrafficPolicy=Global")
klog.V(4).Infof("Releasing healthCheckNodePort: %d", oldHealthCheckNodePort)
nodePortOp.ReleaseDeferred(int(oldHealthCheckNodePort))
// Clear the HealthCheckNodePort field.
service.Spec.HealthCheckNodePort = 0
// Case 3: Remain in needs HealthCheckNodePort.
// Reject changing the value of the HealthCheckNodePort field.
case neededHealthCheckNodePort && needsHealthCheckNodePort:
if oldHealthCheckNodePort != newHealthCheckNodePort {
//FIXME: Let validation do this.
klog.Warningf("Attempt to change value of health check node port DENIED")
fldPath := field.NewPath("spec", "healthCheckNodePort")
el := field.ErrorList{field.Invalid(fldPath, newHealthCheckNodePort,
@ -499,7 +479,6 @@ func (rs *REST) Update(ctx context.Context, name string, objInfo rest.UpdatedObj
if !success || err != nil {
return nil, false, err
}
externalTrafficPolicyUpdate(oldService, service)
if errs := validation.ValidateServiceExternalTrafficFieldsCombination(service); len(errs) > 0 {
return nil, false, errors.NewInvalid(api.Kind("Service"), service.Name, errs)
}

View File

@ -509,6 +509,12 @@ func dropTypeDependentFields(newSvc *api.Service, oldSvc *api.Service) {
newSvc.Spec.LoadBalancerClass = nil
}
// If a user is switching to a type that doesn't need ExternalTrafficPolicy
// AND they did not change this field, it is safe to drop it.
if needsExternalTrafficPolicy(oldSvc) && !needsExternalTrafficPolicy(newSvc) && sameExternalTrafficPolicy(oldSvc, newSvc) {
newSvc.Spec.ExternalTrafficPolicy = api.ServiceExternalTrafficPolicyType("")
}
// NOTE: there are other fields like `selector` which we could wipe.
// Historically we did not wipe them and they are not allocated from
// finite pools, so we are (currently) choosing to leave them alone.
@ -597,6 +603,14 @@ func sameLoadBalancerClass(oldSvc, newSvc *api.Service) bool {
return *oldSvc.Spec.LoadBalancerClass == *newSvc.Spec.LoadBalancerClass
}
func needsExternalTrafficPolicy(svc *api.Service) bool {
return svc.Spec.Type == api.ServiceTypeNodePort || svc.Spec.Type == api.ServiceTypeLoadBalancer
}
func sameExternalTrafficPolicy(oldSvc, newSvc *api.Service) bool {
return oldSvc.Spec.ExternalTrafficPolicy == newSvc.Spec.ExternalTrafficPolicy
}
// this func allows user to downgrade a service by just changing
// IPFamilyPolicy to SingleStack
func trimFieldsForDualStackDowngrade(newService, oldService *api.Service) {