Minimum bookable free mcp

[ramonfigueiredo]

Prevent booking frames on hosts with full MCP

Change the host hardwareState from REPAIR to UP, when the amount of free space in the /mcp directory is greater or equals to the minimum required and delete the comment associated with the host. Check if the host with REPAIR status has comments with subject=SUBJECT_COMMENT_FULL_MCP_DIR and user=CUEBOT_COMMENT_USER and delete the comments. If the comment exists, change the host hardwareState from REPAIR to UP.

[DiegoTavares]

@bcipriano as I was part of the development of this feature, I think I'm biased to review it.

[bcipriano]

It's my understanding that for most users, the thing we call "MCP" will not actually be the /mcp directory:

OpenCue/rqd/rqd/rqmachine.py

Lines 519 to 523 in 6703e07

	def getTempPath(self):
	"""Returns the correct mcp path for the given machine"""
	if os.path.isdir("/mcp/"):
	return "/mcp/"
	return '%s/' % tempfile.gettempdir()

Ideally we would update all of OpenCue to use more fitting terminology, but that's a big project and not something we need to do right now.

However, in things like user-readable strings, and code comments, is there a different term we can use? This line is one example but there are a few others in this PR.

The current string would seem to be a bit misleading, if users have RQD hosts that aren't actually using the literal /mcp directory.

[ramonfigueiredo]

Hi @bcipriano

I propose we use the term TempDir to replace Mcp. What do you think?

I will replace all the Mcp by TempDir in my pull resquest.

However, as you said, we can create a new task later on to update all of OpenCue code to fit the terminology.

[ramonfigueiredo]

Done!

I changed the comments, variables and methods reference from "mcp" to "temporary directory" or "TempDir".

See commit: 04ec106

[bcipriano]

nit: can we call env.getRequiredProperty("dispatcher.min_bookable_free_mcp_kb", Long.class) to fetch this value instead of passing it as a function parameter?

Fetching properties should be cheap.

[ramonfigueiredo]

Hi Brian.

I updated the code following you recommendation. Now the solution is calling env.getRequiredProperty("dispatcher.min_bookable_free_mcp_kb", Long.class) twice. Inside the changeHardwareState() method and inside handleHostReport() method.

Please note that currently the only place the changeHardwareState() method is called is in the handleHostReport() method. My previous implementation did only one call of env.getRequiredProperty("dispatcher.min_bookable_free_mcp_kb", Long.class) because the minBookableFreeMCP variable was used later on in the handleHostReport() method to log the message of minimum amount of free space.

• Use of minBookableFreeMCP latter in the handleHostReport() method:

if (minBookableFreeMCP != -1 && report.getHost().getFreeMcp() < minBookableFreeMCP) {
                msg = String.format("%s doens't have enough free space in the /mcp directory, %dMB needs %dMB",
                        host.name, (report.getHost().getFreeMcp()/1024),  (minBookableFreeMCP/1024));
            }

However, I understand your point of view that the changeHardwareState method should know the min_bookable_free_mcp_kb value and that etching properties should be cheap.

[bcipriano]

Change LGTM. Thank you for the detailed tests!

Let's just do a merge from master so we get some fresh test runs.

[ramonfigueiredo]

Hi @bcipriano,

I need write access to merge the code. Can you give me write access?

I am seeing the message below and I can't see the merge button.
Only those with write access to this repository can merge pull requests.

[bcipriano]

Sorry to clarify, I meant you should update this PR's branch by pulling in the latest changes from master. Once you push the updated branch it will trigger the tests to run again.