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

neofs-cli container set-eacl hangs without error report #1652

Closed
aprasolova opened this issue Aug 3, 2022 · 10 comments
Closed

neofs-cli container set-eacl hangs without error report #1652

aprasolova opened this issue Aug 3, 2022 · 10 comments

Comments

@aprasolova
Copy link
Contributor

Sometimes when I run neofs-cli container set-eacl --await ... it hangs for 30 seconds, then fails without any error message neither in console nor in SN log. CLI exit code is 0.

$ neofs-cli --rpc-endpoint s01.neofs.devenv:8080 --wallet wallet.json container set-eacl \
    --cid TitkoZ6znKvAEpxSMevWeayo8YJVGzeFKXYMdV1ux4T \
    --table eacl_table.json --config empty_passwd.yml --await 	
awaiting...

The command was run at 18:51:50.459. These log messages related to eACL setting I have found in the s01.neofs.devenv log

2022-08-03T15:51:50.896Z	debug	client/notary.go:522	notary request invoked	{"method": "setEACL", "valid_until_block": 800, "fallback_valid_for": 40, "tx_hash": "a08d4fc58f4b8af831ea02fb324a72d560b9c1b3f711d840e0dce1fb2d272f3e"}
2022-08-03T15:51:51.522Z	debug	event/listener.go:319	event parser not set	{"script hash LE": "4da19a534afb5578d7739f688733ab8289215507", "event type": "SetEACLSuccess"}
2022-08-03T15:52:20.916Z	debug	client/notary.go:522	notary request invoked	{"method": "setEACL", "valid_until_block": 800, "fallback_valid_for": 40, "tx_hash": "48584209947be355a4ca961b3271c306817e4544ea91042f3fabde09254aa5bc"}
2022-08-03T15:52:21.579Z	debug	event/listener.go:319	event parser not set	{"script hash LE": "4da19a534afb5578d7739f688733ab8289215507", "event type": "SetEACLSuccess"}

When I requested two transactions from log from morph-chain.neofs.devenv, got no response

$ curl -XPOST -d@data http://morph-chain.neofs.devenv:30333
{"id":1,"jsonrpc":"2.0","error":{"code":-100,"message":"Unknown transaction"}}
$ cat data
{
  "jsonrpc": "2.0",
  "id": 1,
  "method": "getrawtransaction",
  "params": ["0x48584209947be355a4ca961b3271c306817e4544ea91042f3fabde09254aa5bc"]
}

Your Environment

  • neofs-dev-env 374724f04f4de8bdd494ac9d6a9319071b20406a
  • neofs-cli v0.30.2
@carpawell
Copy link
Member

@aprasolova, were you able to get that eACL via get-eacl? Logs say that the chain responded (via the notification) that the contract has saved a table.

@cthulhu-rider
Copy link
Contributor

cthulhu-rider commented Aug 11, 2022

We actually need to see inner ring logs within this issue. It'd be also useful to see how container is formed: is ACL extension enabled?

That's what I got with non-extendable ACL:

$ neofs-cli --rpc-endpoint s01.neofs.devenv:8080 --wallet <WALLET> container set-eacl --cid <CID> --table <TABLE> --await
Enter password > 
awaiting...
timeout: EACL has not been persisted on sidechain
$ echo $?
1

@cthulhu-rider cthulhu-rider self-assigned this Aug 11, 2022
@aprasolova
Copy link
Contributor Author

You might run any robot test from the directory, the issue reproduces pretty often.

By the way, what response recieves a user when they try to set eACL upon the container whose ACL cannot be extended? I'd propose to print exact error message, not timeout error.

@carpawell
Copy link
Member

carpawell commented Aug 15, 2022

what response recieves a user when they try to set eACL upon the container whose ACL cannot be extended?

No response is expected now since the contract does not check that, all the validations (except non-existing container) are performed on the IR side so that TX is considered incorrect and not going to be added to the chain -> timeout on the CLI side.

@aprasolova
Copy link
Contributor Author

It doesn't look human-friendly, does it?
Is it possible to check the ACL extension possibility, for instance, by the IR side?

@carpawell
Copy link
Member

It doesn't look human-friendly, does it?

Maybe, but that is how setting eACL works (CLI -> SN ---*async*---> IR -> Morph). But we can check if container support extending via another call in CLI. @cthulhu-rider, @fyrchik

Is it possible to check the ACL extension possibility, for instance, by the IR side?

Yes, IR is the only app that validates setting eACL and that is why CLI should just wait for TX to be persisted on the Morph since neither CLI nor the SN communicate to the IR directly only via the Morph.

@cthulhu-rider
Copy link
Contributor

Is it possible to check the ACL extension possibility

@aprasolova we can check the extensibility inside the command itself by reading the container from the network (as we all remember extensibility is an immutable prop same as all others in the container). Pros: we can also cover the cases of non-existent containers.

So, I propose to support --check flag makes command to modify eACLs only in existent extendable containers. Later we can consider this behavior as 99%-useful and make it default.

cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue Aug 17, 2022
…acl`

Container ACL in NeoFS can be extended only for container in which the
corresponding option is enabled. In previous implementation command
`set-eacl` could hang up on modifying eACL of the non-existent or
non-extendable container. To improve UX, there is a need to pre-check
the availability of `SETEACL` operation.

Add boolean `precheck` flag to `set-eacl` cmd which reads the container
before the actual transaction formation. If flag is set, command fails
on non-extendable container ACL.

Signed-off-by: Leonard Lyubich <leonard@nspcc.ru>
cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue Aug 17, 2022
… struct

Signed-off-by: Leonard Lyubich <leonard@nspcc.ru>
cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue Aug 17, 2022
…acl`

