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

Jump to bottom

Add option to set a service nodeport #33319

Merged
merged 4 commits into from
Oct 5, 2016
Merged

Add option to set a service nodeport #33319

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6b83f89
Add option to set a service nodeport
juanvallejo Sep 22, 2016
cfbdcec
make portallocator.ErrNotInRange a type
juanvallejo Sep 23, 2016
395f6fd
update known flags
juanvallejo Sep 23, 2016
7d1461b
remove unused GetFlagInt32 func
juanvallejo Sep 30, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions hack/verify-flags/known-flags.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions pkg/kubectl/cmd/create_service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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().Int("node-port", 0, "Port used to expose the service on each node in a cluster.")
addPortFlags(cmd)
return cmd
}
Expand All @@ -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.GetFlagInt(cmd, "node-port"),
}
default:
return cmdutil.UsageError(cmd, fmt.Sprintf("Generator: %s not supported.", generatorName))
Expand Down
12 changes: 6 additions & 6 deletions pkg/kubectl/cmd/util/helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
}
Expand All @@ -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
}
Expand All @@ -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
}
Expand All @@ -349,15 +349,15 @@ 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
}

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
}
Expand Down
4 changes: 4 additions & 0 deletions pkg/kubectl/service_basic.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ type ServiceCommonGeneratorV1 struct {
TCP []string
Type api.ServiceType
ClusterIP string
NodePort int
}

type ServiceClusterIPGeneratorV1 struct {
Expand All @@ -56,6 +57,7 @@ func (ServiceNodePortGeneratorV1) ParamNames() []GeneratorParam {
return []GeneratorParam{
{"name", true},
{"tcp", true},
{"nodeport", true},
}
}
func (ServiceLoadBalancerGeneratorV1) ParamNames() []GeneratorParam {
Expand Down Expand Up @@ -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: int32(s.NodePort),
})
}

Expand Down
13 changes: 11 additions & 2 deletions pkg/registry/core/service/portallocator/allocator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -82,7 +89,9 @@ 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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to turn ErrNotInRange into a type if you want to add information to it like this. People might be checking for equality with ErrNotInRange and this code would break them.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imo, this will break people who use ErrNotInRange to check allocate error.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, it's much better now. The compiler will show you any code that needs to be updated. :)

validPorts := r.portRange.String()
return &ErrNotInRange{validPorts}
}

allocated, err := r.alloc.Allocate(offset)
Expand Down
13 changes: 10 additions & 3 deletions pkg/registry/core/service/portallocator/allocator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion pkg/registry/core/service/portallocator/controller/repair.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down