From 6b83f89d476717a452f6f4edb58f652d996a8f99 Mon Sep 17 00:00:00 2001 From: juanvallejo Date: Thu, 22 Sep 2016 16:35:08 -0400 Subject: [PATCH 1/4] Add option to set a service nodeport This patch adds the option to set a nodeport when creating a NodePort service. In case of a port allocation error due to a specified port being out of the valid range, the error now includes the valid range. If a `--node-port` value is not specified, it defaults to zero, in which case the allocator will default to its current behavior of assigning an available port. This patch also adds a new helper function in `cmd/util/helpers.go` to retrieve `Int32` cobra flags. **Example** ``` $ kubectl create service nodeport mynodeport --tcp=8080:7777 --node-port=1 The Service "mynodeport" is invalid: spec.ports[0].nodePort: Invalid value: 1: provided port is not in the valid range. Valid ports range from 30000-32767 $ kubectl create service nodeport mynodeport --tcp=8080:7777 --node-port=30000 service "mynodeport" created $ oc describe service mynodeport Name: mynodeport Namespace: default Labels: app=mynodeport Selector: app=mynodeport Type: NodePort IP: 172.30.81.254 Port: 8080-7777 8080/TCP NodePort: 8080-7777 30000/TCP Endpoints: Session Affinity: None No events. ``` --- pkg/kubectl/cmd/create_service.go | 2 ++ pkg/kubectl/cmd/util/helpers.go | 21 +++++++++++++------ pkg/kubectl/service_basic.go | 4 ++++ .../core/service/portallocator/allocator.go | 5 ++++- 4 files changed, 25 insertions(+), 7 deletions(-) diff --git a/pkg/kubectl/cmd/create_service.go b/pkg/kubectl/cmd/create_service.go index 61658c42280..8a1459812d3 100644 --- a/pkg/kubectl/cmd/create_service.go +++ b/pkg/kubectl/cmd/create_service.go @@ -134,6 +134,7 @@ func NewCmdCreateServiceNodePort(f *cmdutil.Factory, cmdOut io.Writer) *cobra.Co cmdutil.AddValidateFlags(cmd) cmdutil.AddPrinterFlags(cmd) cmdutil.AddGeneratorFlags(cmd, cmdutil.ServiceNodePortGeneratorV1Name) + cmd.Flags().Int32("node-port", 0, "Port used to expose the service on each node in a cluster.") addPortFlags(cmd) return cmd } @@ -152,6 +153,7 @@ func CreateServiceNodePort(f *cmdutil.Factory, cmdOut io.Writer, cmd *cobra.Comm TCP: cmdutil.GetFlagStringSlice(cmd, "tcp"), Type: api.ServiceTypeNodePort, ClusterIP: "", + NodePort: cmdutil.GetFlagInt32(cmd, "node-port"), } default: return cmdutil.UsageError(cmd, fmt.Sprintf("Generator: %s not supported.", generatorName)) diff --git a/pkg/kubectl/cmd/util/helpers.go b/pkg/kubectl/cmd/util/helpers.go index 97f6ce0d369..60b1c973791 100644 --- a/pkg/kubectl/cmd/util/helpers.go +++ b/pkg/kubectl/cmd/util/helpers.go @@ -296,7 +296,7 @@ func isWatch(cmd *cobra.Command) bool { func GetFlagString(cmd *cobra.Command, flag string) string { s, err := cmd.Flags().GetString(flag) if err != nil { - glog.Fatalf("err accessing flag %s for command %s: %v", flag, cmd.Name(), err) + glog.Fatalf("error accessing flag %s for command %s: %v", flag, cmd.Name(), err) } return s } @@ -305,7 +305,7 @@ func GetFlagString(cmd *cobra.Command, flag string) string { func GetFlagStringSlice(cmd *cobra.Command, flag string) []string { s, err := cmd.Flags().GetStringSlice(flag) if err != nil { - glog.Fatalf("err accessing flag %s for command %s: %v", flag, cmd.Name(), err) + glog.Fatalf("error accessing flag %s for command %s: %v", flag, cmd.Name(), err) } return s } @@ -331,7 +331,7 @@ func GetWideFlag(cmd *cobra.Command) bool { func GetFlagBool(cmd *cobra.Command, flag string) bool { b, err := cmd.Flags().GetBool(flag) if err != nil { - glog.Fatalf("err accessing flag %s for command %s: %v", flag, cmd.Name(), err) + glog.Fatalf("error accessing flag %s for command %s: %v", flag, cmd.Name(), err) } return b } @@ -340,7 +340,7 @@ func GetFlagBool(cmd *cobra.Command, flag string) bool { func GetFlagInt(cmd *cobra.Command, flag string) int { i, err := cmd.Flags().GetInt(flag) if err != nil { - glog.Fatalf("err accessing flag %s for command %s: %v", flag, cmd.Name(), err) + glog.Fatalf("error accessing flag %s for command %s: %v", flag, cmd.Name(), err) } return i } @@ -349,7 +349,16 @@ func GetFlagInt(cmd *cobra.Command, flag string) int { func GetFlagInt64(cmd *cobra.Command, flag string) int64 { i, err := cmd.Flags().GetInt64(flag) if err != nil { - glog.Fatalf("err accessing flag %s for command %s: %v", flag, cmd.Name(), err) + glog.Fatalf("error accessing flag %s for command %s: %v", flag, cmd.Name(), err) + } + return i +} + +// Assumes the flag has a default value. +func GetFlagInt32(cmd *cobra.Command, flag string) int32 { + i, err := cmd.Flags().GetInt32(flag) + if err != nil { + glog.Fatalf("error accessing flag %s for command %s: %v", flag, cmd.Name(), err) } return i } @@ -357,7 +366,7 @@ func GetFlagInt64(cmd *cobra.Command, flag string) int64 { func GetFlagDuration(cmd *cobra.Command, flag string) time.Duration { d, err := cmd.Flags().GetDuration(flag) if err != nil { - glog.Fatalf("err accessing flag %s for command %s: %v", flag, cmd.Name(), err) + glog.Fatalf("error accessing flag %s for command %s: %v", flag, cmd.Name(), err) } return d } diff --git a/pkg/kubectl/service_basic.go b/pkg/kubectl/service_basic.go index ee080aae3a9..c97c22ca12a 100644 --- a/pkg/kubectl/service_basic.go +++ b/pkg/kubectl/service_basic.go @@ -31,6 +31,7 @@ type ServiceCommonGeneratorV1 struct { TCP []string Type api.ServiceType ClusterIP string + NodePort int32 } type ServiceClusterIPGeneratorV1 struct { @@ -56,6 +57,7 @@ func (ServiceNodePortGeneratorV1) ParamNames() []GeneratorParam { return []GeneratorParam{ {"name", true}, {"tcp", true}, + {"nodeport", true}, } } func (ServiceLoadBalancerGeneratorV1) ParamNames() []GeneratorParam { @@ -174,12 +176,14 @@ func (s ServiceCommonGeneratorV1) StructuredGenerate() (runtime.Object, error) { if err != nil { return nil, err } + portName := strings.Replace(tcpString, ":", "-", -1) ports = append(ports, api.ServicePort{ Name: portName, Port: port, TargetPort: targetPort, Protocol: api.Protocol("TCP"), + NodePort: s.NodePort, }) } diff --git a/pkg/registry/core/service/portallocator/allocator.go b/pkg/registry/core/service/portallocator/allocator.go index 92a28777599..b84ad4378a9 100644 --- a/pkg/registry/core/service/portallocator/allocator.go +++ b/pkg/registry/core/service/portallocator/allocator.go @@ -82,7 +82,10 @@ func (r *PortAllocator) Free() int { func (r *PortAllocator) Allocate(port int) error { ok, offset := r.contains(port) if !ok { - return ErrNotInRange + // include valid port range in error + validPorts := r.portRange.String() + msg := fmt.Sprintf("Valid ports range is %s", validPorts) + return fmt.Errorf("%v. %s", ErrNotInRange, msg) } allocated, err := r.alloc.Allocate(offset) From cfbdcec7d6e8d1fbbefe12cb14bbe0764100805f Mon Sep 17 00:00:00 2001 From: juanvallejo Date: Fri, 23 Sep 2016 10:21:20 -0400 Subject: [PATCH 2/4] make portallocator.ErrNotInRange a type --- pkg/kubectl/cmd/create_service.go | 4 ++-- pkg/kubectl/service_basic.go | 4 ++-- .../core/service/portallocator/allocator.go | 12 +++++++++--- .../core/service/portallocator/allocator_test.go | 13 ++++++++++--- .../core/service/portallocator/controller/repair.go | 2 +- 5 files changed, 24 insertions(+), 11 deletions(-) diff --git a/pkg/kubectl/cmd/create_service.go b/pkg/kubectl/cmd/create_service.go index 8a1459812d3..115f4c54a89 100644 --- a/pkg/kubectl/cmd/create_service.go +++ b/pkg/kubectl/cmd/create_service.go @@ -134,7 +134,7 @@ func NewCmdCreateServiceNodePort(f *cmdutil.Factory, cmdOut io.Writer) *cobra.Co cmdutil.AddValidateFlags(cmd) cmdutil.AddPrinterFlags(cmd) cmdutil.AddGeneratorFlags(cmd, cmdutil.ServiceNodePortGeneratorV1Name) - cmd.Flags().Int32("node-port", 0, "Port used to expose the service on each node in a cluster.") + cmd.Flags().Int("node-port", 0, "Port used to expose the service on each node in a cluster.") addPortFlags(cmd) return cmd } @@ -153,7 +153,7 @@ func CreateServiceNodePort(f *cmdutil.Factory, cmdOut io.Writer, cmd *cobra.Comm TCP: cmdutil.GetFlagStringSlice(cmd, "tcp"), Type: api.ServiceTypeNodePort, ClusterIP: "", - NodePort: cmdutil.GetFlagInt32(cmd, "node-port"), + NodePort: cmdutil.GetFlagInt(cmd, "node-port"), } default: return cmdutil.UsageError(cmd, fmt.Sprintf("Generator: %s not supported.", generatorName)) diff --git a/pkg/kubectl/service_basic.go b/pkg/kubectl/service_basic.go index c97c22ca12a..9f1c3d2b6e4 100644 --- a/pkg/kubectl/service_basic.go +++ b/pkg/kubectl/service_basic.go @@ -31,7 +31,7 @@ type ServiceCommonGeneratorV1 struct { TCP []string Type api.ServiceType ClusterIP string - NodePort int32 + NodePort int } type ServiceClusterIPGeneratorV1 struct { @@ -183,7 +183,7 @@ func (s ServiceCommonGeneratorV1) StructuredGenerate() (runtime.Object, error) { Port: port, TargetPort: targetPort, Protocol: api.Protocol("TCP"), - NodePort: s.NodePort, + NodePort: int32(s.NodePort), }) } diff --git a/pkg/registry/core/service/portallocator/allocator.go b/pkg/registry/core/service/portallocator/allocator.go index b84ad4378a9..8e3d42be08c 100644 --- a/pkg/registry/core/service/portallocator/allocator.go +++ b/pkg/registry/core/service/portallocator/allocator.go @@ -37,11 +37,18 @@ type Interface interface { var ( ErrFull = errors.New("range is full") - ErrNotInRange = errors.New("provided port is not in the valid range") ErrAllocated = errors.New("provided port is already allocated") ErrMismatchedNetwork = errors.New("the provided port range does not match the current port range") ) +type ErrNotInRange struct { + ValidPorts string +} + +func (e *ErrNotInRange) Error() string { + return fmt.Sprintf("provided port is not in the valid range. The range of valid ports is %s", e.ValidPorts) +} + type PortAllocator struct { portRange net.PortRange @@ -84,8 +91,7 @@ func (r *PortAllocator) Allocate(port int) error { if !ok { // include valid port range in error validPorts := r.portRange.String() - msg := fmt.Sprintf("Valid ports range is %s", validPorts) - return fmt.Errorf("%v. %s", ErrNotInRange, msg) + return &ErrNotInRange{validPorts} } allocated, err := r.alloc.Allocate(offset) diff --git a/pkg/registry/core/service/portallocator/allocator_test.go b/pkg/registry/core/service/portallocator/allocator_test.go index d060528b2ce..7386172b5e1 100644 --- a/pkg/registry/core/service/portallocator/allocator_test.go +++ b/pkg/registry/core/service/portallocator/allocator_test.go @@ -73,16 +73,23 @@ func TestAllocate(t *testing.T) { if err := r.Release(released); err != nil { t.Fatal(err) } - if err := r.Allocate(1); err != ErrNotInRange { + + err = r.Allocate(1) + if _, ok := err.(*ErrNotInRange); !ok { t.Fatal(err) } + if err := r.Allocate(10001); err != ErrAllocated { t.Fatal(err) } - if err := r.Allocate(20000); err != ErrNotInRange { + + err = r.Allocate(20000) + if _, ok := err.(*ErrNotInRange); !ok { t.Fatal(err) } - if err := r.Allocate(10201); err != ErrNotInRange { + + err = r.Allocate(10201) + if _, ok := err.(*ErrNotInRange); !ok { t.Fatal(err) } if f := r.Free(); f != 1 { diff --git a/pkg/registry/core/service/portallocator/controller/repair.go b/pkg/registry/core/service/portallocator/controller/repair.go index c91a9985ac6..b072fde703f 100644 --- a/pkg/registry/core/service/portallocator/controller/repair.go +++ b/pkg/registry/core/service/portallocator/controller/repair.go @@ -114,7 +114,7 @@ func (c *Repair) runOnce() error { // TODO: send event // port is broken, reallocate runtime.HandleError(fmt.Errorf("the port %d for service %s/%s was assigned to multiple services; please recreate", port, svc.Name, svc.Namespace)) - case portallocator.ErrNotInRange: + case err.(*portallocator.ErrNotInRange): // TODO: send event // port is broken, reallocate runtime.HandleError(fmt.Errorf("the port %d for service %s/%s is not within the port range %v; please recreate", port, svc.Name, svc.Namespace, c.portRange)) From 395f6fda816fbb82cab92379c4e1ee5f8821ea16 Mon Sep 17 00:00:00 2001 From: juanvallejo Date: Fri, 23 Sep 2016 16:02:40 -0400 Subject: [PATCH 3/4] update known flags --- hack/verify-flags/known-flags.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/hack/verify-flags/known-flags.txt b/hack/verify-flags/known-flags.txt index 90be3f0aaf5..b9cb24bad05 100644 --- a/hack/verify-flags/known-flags.txt +++ b/hack/verify-flags/known-flags.txt @@ -384,6 +384,7 @@ node-monitor-period node-name node-os-distro node-path-override +node-port node-startup-grace-period node-status-update-frequency node-sync-period From 7d1461be8ebe533bb2cd5999128b5ccbb9f1a972 Mon Sep 17 00:00:00 2001 From: juanvallejo Date: Fri, 30 Sep 2016 10:00:59 -0400 Subject: [PATCH 4/4] remove unused GetFlagInt32 func --- pkg/kubectl/cmd/util/helpers.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/pkg/kubectl/cmd/util/helpers.go b/pkg/kubectl/cmd/util/helpers.go index 60b1c973791..e35df6d4160 100644 --- a/pkg/kubectl/cmd/util/helpers.go +++ b/pkg/kubectl/cmd/util/helpers.go @@ -354,15 +354,6 @@ func GetFlagInt64(cmd *cobra.Command, flag string) int64 { return i } -// Assumes the flag has a default value. -func GetFlagInt32(cmd *cobra.Command, flag string) int32 { - i, err := cmd.Flags().GetInt32(flag) - if err != nil { - glog.Fatalf("error accessing flag %s for command %s: %v", flag, cmd.Name(), err) - } - return i -} - func GetFlagDuration(cmd *cobra.Command, flag string) time.Duration { d, err := cmd.Flags().GetDuration(flag) if err != nil {