Container ACL in NeoFS can be extended only for container in which the
corresponding option is enabled. In previous implementation command
`set-eacl` could hang up on modifying eACL of the non-existent or
non-extendable container. To improve UX, there is a need to pre-check
the availability of `SETEACL` operation.

Add boolean `precheck` flag to `set-eacl` cmd which reads the container
before the actual transaction formation. If flag is set, command fails
on non-extendable container ACL.

Signed-off-by: Leonard Lyubich <leonard@nspcc.ru>
cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue Aug 17, 2022
… struct

Signed-off-by: Leonard Lyubich <leonard@nspcc.ru>
@cthulhu-rider
Copy link
Contributor

You might run any robot test from the directory, the issue reproduces pretty often.

@aprasolova I couldn't reproduce it even once.

@aprasolova
Copy link
Contributor Author

Please look at the robot test runs for one of recent pull requests. It reproduces multiple times

cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue Sep 2, 2022
…ommand

Flag `--pre-check` of `set-eacl` command found to be in demand in most
cases. based on this, it makes sense to add its action to the default
behavior.

Pre-check container extensibility by default. Rename flag to
`--no-precheck` and invert its action.

Signed-off-by: Leonard Lyubich <leonard@nspcc.ru>
cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue Sep 2, 2022
…acl`

Container ACL in NeoFS can be extended only for container in which the
corresponding option is enabled. In previous implementation command
`set-eacl` could hang up on modifying eACL of the non-existent or
non-extendable container. To improve UX, there is a need to pre-check
the availability of `SETEACL` operation.

Add boolean `precheck` flag to `set-eacl` cmd which reads the container
before the actual transaction formation. If flag is set, command fails
on non-extendable container ACL.

Signed-off-by: Leonard Lyubich <leonard@nspcc.ru>
cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue Sep 2, 2022
… struct

Signed-off-by: Leonard Lyubich <leonard@nspcc.ru>
cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue Sep 2, 2022
…ommand

Flag `--pre-check` of `set-eacl` command found to be in demand in most
cases. based on this, it makes sense to add its action to the default
behavior.

Pre-check container extensibility by default. Rename flag to
`--no-precheck` and invert its action.

Signed-off-by: Leonard Lyubich <leonard@nspcc.ru>
cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue Sep 2, 2022
Signed-off-by: Leonard Lyubich <leonard@nspcc.ru>
cthulhu-rider pushed a commit that referenced this issue Sep 2, 2022
Container ACL in NeoFS can be extended only for container in which the
corresponding option is enabled. In previous implementation command
`set-eacl` could hang up on modifying eACL of the non-existent or
non-extendable container. To improve UX, there is a need to pre-check
the availability of `SETEACL` operation.

Add boolean `precheck` flag to `set-eacl` cmd which reads the container
before the actual transaction formation. If flag is set, command fails
on non-extendable container ACL.

Signed-off-by: Leonard Lyubich <leonard@nspcc.ru>
cthulhu-rider pushed a commit that referenced this issue Sep 2, 2022
Signed-off-by: Leonard Lyubich <leonard@nspcc.ru>
cthulhu-rider pushed a commit that referenced this issue Sep 2, 2022
Flag `--pre-check` of `set-eacl` command found to be in demand in most
cases. based on this, it makes sense to add its action to the default
behavior.

Pre-check container extensibility by default. Rename flag to
`--no-precheck` and invert its action.

Signed-off-by: Leonard Lyubich <leonard@nspcc.ru>
cthulhu-rider pushed a commit that referenced this issue Sep 2, 2022
Signed-off-by: Leonard Lyubich <leonard@nspcc.ru>
aprasolova pushed a commit to aprasolova/neofs-node that referenced this issue Oct 19, 2022
…acl`

Container ACL in NeoFS can be extended only for container in which the
corresponding option is enabled. In previous implementation command
`set-eacl` could hang up on modifying eACL of the non-existent or
non-extendable container. To improve UX, there is a need to pre-check
the availability of `SETEACL` operation.

Add boolean `precheck` flag to `set-eacl` cmd which reads the container
before the actual transaction formation. If flag is set, command fails
on non-extendable container ACL.

Signed-off-by: Leonard Lyubich <leonard@nspcc.ru>
aprasolova pushed a commit to aprasolova/neofs-node that referenced this issue Oct 19, 2022
… struct

Signed-off-by: Leonard Lyubich <leonard@nspcc.ru>
aprasolova pushed a commit to aprasolova/neofs-node that referenced this issue Oct 19, 2022
…ommand

Flag `--pre-check` of `set-eacl` command found to be in demand in most
cases. based on this, it makes sense to add its action to the default
behavior.

Pre-check container extensibility by default. Rename flag to
`--no-precheck` and invert its action.

Signed-off-by: Leonard Lyubich <leonard@nspcc.ru>
aprasolova pushed a commit to aprasolova/neofs-node that referenced this issue Oct 19, 2022
Signed-off-by: Leonard Lyubich <leonard@nspcc.ru>
@roman-khimov
Copy link
Member

Doesn't reproduce currently, we have testcases running regularly with similar functionality.

@roman-khimov roman-khimov closed this as not planned Won't fix, can't repro, duplicate, stale Dec 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants