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

GCE provider: Create TargetPool with 200 instances, then update with rest #27829

Merged
merged 2 commits into from
Jun 22, 2016
Merged

GCE provider: Create TargetPool with 200 instances, then update with rest #27829

merged 2 commits into from
Jun 22, 2016

Conversation

zmerlynn
Copy link
Member

GCE provider: Create TargetPool with 200 instances, then update with rest

Tested with 2000 nodes, this actually meets the GCE API specifications (which is nutty). Previous PR (#25178) was based on a mistaken understanding of a poorly documented set of limitations, and even poorer testing, for which I am embarassed.

Also includes the revert of #25178 (review commits separately).

Analytics

zmerlynn added 2 commits June 21, 2016 09:54
@zmerlynn
Revert "GCE: Allow nodes to exceed target pool maximums"
ebc65f1 
This reverts commit faf0c44.
@zmerlynn
GCE provider: Create TargetPool with 200 instances, then update with …
f63ac19 
…rest

Tested with 2000 nodes, this actually meets the GCE API specifications
(which is nutty). Previous PR (kubernetes#25178) was based on a mistaken
understanding of a poorly documented set of limitations, and even
poorer testing, for which I am embarassed.
@zmerlynn zmerlynn added priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. release-note Denotes a PR that will be considered when it comes time to generate release notes. cherrypick-candidate labels Jun 22, 2016
@zmerlynn zmerlynn added this to the v1.3 milestone Jun 22, 2016
@k8s-github-robot k8s-github-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jun 22, 2016
@k8s-bot
Copy link

k8s-bot commented Jun 22, 2016

GCE e2e build/test passed for commit f63ac19.

@wojtek-t wojtek-t added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 22, 2016
@wojtek-t
Copy link
Member

LGTM - thanks!

@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot
Copy link

k8s-bot commented Jun 22, 2016

GCE e2e build/test passed for commit f63ac19.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 48f2b11 into kubernetes:master Jun 22, 2016
@zmerlynn zmerlynn modified the milestones: v1.2, v1.3 Jun 22, 2016
@zmerlynn zmerlynn added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Jun 22, 2016
This was referenced Jun 22, 2016
Merged
Closed
@zmerlynn zmerlynn deleted the fix-tp-max-2 branch June 22, 2016 15:29
roberthbailey added a commit that referenced this pull request Jun 22, 2016
@roberthbailey
Merge pull request #27865 from zmerlynn/manual-cherrypick-25178
7f98cfe 
Manual cherry pick of #27829 on release-1.2
@roberthbailey roberthbailey modified the milestones: v1.3, v1.2 Jun 22, 2016
@eparis eparis removed cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. cherrypick-candidate labels Jun 24, 2016
@thockin
Copy link
Member

thockin commented Jun 27, 2016

@chrislovecnm

@chrislovecnm
Copy link
Contributor

chrislovecnm commented Jun 27, 2016 •
edited
Loading

@thockin ... I think I MIGHT be able to get ingress, up. I set affinity rules so that the RC would only be on two nodes. @bprashanth is assisting.

Fingers crossed

@zmerlynn
Copy link
Member Author

@chrislovecnm: We'll also have a v1.2.5 that includes this PR soon.

@roberthbailey
Copy link
Contributor

v1.2.5 is now out!

@chrislovecnm
Copy link
Contributor

@zmerlynn I am running 1.3 beta 2 and have 1009 nodes ... I am guessing I am going to hit a gce error...

@zmerlynn
Copy link
Member Author

I thought this made it into beta.2.

@chrislovecnm
Copy link
Contributor

@zmerlynn ping me on slack .. I am working with @bprashanth to figure out what is going on. May be user error.

@chrislovecnm
Copy link
Contributor

So GCE api is blowing up on me:

E0628 02:26:40.427634       7 utils.go:103] Requeuing pet-race-ui/pet-race-ui, err googleapi: Error 403: Exceeded limit 'MAX_INSTANCES_IN_INSTANCE_GROUP' on resource 'k8s-ig--c9a6052e1676112c'. Limit: 1000.0, limitExceeded, unable to get loadbalancer: Loadbalancer pet-race-ui-pet-race-ui--c9a6052e1676112c not in pool
I0628 02:26:40.429150       7 event.go:216] Event(api.ObjectReference{Kind:"Ingress", Namespace:"pet-race-ui", Name:"pet-race-ui", UID:"fb01f977-3cd5-11e6-af56-42010a800002", APIVersion:"extensions", ResourceVersion:"57749081", FieldPath:""}): type: 'Warning' reason: 'GCE :Quota' googleapi: Error 403: Exceeded limit 'MAX_INSTANCES_IN_INSTANCE_GROUP' on resource 'k8s-ig--c9a6052e1676112c'. Limit: 1000.0, limitExceeded
E0628 02:26:41.827753       7 utils.go:103] Requeuing pet-race-ui/pet-race-ui, err googleapi: Error 403: Exceeded limit 'MAX_INSTANCES_IN_INSTANCE_GROUP' on resource 'k8s-ig--c9a6052e1676112c'. Limit: 1000.0, limitExceeded, unable to get loadbalancer: Loadbalancer pet-race-ui-pet-race-ui--c9a6052e1676112c not in pool
I0628 02:26:41.827937       7 event.go:216] Event(api.ObjectReference{Kind:"Ingress", Namespace:"pet-race-ui", Name:"pet-race-ui", UID:"fb01f977-3cd5-11e6-af56-42010a800002", APIVersion:"extensions", ResourceVersion:"57749081", FieldPath:""}): type: 'Warning' reason: 'GCE :Quota' googleapi: Error 403: Exceeded limit 'MAX_INSTANCES_IN_INSTANCE_GROUP' on resource 'k8s-ig--c9a6052e1676112c'. Limit: 1000.0, limitExceeded

