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

Sync ingress-specific backends and minor logging changes #123

Merged
merged 2 commits into from
Feb 6, 2018

Conversation

nicksardo
Copy link
Contributor

@nicksardo nicksardo commented Feb 3, 2018

Changes

  • Backend service pool only syncs the list of service ports used for the respective ingress.
  • Consolidate all ingress references to a single object which was deep copied.
  • Changing log level of a frequently printed line from 4->5.
  • Minor modifications to event handlers

Example with two ingresses in separate namespaces:

Before

# While syncing any ingress
I0205 23:03:18.031082       1 backends.go:216] Sync: backends [{30466 HTTP testing/my-echo-svc {1 0 my-http-port}} {32349 HTTP default/my-echo-svc {1 0 my-http-port}}]

After

# While syncing default/my-echo-svc
I0205 22:59:43.566574       1 backends.go:224] Sync: backends [{32349 HTTP default/my-echo-svc {1 0 my-http-port} 80 false}]
# While syncing testing/my-echo-svc
I0205 22:59:45.215794       1 backends.go:224] Sync: backends [{30466 HTTP testing/my-echo-svc {1 0 my-http-port} 80 false}]

@nicksardo nicksardo added the do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. label Feb 3, 2018
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Feb 3, 2018
@nicksardo nicksardo changed the title Minor changes to backend service pool sync and logging Sync ingress-specific backends and minor logging change Feb 5, 2018
@nicksardo
Copy link
Contributor Author

nicksardo commented Feb 5, 2018

/cc @nikhiljindal
Not yet ready for review; just giving you a heads up.

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Feb 6, 2018
@nicksardo nicksardo changed the title Sync ingress-specific backends and minor logging change Sync ingress-specific backends and minor logging changes Feb 6, 2018
@nicksardo nicksardo removed the do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. label Feb 6, 2018
@nicksardo nicksardo requested a review from bowei February 6, 2018 01:34
@nicksardo
Copy link
Contributor Author

PTAL everyone

@nikhiljindal
Copy link
Contributor

/lgtm.
IIUC this should speed up the controller right? Only sync the ingress that changed!

@nicksardo
Copy link
Contributor Author

nicksardo commented Feb 6, 2018

This should improve performance along with #106.

@nicksardo nicksardo merged commit 5291f96 into kubernetes:master Feb 6, 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. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants