Added support to disable the project size feature

[abujeda]

This changes disables the project size feature by setting the existing project_size_timeout property to 0

This changes disables the project size feature by setting a new Configuration.project_size_enabled property to false

Fixes #3530

[CSC-swesters]

Thanks for picking up our issue!

I feel like the suggested implementation with setting project_size_timeout: 0 or project_size_timeout: "" is a bit confusing. The background is that timeout(1) on RHEL 8 (timeout (GNU coreutils) 8.30) specifies that:

A duration of 0 disables the associated timeout.

I.e., it means an infinite time. I think it's confusing that setting project_size_timeout to 0 means the exact opposite of what the code I quoted in the issue would suggest:

Open3.capture3('timeout', "#{Configuration.project_size_timeout}s", 'du', '-s', #...

If you want to go down this route anyway, perhaps setting a negative value could make more sense, since it's "illegal" in the timeout(1) program:

$ timeout -1 echo "hello"
timeout: invalid option -- '1'
Try 'timeout --help' for more information.

$ timeout --kill-after=-1 echo "hello"
timeout: invalid time interval ‘-1’
Try 'timeout --help' for more information.

I'd personally prefer a separate configuration option, like calculate_project_size: <boolean>, to avoid complicating the meaning of existing configuration values.

It was also a bit confusing to me that the project_size_enabled check didn't happen in the size method, where the du call would happen.

ondemand/apps/dashboard/app/models/project.rb

Lines 188 to 193 in 1a35d7e

	def size
	if Dir.exist? project_dataroot
	o, e, s = Open3.capture3('timeout', "#{Configuration.project_size_timeout}s", 'du', '-s', '-b', project_dataroot.to_s)
	o.split('/')[0].to_i
	end
	end

It would be good to have a sanity check there that the parameter actually is a positive floating point number, to guard against future changes.

Let me know what you think!

[abujeda]

Thanks for the review, will make some improvements based on your comments.

[abujeda]

I have made some improvements based on the comments:

• I added a new boolean variable to Configuration: project_size_enabled to avoid confusion with project_size_timeout
• The project UI will not make a request for the project size if project_size_enabled is false.
• The JSON response will no longer get the project size if project_size_enabled is false.
• There is no need to add this check in the Project.size method.

Regarding the validation of the project_size_timeout, I will leave that to @johrstrom. We can raise a new issue if that requires improvements.

[johrstrom]

k8s tests are broken apparently. I'm fine with the changes, but it appears we need to fix that first.

[CSC-swesters]

Thank you! @abujeda

[abujeda]

It seems that some of the Ubuntu and Debian tests are broken now.

[abujeda]

Triggered a CI build and all tests are green

[johrstrom]

Works for me.