WIP: ServiceType & NodePort work #8707
Merged
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
A service with a NodePort set will listen on that port, on every node. This is both handy for some load balancers (AWS ELB) and for people that want to expose a service without using a load balancer.
This will replace publicIPs
|
LGTM so far |
We prevent it from being set by validation
…atest API We need to keep them in the API so that we can round-trip between versions.
Including some refactoring of IP allocator
We still don't actually enable this though!
Although it takes some time to destroy a load balancer, we hide this complexity from the user.
|
LGTM |
thockin
added
the
lgtm
dchen1107
added a commit
that referenced
this pull request
May 22, 2015
WIP: ServiceType & NodePort work
|
I am monitoring jenkin and will update the e2e status. |
|
Thanks Dawn and Justin! This is a huge improvement. |
|
@justinsb and @thockin My Jenkin run with this pr failed. Actually run is not finished, but I observed several service tests failure already: |
|
@dchen1107 this is on GCE right? Looking right now... |
|
e2e issues should be fixed by #8728 ; just running through a -up / -test / -down cycle. |
|
@justinsb Thanks for the fixing. I or @lavalamp or @quinton-hoole will update with you the jenkin result tomorrow morning. Sometimes I have trouble to access our Jenkin through VPN too. |
|
@justinsb @thockin Still have one consistent failure: |
|
These two tests have occasional flakes:
|
|
I think we can live with this, if @justinsb will fix the services.sh test. :) |
|
Will definitely fix - sorry! If you can provide me with an output from one of the flakes, I can have a look at those as well. |
|
Hm there are actually a number of flakes in this most recent run: |
|
Status update: I now get a different error from services.sh; it looks like it isn't doing strict round-robin between the nodes. It takes a while for endpoints to propogate, and then "verifying the portals from a container fails because it sees one of the pods twice: I also have an idea on a potential cause for LB leakage - I made a late change to zero out the LoadBalancerStatus immediately, so I need to double-check the logic around service release here. Investigating both issues! |
|
Thanks for working on this, Justin! |
|
It is possible there is a leak. |
|
A new flake, possibly all because we've leaked load balancers and I don't know which it's safe to delete because they're all named with long hex values: |
|
I just deleted all the forwarding rules... Let's see if this makes it green for a bit. |
|
The default e2e setup does not configure any forwarding rules. So if this project is only running e2e, it should be safe to delete all the forwarding-rules (but I don't really know your setup). You could also check that they have no attached instances. I've also yet to see a leak when running it myself (at the end of the test, The hex string is the UUID of the k8s service, with a prefix of "a", I believe. |
|
So deleting all that stuff got us a totally green run. :) |
|
I think it's actually target pools that are leaking. The forwarding rules seem to get cleaned up. |
|
So to follow up: it's target pools leaking, from GKE's e2e test run that's using an old version. They share test projects & quotas with our GCE e2e testing. |
|
When I originally wrote the verifying code I seem to recal reading 2x as On Sat, May 23, 2015 at 4:35 PM, Justin Santa Barbara <
|
|
@a-robinson in case nobody did On Sun, May 24, 2015 at 2:14 PM, Daniel Smith notifications@github.com
|
|
The GKE tests leaking stuff is discussed in #7753 (comment). Tomorrow I'll check into adding the sleep to jenkins that I mention in the last comment there. |
|
I just saw this. So PublicIPs were deprecated, what were they replaced with here? |
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.
Integrating all the PRs into a clean/mergeable history. Mostly creating PR for testing purposes.