github/kubernetes
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #114909 from aimuz/fix-114402

Browse Source 
fix: kubectl expose fails for apps with same-port, different-protocol
This commit is contained in:
Kubernetes Prow Robot
2023-07-06 03:27:03 -07:00
committed by GitHub
parent d48fc2ad2d e5116a39c9
commit 2075b20b28
7 changed files with 475 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

44
staging/src/k8s.io/kubectl/pkg/cmd/expose/expose.go
View File

@@ -38,7 +38,6 @@ import (
"k8s.io/cli-runtime/pkg/printers"
"k8s.io/cli-runtime/pkg/resource"
cmdutil "k8s.io/kubectl/pkg/cmd/util"
"k8s.io/kubectl/pkg/generate"
"k8s.io/kubectl/pkg/polymorphichelpers"
"k8s.io/kubectl/pkg/scheme"
"k8s.io/kubectl/pkg/util"
@@ -123,7 +122,7 @@ type ExposeServiceOptions struct {
CanBeExposed polymorphichelpers.CanBeExposedFunc
MapBasedSelectorForObject func(runtime.Object) (string, error)
PortsForObject polymorphichelpers.PortsForObjectFunc
ProtocolsForObject func(runtime.Object) (map[string]string, error)
ProtocolsForObject polymorphichelpers.MultiProtocolsWithForObjectFunc
Namespace string
Mapper meta.RESTMapper
@@ -276,7 +275,7 @@ func (o *ExposeServiceOptions) Complete(f cmdutil.Factory) error {
o.ClientForMapping = f.ClientForMapping
o.CanBeExposed = polymorphichelpers.CanBeExposedFn
o.MapBasedSelectorForObject = polymorphichelpers.MapBasedSelectorForObjectFn
o.ProtocolsForObject = polymorphichelpers.ProtocolsForObjectFn
o.ProtocolsForObject = polymorphichelpers.MultiProtocolsForObjectFn
o.PortsForObject = polymorphichelpers.PortsForObjectFn
o.Mapper, err = f.ToRESTMapper()
@@ -361,7 +360,7 @@ func (o *ExposeServiceOptions) RunExpose(cmd *cobra.Command, args []string) erro
if err != nil {
return fmt.Errorf("couldn't find protocol via introspection: %v", err)
}
if protocols := generate.MakeProtocols(protocolsMap); !generate.IsZero(protocols) {
if protocols := makeProtocols(protocolsMap); len(protocols) > 0 {
o.Protocols = protocols
}
@@ -375,13 +374,11 @@ func (o *ExposeServiceOptions) RunExpose(cmd *cobra.Command, args []string) erro
// Generate new object
service, err := o.createService()
if err != nil {
return err
}
overrideService, err := o.NewOverrider(&corev1.Service{}).Apply(service)
if err != nil {
return err
}
@@ -455,7 +452,7 @@ func (o *ExposeServiceOptions) createService() (*corev1.Service, error) {
}
}
var portProtocolMap map[string]string
var portProtocolMap map[string][]string
if o.Protocols != "" {
portProtocolMap, err = parseProtocols(o.Protocols)
if err != nil {
@@ -499,8 +496,20 @@ func (o *ExposeServiceOptions) createService() (*corev1.Service, error) {
case len(protocol) == 0 && len(portProtocolMap) > 0:
// no --protocol and we expose a multiprotocol resource
protocol = "TCP" // have the default so we can stay sane
if exposeProtocol, found := portProtocolMap[stillPortString]; found {
protocol = exposeProtocol
if exposeProtocols, found := portProtocolMap[stillPortString]; found {
if len(exposeProtocols) == 1 {
protocol = exposeProtocols[0]
break
}
for _, exposeProtocol := range exposeProtocols {
name := fmt.Sprintf("port-%d-%s", i+1, strings.ToLower(exposeProtocol))
ports = append(ports, corev1.ServicePort{
Name: name,
Port: int32(port),
Protocol: corev1.Protocol(exposeProtocol),
})
}
continue
}
}
ports = append(ports, corev1.ServicePort{
@@ -590,12 +599,22 @@ func parseLabels(labelSpec string) (map[string]string, error) {
return labels, nil
}
func makeProtocols(protocols map[string][]string) string {
var out []string
for key, value := range protocols {
for _, s := range value {
out = append(out, fmt.Sprintf("%s/%s", key, s))
}
}
return strings.Join(out, ",")
}
// parseProtocols turns a string representation of a protocols set into a map[string]string
func parseProtocols(protocols string) (map[string]string, error) {
func parseProtocols(protocols string) (map[string][]string, error) {
if len(protocols) == 0 {
return nil, fmt.Errorf("no protocols passed")
}
portProtocolMap := map[string]string{}
portProtocolMap := map[string][]string{}
protocolsSlice := strings.Split(protocols, ",")
for ix := range protocolsSlice {
portProtocol := strings.Split(protocolsSlice[ix], "/")
@@ -608,7 +627,8 @@ func parseProtocols(protocols string) (map[string]string, error) {
if len(portProtocol[1]) == 0 {
return nil, fmt.Errorf("unexpected empty protocol")
}
portProtocolMap[portProtocol[0]] = portProtocol[1]
port := portProtocol[0]
portProtocolMap[port] = append(portProtocolMap[port], portProtocol[1])
}
return portProtocolMap, nil
}

34
staging/src/k8s.io/kubectl/pkg/cmd/expose/expose_test.go
View File

@@ -1688,6 +1688,40 @@ func TestGenerateService(t *testing.T) {
},
},
},
// Fixed #114402 kubectl expose fails for apps with same-port, different-protocol
"test #114402": {
selector: "foo=bar,baz=blah",
name: "test",
clusterIP: "None",
protocols: "53/TCP,53/UDP",
port: "53",
expected: &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
},
Spec: corev1.ServiceSpec{
Selector: map[string]string{
"foo": "bar",
"baz": "blah",
},
Ports: []corev1.ServicePort{
{
Name: "port-1-tcp",
Port: 53,
Protocol: corev1.ProtocolTCP,
TargetPort: intstr.FromInt(53),
},
{
Name: "port-1-udp",
Port: 53,
Protocol: corev1.ProtocolUDP,
TargetPort: intstr.FromInt(53),
},
},
ClusterIP: corev1.ClusterIPNone,
},
},
},
"check selector": {
name: "test",
protocol: "SCTP",

10
staging/src/k8s.io/kubectl/pkg/polymorphichelpers/interface.go
View File

@@ -67,11 +67,21 @@ var MapBasedSelectorForObjectFn MapBasedSelectorForObjectFunc = mapBasedSelector
// ProtocolsForObjectFunc will call the provided function on the protocols for the object,
// return nil-map if no protocols for the object, or return an error.
// Deprecated: use PortsProtocolsForObjectFunc instead.
// When the same port has different protocols, data will be lost
type ProtocolsForObjectFunc func(object runtime.Object) (map[string]string, error)
// ProtocolsForObjectFn gives a way to easily override the function for unit testing if needed
// Deprecated: use MultiProtocolsForObjectFn instead.
var ProtocolsForObjectFn ProtocolsForObjectFunc = protocolsForObject
// MultiProtocolsWithForObjectFunc will call the provided function on the protocols for the object,
// return nil-map if no protocols for the object, or return an error.
type MultiProtocolsWithForObjectFunc func(object runtime.Object) (map[string][]string, error)
// MultiProtocolsForObjectFn gives a way to easily override the function for unit testing if needed
var MultiProtocolsForObjectFn MultiProtocolsWithForObjectFunc = multiProtocolsForObject
// PortsForObjectFunc returns the ports associated with the provided object
type PortsForObjectFunc func(object runtime.Object) ([]string, error)

95
staging/src/k8s.io/kubectl/pkg/polymorphichelpers/multiprotocolsforobject.go Normal file
View File

@@ -0,0 +1,95 @@
/*
Copyright 2018 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package polymorphichelpers
import (
"fmt"
"strconv"
appsv1 "k8s.io/api/apps/v1"
appsv1beta1 "k8s.io/api/apps/v1beta1"
appsv1beta2 "k8s.io/api/apps/v1beta2"
corev1 "k8s.io/api/core/v1"
extensionsv1beta1 "k8s.io/api/extensions/v1beta1"
"k8s.io/apimachinery/pkg/runtime"
)
func multiProtocolsForObject(object runtime.Object) (map[string][]string, error) {
// TODO: replace with a swagger schema based approach (identify pod selector via schema introspection)
switch t := object.(type) {
case *corev1.ReplicationController:
return getMultiProtocols(t.Spec.Template.Spec), nil
case *corev1.Pod:
return getMultiProtocols(t.Spec), nil
case *corev1.Service:
return getServiceMultiProtocols(t.Spec), nil
case *extensionsv1beta1.Deployment:
return getMultiProtocols(t.Spec.Template.Spec), nil
case *appsv1.Deployment:
return getMultiProtocols(t.Spec.Template.Spec), nil
case *appsv1beta2.Deployment:
return getMultiProtocols(t.Spec.Template.Spec), nil
case *appsv1beta1.Deployment:
return getMultiProtocols(t.Spec.Template.Spec), nil
case *extensionsv1beta1.ReplicaSet:
return getMultiProtocols(t.Spec.Template.Spec), nil
case *appsv1.ReplicaSet:
return getMultiProtocols(t.Spec.Template.Spec), nil
case *appsv1beta2.ReplicaSet:
return getMultiProtocols(t.Spec.Template.Spec), nil
default:
return nil, fmt.Errorf("cannot extract protocols from %T", object)
}
}
func getMultiProtocols(spec corev1.PodSpec) map[string][]string {
result := make(map[string][]string)
var protocol corev1.Protocol
for _, container := range spec.Containers {
for _, port := range container.Ports {
// Empty protocol must be defaulted (TCP)
protocol = corev1.ProtocolTCP
if len(port.Protocol) > 0 {
protocol = port.Protocol
}
p := strconv.Itoa(int(port.ContainerPort))
result[p] = append(result[p], string(protocol))
}
}
return result
}
// Extracts the protocols exposed by a service from the given service spec.
func getServiceMultiProtocols(spec corev1.ServiceSpec) map[string][]string {
result := make(map[string][]string)
var protocol corev1.Protocol
for _, servicePort := range spec.Ports {
// Empty protocol must be defaulted (TCP)
protocol = corev1.ProtocolTCP
if len(servicePort.Protocol) > 0 {
protocol = servicePort.Protocol
}
p := strconv.Itoa(int(servicePort.Port))
result[p] = append(result[p], string(protocol))
}
return result
}

213
staging/src/k8s.io/kubectl/pkg/polymorphichelpers/multiprotocolsforobject_test.go Normal file
View File

@@ -0,0 +1,213 @@
/*
Copyright 2018 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package polymorphichelpers
import (
"reflect"
"testing"
corev1 "k8s.io/api/core/v1"
extensionsv1beta1 "k8s.io/api/extensions/v1beta1"
"k8s.io/apimachinery/pkg/runtime"
)
func TestMultiProtocolsForObject(t *testing.T) {
tests := []struct {
name string
object runtime.Object
expectErr bool
expected map[string][]string
}{
{
name: "pod with TCP protocol",
object: &corev1.Pod{
Spec: corev1.PodSpec{
Containers: []corev1.Container{
{
Ports: []corev1.ContainerPort{
{
ContainerPort: 101,
Protocol: "TCP",
},
},
},
},
},
},
expected: map[string][]string{"101": {"TCP"}},
},
// No protocol--should default to TCP.
{
name: "pod with no protocol",
object: &corev1.Pod{
Spec: corev1.PodSpec{
Containers: []corev1.Container{
{
Ports: []corev1.ContainerPort{
{
ContainerPort: 101,
},
},
},
},
},
},
expected: map[string][]string{"101": {"TCP"}},
},
{
name: "pod with same-port,different-protocol",
object: &corev1.Pod{
Spec: corev1.PodSpec{
Containers: []corev1.Container{
{
Ports: []corev1.ContainerPort{
{
ContainerPort: 101,
Protocol: "TCP",
},
{
ContainerPort: 101,
Protocol: "UDP",
},
},
},
},
},
},
expected: map[string][]string{"101": {"TCP", "UDP"}},
},
{
name: "service with TCP protocol",
object: &corev1.Service{
Spec: corev1.ServiceSpec{
Ports: []corev1.ServicePort{
{
Port: 101,
Protocol: "TCP",
},
},
},
},
expected: map[string][]string{"101": {"TCP"}},
},
// No protocol for service port--default to TCP
{
name: "service with no protocol",
object: &corev1.Service{
Spec: corev1.ServiceSpec{
Ports: []corev1.ServicePort{
{
Port: 101,
},
},
},
},
expected: map[string][]string{"101": {"TCP"}},
},
{
name: "replication with TCP protocol",
object: &corev1.ReplicationController{
Spec: corev1.ReplicationControllerSpec{
Template: &corev1.PodTemplateSpec{
Spec: corev1.PodSpec{
Containers: []corev1.Container{
{
Ports: []corev1.ContainerPort{
{
ContainerPort: 101,
Protocol: "TCP",
},
},
},
},
},
},
},
},
expected: map[string][]string{"101": {"TCP"}},
},
{
name: "deployment with TCP protocol",
object: &extensionsv1beta1.Deployment{
Spec: extensionsv1beta1.DeploymentSpec{
Template: corev1.PodTemplateSpec{
Spec: corev1.PodSpec{
Containers: []corev1.Container{
{
Ports: []corev1.ContainerPort{
{
ContainerPort: 101,
Protocol: "TCP",
},
},
},
},
},
},
},
},
expected: map[string][]string{"101": {"TCP"}},
},
{
name: "replicaset with TCP protocol",
object: &extensionsv1beta1.ReplicaSet{
Spec: extensionsv1beta1.ReplicaSetSpec{
Template: corev1.PodTemplateSpec{
Spec: corev1.PodSpec{
Containers: []corev1.Container{
{
Ports: []corev1.ContainerPort{
{
ContainerPort: 101,
Protocol: "TCP",
},
},
},
},
},
},
},
},
expected: map[string][]string{"101": {"TCP"}},
},
{
name: "unsupported object",
object: &corev1.Node{},
expectErr: true,
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
actual, err := multiProtocolsForObject(test.object)
if test.expectErr {
if err == nil {
t.Error("unexpected non-error")
}
return
}
if !test.expectErr && err != nil {
t.Errorf("unexpected error: %v", err)
return
}
if !reflect.DeepEqual(actual, test.expected) {
t.Errorf("expected ports %v, but got %v", test.expected, actual)
}
})
}
}

20
staging/src/k8s.io/kubectl/pkg/polymorphichelpers/portsforobject.go
View File

@@ -60,19 +60,31 @@ func portsForObject(object runtime.Object) ([]string, error) {
}
func getPorts(spec corev1.PodSpec) []string {
result := []string{}
var result []string
exists := map[string]struct{}{}
for _, container := range spec.Containers {
for _, port := range container.Ports {
result = append(result, strconv.Itoa(int(port.ContainerPort)))
// remove duplicate ports
key := strconv.Itoa(int(port.ContainerPort))
if _, ok := exists[key]; !ok {
exists[key] = struct{}{}
result = append(result, key)
}
}
}
return result
}
func getServicePorts(spec corev1.ServiceSpec) []string {
result := []string{}
var result []string
exists := map[string]struct{}{}
for _, servicePort := range spec.Ports {
result = append(result, strconv.Itoa(int(servicePort.Port)))
// remove duplicate ports
key := strconv.Itoa(int(servicePort.Port))
if _, ok := exists[key]; !ok {
exists[key] = struct{}{}
result = append(result, key)
}
}
return result
}

78
staging/src/k8s.io/kubectl/pkg/polymorphichelpers/portsforobject_test.go
View File

@@ -30,6 +30,7 @@ func TestPortsForObject(t *testing.T) {
tests := []struct {
object runtime.Object
expectErr bool
expected []string
}{
{
object: &corev1.Pod{
@@ -45,6 +46,74 @@ func TestPortsForObject(t *testing.T) {
},
},
},
expected: []string{"101"},
},
{
object: &corev1.Pod{
Spec: corev1.PodSpec{
Containers: []corev1.Container{
{
Ports: []corev1.ContainerPort{
{
ContainerPort: 101,
},
{
ContainerPort: 102,
},
},
},
},
},
},
expected: []string{"101", "102"},
},
{
object: &corev1.Pod{
Spec: corev1.PodSpec{
Containers: []corev1.Container{
{
Ports: []corev1.ContainerPort{
{
ContainerPort: 101,
Protocol: corev1.ProtocolTCP,
},
{
ContainerPort: 101,
Protocol: corev1.ProtocolUDP,
},
{
ContainerPort: 102,
},
},
},
},
},
},
expected: []string{"101", "102"},
},
{
object: &corev1.Pod{
Spec: corev1.PodSpec{
Containers: []corev1.Container{
{
Ports: []corev1.ContainerPort{
{
ContainerPort: 101,
Protocol: corev1.ProtocolTCP,
},
{
ContainerPort: 102,
},
{
ContainerPort: 101,
Protocol: corev1.ProtocolUDP,
},
},
},
},
},
},
expected: []string{"101", "102"},
},
{
object: &corev1.Service{
@@ -56,6 +125,7 @@ func TestPortsForObject(t *testing.T) {
},
},
},
expected: []string{"101"},
},
{
object: &corev1.ReplicationController{
@@ -75,6 +145,7 @@ func TestPortsForObject(t *testing.T) {
},
},
},
expected: []string{"101"},
},
{
object: &extensionsv1beta1.Deployment{
@@ -94,6 +165,7 @@ func TestPortsForObject(t *testing.T) {
},
},
},
expected: []string{"101"},
},
{
object: &extensionsv1beta1.ReplicaSet{
@@ -113,13 +185,13 @@ func TestPortsForObject(t *testing.T) {
},
},
},
expected: []string{"101"},
},
{
object: &corev1.Node{},
expectErr: true,
},
}
expectedPorts := []string{"101"}
for _, test := range tests {
actual, err := portsForObject(test.object)
@@ -133,8 +205,8 @@ func TestPortsForObject(t *testing.T) {
t.Errorf("unexpected error: %v", err)
continue
}
if !reflect.DeepEqual(actual, expectedPorts) {
t.Errorf("expected ports %v, but got %v", expectedPorts, actual)
if !reflect.DeepEqual(actual, test.expected) {
t.Errorf("expected ports %v, but got %v", test.expected, actual)
}
}
}