-
Notifications
You must be signed in to change notification settings - Fork 303
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
Replace snapshotter in pkg/backends with a GCE cloud lister implementation that is solely used for garbage collection #514
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rramkumar1 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
…ation that is solely used for garbage collection
1ce2944
to
315742e
Compare
/assign @bowei |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
keyFunc := func(i interface{}) (string, error) { | ||
bs := i.(*compute.BackendService) | ||
if !namer.NameBelongsToCluster(bs.Name) { | ||
return "", fmt.Errorf("unrecognized name %v", bs.Name) | ||
} | ||
return bs.Name, nil | ||
} | ||
backendPool.snapshotter = storage.NewCloudListingPool("backends", keyFunc, backendPool, 30*time.Second) | ||
backendPool.gceLister = cloudlist.NewGCELister("backends", keyFunc, backendPool) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to share the backends lister with anything outside pkg/backends?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope.
/assign @freehan |
Closing in favor of #590 |
Currently, pkg/backends uses a snapshotter which is used to keep some state on what backends that the controller managed. This snapshotter lists from the cloud periodically but this is unnecessary. We only need to list from the cloud when we are performing garbage collection.
This PR adds a new package called cloudlist which contains an implementation for a simple cloud lister that is decoupled from the existing snapshotter. The existing snapshotter implementation in pkg/backends is then replaced with the new cloud listing implementation.
The goal is to use the new cloud lister in pkg/loadbalancers and pkg/instances as well. Once this is done, the snapshotter code in pkg/pools can be completely removed.