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

Condense backendPool and defaultBackendPool #242

Merged
merged 1 commit into from
May 10, 2018

Conversation

rramkumar1
Copy link
Contributor

@rramkumar1 rramkumar1 commented Apr 27, 2018

This PR attempts to condense management of all backends into one backend pool. Users of the condensed backend pool can now access the default backend's NodePort through a method in the BackendPool interface.

/assign @nicksardo

Should fix #184 & #127

@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 27, 2018
return &L7s{cloud, storage.NewInMemoryPool(), nil, defaultBackendPool, defaultBackendNodePort, namer}
// - backendPool: Used to sync BackendServices with the cloud.
func NewLoadBalancerPool(cloud LoadBalancers, backendPool backends.BackendPool, namer *utils.Namer) LoadBalancerPool {
return &L7s{cloud, storage.NewInMemoryPool(), backendPool, namer}
Copy link
Contributor

Choose a reason for hiding this comment

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

My goal was that the load balancer pool doesn't need to have a reference to the backendpool.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, gonna work on this more to see if I can remove it from LB pool.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was able to remove the backend pool reference from the load balancer pool. This required some semi-significant refactoring but I think everything is a lot cleaner now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed the tests files just for clarity.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 27, 2018
@rramkumar1 rramkumar1 changed the title Condense backendPool and defaultBackendPool Condense backendPool and defaultBackendPool [WIP] Apr 27, 2018
@rramkumar1 rramkumar1 force-pushed the default-backend-cleanup branch 4 times, most recently from 044c947 to 468956c Compare April 27, 2018 19:57
@rramkumar1 rramkumar1 changed the title Condense backendPool and defaultBackendPool [WIP] Condense backendPool and defaultBackendPool May 10, 2018
@nicksardo
Copy link
Contributor

Test panicked.

@nicksardo
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 10, 2018
@nicksardo nicksardo merged commit 8a842a9 into kubernetes:master May 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Condense backend pool with default backend pool
3 participants