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

Adding NodeName to EndpointSlice API, deprecation updates #96440

Merged
merged 8 commits into from
Nov 13, 2020
Merged

Adding NodeName to EndpointSlice API, deprecation updates #96440

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
9613f79
Adding NodeName to EndpointSlice API, deprecation updates
robscott Nov 11, 2020
e9573ee
Adding EndpointSliceNodeName feature gate
robscott Nov 11, 2020
d985438
Updating EndpointSlice controllers to support NodeName field
robscott Nov 11, 2020
877ad98
Updating ControlPlane to support NodeName field
robscott Nov 11, 2020
506861c
Removing "IP" from supported EndpointSlice address types in kube-proxy
robscott Nov 11, 2020
ff46573
Cleaning up EndpointSlice update validation tests
robscott Nov 11, 2020
b98cab7
Updating EndpointSlice strategy to cover alpha NodeName field
robscott Nov 11, 2020
84e4b30
Updates related to PR feedback
robscott Nov 12, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions api/openapi-spec/swagger.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

27 changes: 15 additions & 12 deletions pkg/apis/discovery/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,6 @@ type EndpointSlice struct {
type AddressType string

const (
// AddressTypeIP represents an IP Address.
// This address type has been deprecated and has been replaced by the IPv4
// and IPv6 adddress types. New resources with this address type will be
// considered invalid. This will be fully removed in 1.18.
// +deprecated
AddressTypeIP = AddressType("IP")
// AddressTypeIPv4 represents an IPv4 Address.
AddressTypeIPv4 = AddressType(api.IPv4Protocol)
// AddressTypeIPv6 represents an IPv6 Address.
Expand Down Expand Up @@ -105,8 +99,14 @@ type Endpoint struct {
// endpoint is located. This should match the corresponding node label.
// * topology.kubernetes.io/region: the value indicates the region where the
// endpoint is located. This should match the corresponding node label.
// This field is deprecated and will be removed in future api versions.
// +optional
Topology map[string]string
// nodeName represents the name of the Node hosting this endpoint. This can
// be used to determine endpoints local to a Node. This field can be enabled
// with the EndpointSliceNodeName feature gate.
// +optional
NodeName *string
}

// EndpointConditions represents the current condition of an endpoint.
Expand All @@ -118,15 +118,18 @@ type EndpointConditions struct {
// "true" for terminating endpoints.
Ready *bool

// serving is identical to ready except that it is set regardless of the terminating
// state of endpoints. This condition should be set to true for a ready endpoint that
// is terminating. If nil, consumers should defer to the ready condition.
// serving is identical to ready except that it is set regardless of the
// terminating state of endpoints. This condition should be set to true for
// a ready endpoint that is terminating. If nil, consumers should defer to
// the ready condition. This field can be enabled with the
// EndpointSliceTerminatingCondition feature gate.
// +optional
Serving *bool

// terminating indicates that this endpoint is terminating. A nil value indicates an
// unknown state. Consumers should interpret this unknown state to mean that the
// endpoint is not terminating.
// terminating indicates that this endpoint is terminating. A nil value
// indicates an unknown state. Consumers should interpret this unknown state
// to mean that the endpoint is not terminating. This field can be enabled
// with the EndpointSliceTerminatingCondition feature gate.
// +optional
Terminating *bool
}
Expand Down
2 changes: 2 additions & 0 deletions pkg/apis/discovery/v1alpha1/zz_generated.conversion.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions pkg/apis/discovery/v1beta1/zz_generated.conversion.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

28 changes: 14 additions & 14 deletions pkg/apis/discovery/validation/validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,6 @@ var (
string(discovery.AddressTypeIPv6),
string(discovery.AddressTypeFQDN),
)
deprecatedAddressTypes = sets.NewString(
string(discovery.AddressTypeIP),
)
supportedPortProtocols = sets.NewString(
string(api.ProtocolTCP),
string(api.ProtocolUDP),
Expand All @@ -53,9 +50,9 @@ var (
var ValidateEndpointSliceName = apimachineryvalidation.NameIsDNSSubdomain

// ValidateEndpointSlice validates an EndpointSlice.
func ValidateEndpointSlice(endpointSlice *discovery.EndpointSlice, validAddressTypes sets.String) field.ErrorList {
func ValidateEndpointSlice(endpointSlice *discovery.EndpointSlice) field.ErrorList {
allErrs := apivalidation.ValidateObjectMeta(&endpointSlice.ObjectMeta, true, ValidateEndpointSliceName, field.NewPath("metadata"))
allErrs = append(allErrs, validateAddressType(endpointSlice.AddressType, validAddressTypes)...)
allErrs = append(allErrs, validateAddressType(endpointSlice.AddressType)...)
allErrs = append(allErrs, validateEndpoints(endpointSlice.Endpoints, endpointSlice.AddressType, field.NewPath("endpoints"))...)
allErrs = append(allErrs, validatePorts(endpointSlice.Ports, field.NewPath("ports"))...)

Expand All @@ -64,12 +61,12 @@ func ValidateEndpointSlice(endpointSlice *discovery.EndpointSlice, validAddressT

// ValidateEndpointSliceCreate validates an EndpointSlice when it is created.
func ValidateEndpointSliceCreate(endpointSlice *discovery.EndpointSlice) field.ErrorList {
return ValidateEndpointSlice(endpointSlice, supportedAddressTypes)
return ValidateEndpointSlice(endpointSlice)
}

// ValidateEndpointSliceUpdate validates an EndpointSlice when it is updated.
func ValidateEndpointSliceUpdate(newEndpointSlice, oldEndpointSlice *discovery.EndpointSlice) field.ErrorList {
allErrs := ValidateEndpointSlice(newEndpointSlice, supportedAddressTypes.Union(deprecatedAddressTypes))
allErrs := ValidateEndpointSlice(newEndpointSlice)
allErrs = append(allErrs, apivalidation.ValidateImmutableField(newEndpointSlice.AddressType, oldEndpointSlice.AddressType, field.NewPath("addressType"))...)

return allErrs
Expand Down Expand Up @@ -97,10 +94,6 @@ func validateEndpoints(endpoints []discovery.Endpoint, addrType discovery.Addres
// This validates known address types, unknown types fall through
// and do not get validated.
switch addrType {
case discovery.AddressTypeIP:
for _, msg := range validation.IsValidIP(address) {
allErrs = append(allErrs, field.Invalid(addressPath.Index(i), address, msg))
}
case discovery.AddressTypeIPv4:
allErrs = append(allErrs, validation.IsValidIPv4Address(addressPath.Index(i), address)...)
case discovery.AddressTypeIPv6:
Expand All @@ -110,6 +103,13 @@ func validateEndpoints(endpoints []discovery.Endpoint, addrType discovery.Addres
}
}

if endpoint.NodeName != nil {
robscott marked this conversation as resolved.
Show resolved Hide resolved
nnPath := idxPath.Child("nodeName")
for _, msg := range apivalidation.ValidateNodeName(*endpoint.NodeName, false) {
allErrs = append(allErrs, field.Invalid(nnPath, *endpoint.NodeName, msg))
}
}

topologyPath := idxPath.Child("topology")
if len(endpoint.Topology) > maxTopologyLabels {
allErrs = append(allErrs, field.TooMany(topologyPath, len(endpoint.Topology), maxTopologyLabels))
Expand Down Expand Up @@ -162,13 +162,13 @@ func validatePorts(endpointPorts []discovery.EndpointPort, fldPath *field.Path)
return allErrs
}

func validateAddressType(addressType discovery.AddressType, validAddressTypes sets.String) field.ErrorList {
func validateAddressType(addressType discovery.AddressType) field.ErrorList {
allErrs := field.ErrorList{}

if addressType == "" {
allErrs = append(allErrs, field.Required(field.NewPath("addressType"), ""))
} else if !validAddressTypes.Has(string(addressType)) {
allErrs = append(allErrs, field.NotSupported(field.NewPath("addressType"), addressType, validAddressTypes.List()))
} else if !supportedAddressTypes.Has(string(addressType)) {
allErrs = append(allErrs, field.NotSupported(field.NewPath("addressType"), addressType, supportedAddressTypes.List()))
}

return allErrs
Expand Down
103 changes: 79 additions & 24 deletions pkg/apis/discovery/validation/validation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func TestValidateEndpointSlice(t *testing.T) {
expectedErrors: 0,
endpointSlice: &discovery.EndpointSlice{
ObjectMeta: standardMeta,
AddressType: discovery.AddressTypeIP,
AddressType: discovery.AddressTypeIPv4,
Ports: []discovery.EndpointPort{{
Name: utilpointer.StringPtr("one"),
Protocol: protocolPtr(api.ProtocolTCP),
Expand Down Expand Up @@ -378,7 +378,7 @@ func TestValidateEndpointSlice(t *testing.T) {
expectedErrors: 1,
endpointSlice: &discovery.EndpointSlice{
ObjectMeta: standardMeta,
AddressType: discovery.AddressTypeIP,
AddressType: discovery.AddressTypeIPv4,
Ports: []discovery.EndpointPort{{
Name: utilpointer.StringPtr("http"),
Protocol: protocolPtr(api.ProtocolTCP),
Expand Down Expand Up @@ -438,7 +438,7 @@ func TestValidateEndpointSlice(t *testing.T) {
expectedErrors: 1,
endpointSlice: &discovery.EndpointSlice{
ObjectMeta: standardMeta,
AddressType: discovery.AddressTypeIP,
AddressType: discovery.AddressTypeIPv4,
Ports: []discovery.EndpointPort{{
Name: utilpointer.StringPtr("http"),
Protocol: protocolPtr(api.ProtocolTCP),
Expand All @@ -458,7 +458,7 @@ func TestValidateEndpointSlice(t *testing.T) {

for name, testCase := range testCases {
t.Run(name, func(t *testing.T) {
errs := ValidateEndpointSlice(testCase.endpointSlice, supportedAddressTypes.Union(deprecatedAddressTypes))
errs := ValidateEndpointSlice(testCase.endpointSlice)
if len(errs) != testCase.expectedErrors {
t.Errorf("Expected %d errors, got %d errors: %v", testCase.expectedErrors, len(errs), errs)
}
Expand All @@ -473,8 +473,9 @@ func TestValidateEndpointSliceCreate(t *testing.T) {
}

testCases := map[string]struct {
expectedErrors int
endpointSlice *discovery.EndpointSlice
expectedErrors int
endpointSlice *discovery.EndpointSlice
nodeNameGateEnabled bool
}{
"good-slice": {
expectedErrors: 0,
Expand All @@ -491,13 +492,45 @@ func TestValidateEndpointSliceCreate(t *testing.T) {
}},
},
},
"good-slice-node-name": {
expectedErrors: 0,
endpointSlice: &discovery.EndpointSlice{
ObjectMeta: standardMeta,
AddressType: discovery.AddressTypeIPv4,
Ports: []discovery.EndpointPort{{
Name: utilpointer.StringPtr("http"),
Protocol: protocolPtr(api.ProtocolTCP),
}},
Endpoints: []discovery.Endpoint{{
Addresses: generateIPAddresses(1),
Hostname: utilpointer.StringPtr("valid-123"),
NodeName: utilpointer.StringPtr("valid-node-name"),
}},
},
},

// expected failures
"bad-node-name": {
expectedErrors: 1,
endpointSlice: &discovery.EndpointSlice{
ObjectMeta: standardMeta,
AddressType: discovery.AddressTypeIPv4,
Ports: []discovery.EndpointPort{{
Name: utilpointer.StringPtr("http"),
Protocol: protocolPtr(api.ProtocolTCP),
}},
Endpoints: []discovery.Endpoint{{
Addresses: generateIPAddresses(1),
Hostname: utilpointer.StringPtr("valid-123"),
NodeName: utilpointer.StringPtr("INvalid-node-name"),
}},
},
},
"deprecated-address-type": {
expectedErrors: 1,
endpointSlice: &discovery.EndpointSlice{
ObjectMeta: standardMeta,
AddressType: discovery.AddressTypeIP,
AddressType: discovery.AddressType("IP"),
Ports: []discovery.EndpointPort{{
Name: utilpointer.StringPtr("http"),
Protocol: protocolPtr(api.ProtocolTCP),
Expand Down Expand Up @@ -537,56 +570,78 @@ func TestValidateEndpointSliceUpdate(t *testing.T) {
standardMeta := metav1.ObjectMeta{Name: "es1", Namespace: "test"}

testCases := map[string]struct {
expectedErrors int
newEndpointSlice *discovery.EndpointSlice
oldEndpointSlice *discovery.EndpointSlice
expectedErrors int
nodeNameGateEnabled bool
oldEndpointSlice *discovery.EndpointSlice
newEndpointSlice *discovery.EndpointSlice
}{
"valid and identical slices": {
newEndpointSlice: &discovery.EndpointSlice{
oldEndpointSlice: &discovery.EndpointSlice{
ObjectMeta: standardMeta,
AddressType: discovery.AddressTypeIPv6,
},
oldEndpointSlice: &discovery.EndpointSlice{
newEndpointSlice: &discovery.EndpointSlice{
ObjectMeta: standardMeta,
AddressType: discovery.AddressTypeIPv6,
},
expectedErrors: 0,
},
"deprecated address type": {
expectedErrors: 0,

// expected errors
"invalide node name set": {
oldEndpointSlice: &discovery.EndpointSlice{
ObjectMeta: standardMeta,
AddressType: discovery.AddressTypeIPv4,
Endpoints: []discovery.Endpoint{{
Addresses: []string{"10.1.2.3"},
}},
},
newEndpointSlice: &discovery.EndpointSlice{
ObjectMeta: standardMeta,
AddressType: discovery.AddressTypeIP,
AddressType: discovery.AddressTypeIPv4,
Endpoints: []discovery.Endpoint{{
Addresses: []string{"10.1.2.3"},
NodeName: utilpointer.StringPtr("INVALID foo"),
}},
},
expectedErrors: 1,
},

"deprecated address type": {
expectedErrors: 1,
oldEndpointSlice: &discovery.EndpointSlice{
ObjectMeta: standardMeta,
AddressType: discovery.AddressTypeIP,
AddressType: discovery.AddressType("IP"),
},
},
"valid and identical slices with different address types": {
newEndpointSlice: &discovery.EndpointSlice{
ObjectMeta: standardMeta,
AddressType: discovery.AddressTypeIP,
AddressType: discovery.AddressType("IP"),
},
},
"valid and identical slices with different address types": {
oldEndpointSlice: &discovery.EndpointSlice{
ObjectMeta: standardMeta,
AddressType: discovery.AddressType("other"),
},
newEndpointSlice: &discovery.EndpointSlice{
ObjectMeta: standardMeta,
AddressType: discovery.AddressTypeIPv4,
},
expectedErrors: 1,
},
"invalid slices with valid address types": {
oldEndpointSlice: &discovery.EndpointSlice{
ObjectMeta: standardMeta,
AddressType: discovery.AddressTypeIPv4,
},
newEndpointSlice: &discovery.EndpointSlice{
ObjectMeta: standardMeta,
AddressType: discovery.AddressTypeIP,
AddressType: discovery.AddressTypeIPv4,
Ports: []discovery.EndpointPort{{
Name: utilpointer.StringPtr(""),
Protocol: protocolPtr(api.Protocol("invalid")),
}},
},
oldEndpointSlice: &discovery.EndpointSlice{
ObjectMeta: standardMeta,
AddressType: discovery.AddressTypeIP,
},
expectedErrors: 1,
},
}
Expand Down
Loading