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

Prevent unrestrained unblocking of workers #2103

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MinetaS
Copy link
Contributor

@MinetaS MinetaS commented Jul 3, 2024

This change prevents non-approver users from unblocking their workers when they are not blocked by themselves.

@MinetaS
Copy link
Contributor Author

MinetaS commented Jul 3, 2024

Note: I have no expertise in MongoDB/PyMongo hence this PR might contain several errors that I failed to notice.

At least this change requires a change of schema for workerdb.

This change prevents non-approver users from unblocking their workers
when they are not blocked by themselves.
@vdbergh
Copy link
Contributor

vdbergh commented Jul 3, 2024

One should note that any worker block can be trivially circumvented by changing the worker name, e.g. via deleting fishtest.cfg. So the worker blocking feature depends on cooperation from the user (who we assume has Fishtest's best interest in mind). I am not sure if making it stricter makes a lot of sense.

I don't really know what to do with a user like Sylvain27 that has two good workers and one bad one, and who keeps unblocking the bad one. The most natural thing would be to block the user, but that would cost some cores. I had another proposition here #2051 (comment) .

@MinetaS
Copy link
Contributor Author

MinetaS commented Jul 3, 2024

One should note that any worker block can be trivially circumvented by changing the worker name, e.g. via deleting fishtest.cfg.

I consider this to be malicious behavior towards the framework, and this is where user block should be stepped in.

Regarding the Sylvain27 case, the owner did not change the worker ID instead they unblocked the worker - which suggest that there is a chance the owner is likely not aware of the reasons why their workers are getting blocked. I thought it would be beneficial to inform users "to contact approvers" before they mindlessly unblock workers, as this would open a channel for communication between users and approvers. (which is important as the situation is due to the lack of the communication.)

@vdbergh
Copy link
Contributor

vdbergh commented Jul 3, 2024

Regarding the Sylvain27 case, the owner did not change the worker ID instead they unblocked the worker - which suggest that there is a chance the owner is likely not aware of the reasons why their workers are getting blocked.

Well the worker prints a message why the worker is being blocked. I assume the user has seen this message because it contains the instructions to unblock the worker.

Also if you unblock the worker you again see the message why it was blocked.

Moreover I also sent an email asking explicitly not to unblock the worker before fixing the time losses issue.

So I find it hard to believe that the user does not know why the worker is being blocked.

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

Successfully merging this pull request may close these issues.

2 participants