@bprashanth
Copy link
Contributor

That's a different type of lb.
This bug is about service.type=loadbalancer and target pools.
youre hitting the max limit on a single instance group.
Try making your serivce of type=lb and see if you get an error about taret pool size.

@bprashanth
Copy link
Contributor

Hmm, for the record apparently either the fix isn't in beta.2, or Chris is not using beta.2, because he got:

to create load balancer for service pet-race-ui/pet-race-ui: failed to create target pool afadfae6f3cd511e6af5642010a80000: googleapi: Error 413: Value for field 'resource.instances' is too large: maximum size 200 element(s); actual size 1000.
More details:
Reason: fieldSizeTooLarge, Message: Value for field 'resource.instances' is too large: maximum size 200 element(s); actual size 1000.
Reason: fieldSizeTooLarge, Message: Value for field 'resource.instances' is too large: maximum size 200 element(s); actual size 1000.

@zmerlynn
Copy link
Member Author

I just glanced, and this PR didn't make beta.2. I was thinking of the other scalability limitation from the same day, #27741. With #27741 and not this PR, the results above are consistent (which I should have remembered, since this was only an intermediate state that existed on my branch when I was testing on large clusters, and annoyingly in 1.3.0-beta.2).

@chrislovecnm
Copy link
Contributor

@zmerlynn I think we have a bug as well. Backend services are not being removed, upon deletion.

@zmerlynn
Copy link
Member Author

Please file an issue with details. This PR has already merged, there's no reason to have a conversation here.

@chrislovecnm
Copy link
Contributor

As soon as I hit enter I realized that. Need sleep ;)

shyamjvs pushed a commit to shyamjvs/kubernetes that referenced this pull request Dec 1, 2016
@roberthbailey
Merge pull request kubernetes#27865 from zmerlynn/manual-cherrypick-2…
a0fcb09 
…5178

Manual cherry pick of kubernetes#27829 on release-1.2
shouhong pushed a commit to shouhong/kubernetes that referenced this pull request Feb 14, 2017
@roberthbailey
Merge pull request kubernetes#27865 from zmerlynn/manual-cherrypick-2…
ad41e43 
…5178

Manual cherry pick of kubernetes#27829 on release-1.2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@gmarek gmarek

@cjcullen cjcullen

@wojtek-t wojtek-t

Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
v1.3
Development

Successfully merging this pull request may close these issues.