TrueCloudLab/frostfs-node
19
1
Fork 27
Code Issues 91 Pull requests 6 Projects Releases 1 Activity Actions

container/ape: Ignore an error when getting a role #1454

Merged
fyrchik merged 1 commit from a-savchuk/frostfs-node:container-get-on-first-epoch into master 2024-10-30 10:41:29 +00:00
Conversation 0 Commits 1 Files changed 1 +9 -12
1 changed files with 9 additions and 12 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files
Showing only changes of commit d28a5d2d7a - Show all commits

21
pkg/services/container/ape.go
View file

@ -537,10 +537,7 @@ func (ac *apeChecker) isContainerKey(pk []byte, cnrID cid.ID, cont *containercor
return false, err
}
in, err := isContainerNode(nm, pk, binCnrID, cont)
if err != nil {
return false, err
} else if in {
if isContainerNode(nm, pk, binCnrID, cont) {
return true, nil
}
@ -551,24 +548,24 @@ func (ac *apeChecker) isContainerKey(pk []byte, cnrID cid.ID, cont *containercor
return false, err
}
return isContainerNode(nm, pk, binCnrID, cont)
return isContainerNode(nm, pk, binCnrID, cont), nil
}
func isContainerNode(nm *netmapSDK.NetMap, pk, binCnrID []byte, cont *containercore.Container) (bool, error) {
cnrVectors, err := nm.ContainerNodes(cont.Value.PlacementPolicy(), binCnrID)
if err != nil {
return false, err
}
func isContainerNode(nm *netmapSDK.NetMap, pk, binCnrID []byte, cont *containercore.Container) bool {
// It could an error only if the network map doesn't have enough nodes to
// fulfil the policy. It's a logical error that doesn't affect an actor role
acid-ant commented 2024-10-29 15:31:14 +00:00
Outdated
Review
Copy link

Link to the issue looks redundant here, it will be in the commit message.

Link to the issue looks redundant here, it will be in the commit message.
a-savchuk commented 2024-10-30 09:49:40 +00:00
Outdated
Review
Copy link

Yeah, removed it✅

Yeah, removed it✅
// determining, so we ignore it
dstepanov-yadro marked this conversation as resolved Outdated
dstepanov-yadro commented 2024-10-29 15:18:32 +00:00
Outdated
Review
Copy link

Why is any error ignored, but not only specific one?

Why is any error ignored, but not only specific one?
a-savchuk commented 2024-10-30 09:43:34 +00:00
Outdated
Review
Copy link

Made the PR description more detailed:

The affected function isContainerNode is used for determining if an actor is a container role when checking an APE rule. It's guaranteed that the policy of a container is valid because that container has been with that policy before. So, the only error could be returned if the map doesn't have enough nodes. It's a logical error, we should ignore it because it doesn't affect an actor role determining.

Moreover, that error is not a part of the SDK interface, so if I ignore only that error, it'll look like this

if err != nil && !strings.Contains(err.Error(), "not enough nodes to SELECT from") {
    return false, err
}
Made the PR description more detailed: > The affected function `isContainerNode` is used for determining if an actor is a container role when checking an APE rule. It's guaranteed that the policy of a container is valid because that container has been with that policy before. So, the only error could be returned if the map doesn't have enough nodes. It's a logical error, we should ignore it because it doesn't affect an actor role determining. Moreover, that error is not a part of the SDK interface, so if I ignore only that error, it'll look like this ```go if err != nil && !strings.Contains(err.Error(), "not enough nodes to SELECT from") { return false, err } ```
cnrVectors, _ := nm.ContainerNodes(cont.Value.PlacementPolicy(), binCnrID)
for i := range cnrVectors {
for j := range cnrVectors[i] {
if bytes.Equal(cnrVectors[i][j].PublicKey(), pk) {
return true, nil
return true
}
}
}
return false, nil
return false
}
func (ac *apeChecker) namespaceByOwner(owner *refs.OwnerID) (string, error) {