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.

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

Rename "portalIP" to "clusterIP" #8740

Merged
merged 5 commits into from
May 28, 2015
Merged

Conversation

thockin
Copy link
Member

@thockin thockin commented May 23, 2015

The term "portal" is confusing to people - we don't need another term for a concept that is closely aligned to one we already know - VIP.

API: Rename the 'portalIP' field to 'clusterIP' in v1 API (v1beta3 and earlier are unchanged)
Flags: Alias the --portal-net flag (on apiserver) to --service-cluster-ip-range. Old flag will continue to work.
Turnup: Rename the PORTAL_NET variable to SERVICE_CLUSTER_IP_RANGE

@bgrant0607
Copy link
Member

Looking good already. Thanks much.

@justinsb
Copy link
Member

I like this; it will be much more consistent.

@thockin thockin force-pushed the nix-portal branch 6 times, most recently from 4f81bc8 to 0a5d1ea Compare May 26, 2015 04:59
@thockin thockin changed the title WIP: Rename "portalIP" to "clusterIP" Rename "portalIP" to "clusterIP" May 26, 2015
@thockin
Copy link
Member Author

thockin commented May 26, 2015

This depends on the services doc update.

Removed WIP - I think it is done. e2e passes (except PD, grr)

@thockin
Copy link
Member Author

thockin commented May 26, 2015

FWIW I find it wordy in places but it is consistent.

@thockin
Copy link
Member Author

thockin commented May 26, 2015

@zmerlynn @roberthbailey @cjcullen regarding master-run addons and config params in general

@roberthbailey
Copy link
Contributor

Does this require a new kube2sky image to be built once the change goes in on the master side?

@bgrant0607 bgrant0607 added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label May 26, 2015
@bgrant0607
Copy link
Member

Please write a PR description that can be used in the release notes. The flag changes, in particular, are important to anyone whose turnup code isn't in our repo.

@@ -128,15 +138,15 @@ Accessing a `Service` without a selector works the same as if it had selector.
The traffic will be routed to endpoints defined by the user (`1.2.3.4:80` in
this example).

## Portals and service proxies
## Virtual IPs and service proxies
Copy link
Member

Choose a reason for hiding this comment

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

Possible ways to relate VIPs and cluster IPs:

  • Virtual cluster IPs
  • Virtual IPs (aka cluster IPs)
  • Cluster-local virtual IPs

@bgrant0607
Copy link
Member

Does this subsume #8655?

@bgrant0607
Copy link
Member

@roberthbailey This only changes the v1 API, so kube2sky, other add-ons, configs, examples, etc. don't need to be updated until we move them to v1.

applicable for services in general.
Sometimes you don't need or want load-balancing and a single virtual IP. In
this case, you can create "headless" services by specifying `"None"` for the
cluster IP (`spec.clusterIP` or `spec.portalIP` in v1beta3 and earlier APIs).
Copy link
Member

Choose a reason for hiding this comment

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

Needs a comma before "or spec.portalIP".

@bgrant0607
Copy link
Member

Looks pretty good overall.

@bgrant0607 bgrant0607 assigned bgrant0607 and unassigned ArtfulCoder May 26, 2015
@thockin
Copy link
Member Author

thockin commented May 26, 2015

PR description (first comment) updated.

Style question - is "ClusterIP" a noun or is it "Cluster IP". Ditto for "NodePort" vs "Node Port" ?

@bgrant0607
Copy link
Member

We're in no way consistent about this, but I'd reserve compound CamelCase (e.g., ReplicationController) for cases where you are referring to the code (fields, kinds, etc.) and just use separate words (e.g., Replication Controller) in most descriptions.

@bgrant0607
Copy link
Member

LGTM. Please merge #8655 first, which I expect to cause this to need to be rebased.

@bgrant0607 bgrant0607 added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 26, 2015
@thockin
Copy link
Member Author

thockin commented May 27, 2015

Rebased and pushed

@bgrant0607 bgrant0607 mentioned this pull request May 28, 2015
thockin added a commit that referenced this pull request May 28, 2015
Rename "portalIP" to "clusterIP"
@thockin thockin merged commit da78ab6 into kubernetes:master May 28, 2015
@j3ffml j3ffml mentioned this pull request May 29, 2015
@thockin thockin deleted the nix-portal branch June 25, 2015 04:09
k8s-github-robot pushed a commit that referenced this pull request Nov 9, 2017
Automatic merge from submit-queue (batch tested with PRs 54868, 52547). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Remove deprecated stale flags of kube-apiserver

**What this PR does / why we need it**:
These flags have been marked as deprecated for more than two years. This PR removes them.

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #
ref: #8740

**Special notes for your reviewer**:

**Release note**:

```release-note
action required: Deprecated flags `--portal-net` and `service-node-ports` of kube-apiserver are removed.
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants