UPSTREAM: 33319: add nodeport option when creating NodePort service #11059
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
[test] |
csrwng
suggested changes
Sep 22, 2016
| func GetFlagInt32(cmd *cobra.Command, flag string) int32 { | ||
| i, err := cmd.Flags().GetInt32(flag) | ||
| if err != nil { | ||
| glog.Fatalf("err accessing flag %s for command %s: %v", flag, cmd.Name(), err) |
| @@ -133,6 +133,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("nodeport", 0, "Port used to expose the service on each node in a cluster.") | |||
There was a problem hiding this comment.
Seems like it should be named "node-port" based on the capitalization used in the API (NodePort)
| return ErrNotInRange | ||
| // include valid port range in error | ||
| validPorts := r.portRange.String() | ||
| msg := fmt.Sprintf("Valid ports range from %s", validPorts) |
There was a problem hiding this comment.
nit ... I think "Valid ports range is %s" would be better
juanvallejo
force-pushed
the
jvallejo/add-nodeport-option
branch
from
1588b81 to
fc8e559
Compare
|
@csrwng Review comments addressed, thanks for the feedback! |
juanvallejo
changed the title
juanvallejo
force-pushed
the
jvallejo/add-nodeport-option
branch
2 times, most recently
from
55fb03e to
2e5ece4
Compare
csrwng
approved these changes
Sep 23, 2016
juanvallejo
force-pushed
the
jvallejo/add-nodeport-option
branch
from
f80e7cf to
f7c9a97
Compare
|
conformance test flaked on #11079 re[test] |
openshift-bot
added
the
needs-rebase
juanvallejo
force-pushed
the
jvallejo/add-nodeport-option
branch
from
f7c9a97 to
5ed2b4c
Compare
openshift-bot
removed
the
needs-rebase
|
@fabianofranz or @csrwng upstream PR merged, PTAL |
|
@juanvallejo is this up-to-date with upstream (4 versus 2 commits)? |
|
@fabianofranz it's up to date, I had forgotten to squash upstream :/ |
UPSTREAM: kubernetes/kubernetes#33319 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** ``` $ oc 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 $ oc 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: <none> Session Affinity: None No events. ```
juanvallejo
force-pushed
the
jvallejo/add-nodeport-option
branch
from
5ed2b4c to
26655c5
Compare
|
Evaluated for origin test up to 26655c5 |
|
[merge] |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/9719/) (Image: devenv-rhel7_5156) |
|
Evaluated for origin merge up to 26655c5 |
|
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/9719/) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
UPSTREAM: kubernetes/kubernetes#33319
Related bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1378012
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
--nodeportvalue is not specified, it defaults to zero, inwhich 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.gotoretrieve
Int32cobra flags.Example
cc @openshift/cli-review @csrwng