Add support to clean Np indexers based on account #262
Conversation
Anandkumar26
commented
Jun 26, 2023
•
Anandkumar26
commented
- When CPA is deleted, it will notify a local event to np controller.
- As part of processing local event, np controller will clear entries corresponding to the account from appliedToSGIndexer, addrSGIndexer, cloudRuleIndexer and networkPolicyIndexer.
- Remove AT/AG groups from retryQueue for the account.
- Remove groups from pendingDeleteGroups based on account.
Anandkumar26
force-pushed
the
account-delete
branch
3 times, most recently
from
baaca9d to
c8f1255
Compare
|
|
||
| case *crdv1alpha1.CloudProviderAccount: | ||
| cpa := event.Object.(*crdv1alpha1.CloudProviderAccount) | ||
| return r.RemoveIndexerObjectsByAccount(types.NamespacedName{Name: cpa.Name, Namespace: cpa.Namespace}.String()) |
Anandkumar26
force-pushed
the
account-delete
branch
4 times, most recently
from
19e54f5 to
e791440
Compare
| } | ||
| for i := range atSgs { | ||
| if atSgs[i] == nil { | ||
| continue |
There was a problem hiding this comment.
Why would this be nil?
There was a problem hiding this comment.
While testing out namespace deletion, I observed a crash saying interface is nil.
I observed it while traversing agSgs.
There was a problem hiding this comment.
When we retrieve objects using by Index, the length of the objects returned can be non-zero, yet while accessing the obj(interface), the obj(interface) can be nil.
I tried this by deleting an object in the indexer and got all the objects using Byindex immediately.
one of the object returned was nil.
So while retrieving objects by Index, we should check if the interface can be converted to a required object and then proceed.
Anandkumar26
force-pushed
the
account-delete
branch
3 times, most recently
from
850b245 to
26a5a3e
Compare
| // Remove AG group from retryQueue and pendingDeleteGroups. | ||
| uName := getGroupUniqueName(agSg.id.CloudResourceID.String(), true) | ||
| uGroupName := getGroupUniqueName(agSg.id.Name, true) | ||
| r.retryQueue.Remove(uName) |
There was a problem hiding this comment.
This wont work. In case of delete, we remove the item from indexer first, and then put in retry queue. So, you wont find the item for lookup in retry queue. You might have to iterate over items in the queue and delete all matching account-id.
Anandkumar26
force-pushed
the
account-delete
branch
2 times, most recently
from
74e17c7 to
5a046db
Compare
| } | ||
|
|
||
| // Remove AT and AG group from retryQueue. | ||
| for _, item := range r.retryQueue.items { |
There was a problem hiding this comment.
What about pending queue.. I see you are removing from retryQueue only.
There was a problem hiding this comment.
In PendingGroup, we only store the GroupMember, so we wont be able to delete an entry based on account.
Anandkumar26
force-pushed
the
account-delete
branch
from
5a046db to
5cb2c92
Compare
- When CPA is deleted, it will notify a local event to np controller. - As part of processing local event, np controller will clear entries corresponding to the account from appliedToSGIndexer, addrSGIndexer, cloudRuleIndexer and networkPolicyIndexer. - Remove AT/AG group from retryQueue based on account - Remove groups from pendingDeleteGroups based on account Signed-off-by: Anand Kumar <[email protected]>
Anandkumar26
force-pushed
the
account-delete
branch
from
5cb2c92 to
2331c0d
Compare
|
|
||
| for _, item := range r.pendingDeleteGroups.items { | ||
| group, ok := item.PendingItem.(*pendingGroup) | ||
| if ok && group.account == namespacedName { |
There was a problem hiding this comment.
nit: Rename to accountID for next PR, to be consistent with Retry Queue
|
/nephe-test-e2e-agentless |
|
/nephe-test-e2e-aws |
