I understand its breaking manila but don't you think client evict with invalid id needs to be handled, it seems like the command with invalid id is correct. Some user might again encounter this as a issue and we might have a ticket raised for this in future.

Are you referring to the issue about all clients getting evicted or the specific issue of handling invalid client-id? Commit 0ef5941 correctly fixes the client eviction part (where all clients were getting evicted), but the commit additionally changes the behaviour of returning -ESRCH on invalid client-id which breaks manila. Also, its good to have commands idempotent, so running the same command twice, which results in the same output is actually a good thing.

I understand its breaking manila but don't you think client evict with invalid id needs to be handled, it seems like the command with invalid id is correct. Some user might again encounter this as a issue and we might have a ticket raised for this in future.

Are you referring to the issue about all clients getting evicted or the specific issue of handling invalid client-id? Commit 0ef5941 correctly fixes the client eviction part (where all clients were getting evicted), but the commit additionally changes the behaviour of returning -ESRCH on invalid client-id which breaks manila. Also, its good to have commands idempotent, so running the same command twice, which results in the same output is actually a good thing.

The tracker issue was about eviction of all clients when wrong flags like -h were passed which got fixed. I am referring to the issue of handling invalid client id, which i tried to fix too. With this patch whether the id passed is valid or invalid , user doesn't get to know what happened...until and unless he checks the client ls output whether number of clients remain same or not, because we don't even get any succesfull message after client eviction. If checking client ls again and again after running command to determine the validness of id is fine, then this is good to go.

jenkins test make check

I understand its breaking manila but don't you think client evict with invalid id needs to be handled, it seems like the command with invalid id is correct. Some user might again encounter this as a issue and we might have a ticket raised for this in future.

Are you referring to the issue about all clients getting evicted or the specific issue of handling invalid client-id? Commit 0ef5941 correctly fixes the client eviction part (where all clients were getting evicted), but the commit additionally changes the behaviour of returning -ESRCH on invalid client-id which breaks manila. Also, its good to have commands idempotent, so running the same command twice, which results in the same output is actually a good thing.

The tracker issue was about eviction of all clients when wrong flags like -h were passed which got fixed. I am referring to the issue of handling invalid client id, which i tried to fix too. With this patch whether the id passed is valid or invalid , user doesn't get to know what happened...until and unless he checks the client ls output whether number of clients remain same or not, because we don't even get any succesfull message after client eviction. If checking client ls again and again after running command to determine the validness of id is fine, then this is good to go.

Manila has also designed their workflow based on session evict not returning an error for invalid id, so, we'll have to keep that bit as it is :/

I know its a bit awkward, but we need to be good citizens with our users and also my earlier point about idempotent operations.

This PR is under test in https://tracker.ceph.com/issues/68179.

jenkins test windows