Change return type of AggregatedListNetworkEndpointGroup in cloudprovideradapter. #938
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
the
size/L
prameshj
changed the title
|
/assign @freehan This implements the review comment in |
pkg/neg/manager.go
Outdated
| @@ -255,17 +255,23 @@ func (manager *syncerManager) garbageCollectSyncer() { | |||
| func (manager *syncerManager) garbageCollectNEG() error { | |||
| // Retrieve aggregated NEG list from cloud | |||
| // Compare against svcPortMap and Remove unintended NEGs by best effort | |||
| zoneNEGList, err := manager.cloud.AggregatedListNetworkEndpointGroup(meta.VersionGA) | |||
| NEGList, err := manager.cloud.AggregatedListNetworkEndpointGroup(meta.VersionGA) | |||
pkg/neg/manager.go
Outdated
| } | ||
| negNames := map[string]string{} | ||
| for key, neg := range NEGList { | ||
| if key.Type() == meta.Global { |
There was a problem hiding this comment.
Do this instead?
if key.Type() != meta.Zonal
pkg/neg/manager.go
Outdated
| if manager.namer.IsNEG(neg.Name) { | ||
| negNames.Insert(neg.Name) | ||
| } | ||
| negNames := map[string]string{} |
There was a problem hiding this comment.
I think negName->zone is not enough.
there may be multiple NEGs with the same name in multiple zones.
// negName -> zones
deleteCandidates := map[string][]string
There was a problem hiding this comment.
Good point, added this and a test case.
pkg/neg/manager.go
Outdated
| continue | ||
| } | ||
| if manager.namer.IsNEG(neg.Name) { | ||
| negNames[neg.Name] = key.Zone |
There was a problem hiding this comment.
if _, ok := deleteCandidates[key.Name]; !ok {
deleteCandidates[key.Name] = []string{}
}
deleteCandidates[key.Name] = deleteCandidates[key.Name].append(key.Zone)
| for _, neg := range list { | ||
| if manager.namer.IsNEG(neg.Name) { | ||
| negNames.Insert(neg.Name) | ||
| deleteCandidates := map[string][]string{} |
There was a problem hiding this comment.
nit: add a comment
// deleteCandidates is a map from neg name to list of zones it is in.
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: freehan, prameshj 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 |
k8s-ci-robot
added
the
approved
No description provided.