From e588ae5e042a78fb76636e814c546bc3e67c9290 Mon Sep 17 00:00:00 2001 From: Pavithra Ramesh Date: Wed, 12 Sep 2018 12:42:24 -0700 Subject: [PATCH] Allow nodeName updates when endPoint is updated. One scenario where nodeName can change for the same ip address is if the endpoints are in hostNetwork mode and nodes are being added/deleted. With the current validation check, if endpoints controller misses a pod delete event, future endpoint updates will never succeed. removed unused helper functions --- pkg/apis/core/validation/validation.go | 52 ++++----------------- pkg/apis/core/validation/validation_test.go | 7 +-- 2 files changed, 12 insertions(+), 47 deletions(-) diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 8f2e6db5026..ee9d71e524a 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -5114,50 +5114,16 @@ func ValidateNamespaceFinalizeUpdate(newNamespace, oldNamespace *core.Namespace) return allErrs } -// Construct lookup map of old subset IPs to NodeNames. -func updateEpAddrToNodeNameMap(ipToNodeName map[string]string, addresses []core.EndpointAddress) { - for n := range addresses { - if addresses[n].NodeName == nil { - continue - } - ipToNodeName[addresses[n].IP] = *addresses[n].NodeName - } -} - -// Build a map across all subsets of IP -> NodeName -func buildEndpointAddressNodeNameMap(subsets []core.EndpointSubset) map[string]string { - ipToNodeName := make(map[string]string) - for i := range subsets { - updateEpAddrToNodeNameMap(ipToNodeName, subsets[i].Addresses) - updateEpAddrToNodeNameMap(ipToNodeName, subsets[i].NotReadyAddresses) - } - return ipToNodeName -} - -func validateEpAddrNodeNameTransition(addr *core.EndpointAddress, ipToNodeName map[string]string, fldPath *field.Path) field.ErrorList { - errList := field.ErrorList{} - existingNodeName, found := ipToNodeName[addr.IP] - if !found { - return errList - } - if addr.NodeName == nil || *addr.NodeName == existingNodeName { - return errList - } - // NodeName entry found for this endpoint IP, but user is attempting to change NodeName - return append(errList, field.Forbidden(fldPath, fmt.Sprintf("Cannot change NodeName for %s to %s", addr.IP, *addr.NodeName))) -} - // ValidateEndpoints tests if required fields are set. func ValidateEndpoints(endpoints *core.Endpoints) field.ErrorList { allErrs := ValidateObjectMeta(&endpoints.ObjectMeta, true, ValidateEndpointsName, field.NewPath("metadata")) allErrs = append(allErrs, ValidateEndpointsSpecificAnnotations(endpoints.Annotations, field.NewPath("annotations"))...) - allErrs = append(allErrs, validateEndpointSubsets(endpoints.Subsets, []core.EndpointSubset{}, field.NewPath("subsets"))...) + allErrs = append(allErrs, validateEndpointSubsets(endpoints.Subsets, field.NewPath("subsets"))...) return allErrs } -func validateEndpointSubsets(subsets []core.EndpointSubset, oldSubsets []core.EndpointSubset, fldPath *field.Path) field.ErrorList { +func validateEndpointSubsets(subsets []core.EndpointSubset, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} - ipToNodeName := buildEndpointAddressNodeNameMap(oldSubsets) for i := range subsets { ss := &subsets[i] idxPath := fldPath.Index(i) @@ -5168,10 +5134,10 @@ func validateEndpointSubsets(subsets []core.EndpointSubset, oldSubsets []core.En allErrs = append(allErrs, field.Required(idxPath, "must specify `addresses` or `notReadyAddresses`")) } for addr := range ss.Addresses { - allErrs = append(allErrs, validateEndpointAddress(&ss.Addresses[addr], idxPath.Child("addresses").Index(addr), ipToNodeName)...) + allErrs = append(allErrs, validateEndpointAddress(&ss.Addresses[addr], idxPath.Child("addresses").Index(addr))...) } for addr := range ss.NotReadyAddresses { - allErrs = append(allErrs, validateEndpointAddress(&ss.NotReadyAddresses[addr], idxPath.Child("notReadyAddresses").Index(addr), ipToNodeName)...) + allErrs = append(allErrs, validateEndpointAddress(&ss.NotReadyAddresses[addr], idxPath.Child("notReadyAddresses").Index(addr))...) } for port := range ss.Ports { allErrs = append(allErrs, validateEndpointPort(&ss.Ports[port], len(ss.Ports) > 1, idxPath.Child("ports").Index(port))...) @@ -5181,7 +5147,7 @@ func validateEndpointSubsets(subsets []core.EndpointSubset, oldSubsets []core.En return allErrs } -func validateEndpointAddress(address *core.EndpointAddress, fldPath *field.Path, ipToNodeName map[string]string) field.ErrorList { +func validateEndpointAddress(address *core.EndpointAddress, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} for _, msg := range validation.IsValidIP(address.IP) { allErrs = append(allErrs, field.Invalid(fldPath.Child("ip"), address.IP, msg)) @@ -5195,10 +5161,6 @@ func validateEndpointAddress(address *core.EndpointAddress, fldPath *field.Path, allErrs = append(allErrs, field.Invalid(fldPath.Child("nodeName"), *address.NodeName, msg)) } } - allErrs = append(allErrs, validateEpAddrNodeNameTransition(address, ipToNodeName, fldPath.Child("nodeName"))...) - if len(allErrs) > 0 { - return allErrs - } allErrs = append(allErrs, validateNonSpecialIP(address.IP, fldPath.Child("ip"))...) return allErrs } @@ -5250,9 +5212,11 @@ func validateEndpointPort(port *core.EndpointPort, requireName bool, fldPath *fi } // ValidateEndpointsUpdate tests to make sure an endpoints update can be applied. +// NodeName changes are allowed during update to accommodate the case where nodeIP or PodCIDR is reused. +// An existing endpoint ip will have a different nodeName if this happens. func ValidateEndpointsUpdate(newEndpoints, oldEndpoints *core.Endpoints) field.ErrorList { allErrs := ValidateObjectMetaUpdate(&newEndpoints.ObjectMeta, &oldEndpoints.ObjectMeta, field.NewPath("metadata")) - allErrs = append(allErrs, validateEndpointSubsets(newEndpoints.Subsets, oldEndpoints.Subsets, field.NewPath("subsets"))...) + allErrs = append(allErrs, validateEndpointSubsets(newEndpoints.Subsets, field.NewPath("subsets"))...) allErrs = append(allErrs, ValidateEndpointsSpecificAnnotations(newEndpoints.Annotations, field.NewPath("annotations"))...) return allErrs } diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 7a5a8b08a91..a0647682ac3 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -12965,11 +12965,12 @@ func newNodeNameEndpoint(nodeName string) *core.Endpoints { func TestEndpointAddressNodeNameUpdateRestrictions(t *testing.T) { oldEndpoint := newNodeNameEndpoint("kubernetes-node-setup-by-backend") updatedEndpoint := newNodeNameEndpoint("kubernetes-changed-nodename") - // Check that NodeName cannot be changed during update (if already set) + // Check that NodeName can be changed during update, this is to accommodate the case where nodeIP or PodCIDR is reused. + // The same ip will now have a different nodeName. errList := ValidateEndpoints(updatedEndpoint) errList = append(errList, ValidateEndpointsUpdate(updatedEndpoint, oldEndpoint)...) - if len(errList) == 0 { - t.Error("Endpoint should not allow changing of Subset.Addresses.NodeName on update") + if len(errList) != 0 { + t.Error("Endpoint should allow changing of Subset.Addresses.NodeName on update") } }