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

Issue 572 - Graceful termination + Update to go-1.10.8, alpine-3.9 #706

Merged
merged 14 commits into from
Apr 25, 2019
Merged

Conversation

roffe
Copy link
Collaborator

@roffe roffe commented Apr 9, 2019

Add graceful termination for IPVS destinations
Cleanup main loop of Network Service Controller
Stop removing services with 0 endpoints
Upgrade vendored libnetwork & netlink library ( to get statistics for IPVS destinations )
Update to go-1.10.8 & alpine 3.9

@roffe roffe changed the title Issue 572 Issue 572 - Graceful termination + Update to go-1.11.8, alpine-3.9 Apr 10, 2019
@mrueg
Copy link
Collaborator

mrueg commented Apr 10, 2019

Not sure if it will help to run the go test with -v for more verbosity?

@roffe roffe changed the title Issue 572 - Graceful termination + Update to go-1.11.8, alpine-3.9 Issue 572 - Graceful termination + Update to go-1.10.8, alpine-3.9 Apr 10, 2019
@roffe
Copy link
Collaborator Author

roffe commented Apr 10, 2019

@mrueg i've spent the whole day trying to get the tests working on 1.11 but it seems the routing controller ones are the ones failing, specificly it seems it's related to gobgp somehow deadlocking in the tests.

@roffe
Copy link
Collaborator Author

roffe commented Apr 10, 2019

Closes #572

@@ -3,7 +3,7 @@ services:

language: go
go:
- 1.10.x
- 1.10.8
Copy link
Collaborator

Choose a reason for hiding this comment

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

1.10.x is technically 1.10.8, not sure if this is needed

Copy link
Collaborator

Choose a reason for hiding this comment

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

would suggest to add a comment that tests are stuck with 1.11.x and later here instead

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The tests for the router controller does not work with Go 1.11, i suspect it being due to the old vendored goBGP server

@@ -1059,9 +1100,9 @@ func (nsc *NetworkServicesController) syncIpvsServices(serviceInfoMap serviceInf
// cleanup stale IPs on dummy interface
glog.V(1).Info("Cleaning up if any, old service IPs on dummy interface")
addrActive := make(map[string]bool)
for k, endpoints := range activeServiceEndpointMap {
for k := range activeServiceEndpointMap {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@murali-reddy The problem I had is that services that does not have any endpoints are flapping, making many unnecessary LN calls to add and remove the service on every sync. also if the only endpoint in the service is gracefully terminating we can't yank the service out under it.

}
}
}

func (nsc *NetworkServicesController) sync() error {
func (nsc *NetworkServicesController) sync(syncType int) {
Copy link
Collaborator Author

@roffe roffe Apr 12, 2019

Choose a reason for hiding this comment

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

The new behaviour is instead of having a blocking sync() on every update we have a overflowing channel, if there is a request to sync while we are syncing we will do it in the next run() loop. we won't have X-XX amount of them hanging in the air due to the mutex

@murali-reddy
Copy link
Member

thanks @roffe LGTM

@murali-reddy murali-reddy merged commit 54eedcd into cloudnativelabs:master Apr 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants