Handbrake Rockon Required Parameters Seem Wrong

Rock-ons
1

I’m trying to install the Handbrake Rockon, and the required parameters during installation seem nonsensical and don’t match what the Docker image’s github describes.

First, the requirement for 4 shares that Rockstor labels as Config, Movies, Output and Watch. Looking at the github documentation, Movies doesn’t exist. The example the author gives is a volume where the static docker files are placed and under which HandBrake/Output and HandBrake/Watch live. It’s described as “contains files from your host that need to be accessible to the application.” That sounds nothing like a volume that would used for Movies. Seems to me like it should be stuff that gets stored in the Rockon root.

Second, there’s 2 fields labeled “Automated Conversion Preset” and “Automated Conversion Format”, and they’re both listed as requirements rather than optional. The info for those fields refers us to the image’s github page for more info, and specifies a section of the documentation. But there is no such information about possible presets at that location. Those fields are not required at all. The info for the field even says “defaults to […]” because the docker image assumes defaults if those aren’t provided. I’m not sure why those fields are mandatory for installation, it’s not even clear what we can enter there even after reading the referred documentation. I ended up just typing in what the field info shows.

Third, I see nowhere that I can pass a DVD drive from Rockstor to the docker image. I can pass an encoding device, but no optical drive. I assume I can add that as a label, but it seems odd that it’s not part of the installation procedure.

1 Like
2

@GoreMaker Thanks for the detailed report.

The most likely reason for this is that the upstream maintainer has modified their arrangements.

As you say the docker-hub reference for the image we currently use:

https://hub.docker.com/r/jlesage/handbrake

is way different from our mappings.

So what is likely required here is a pull request to modify our Rock-on definition accordingly. Either by using a different image (ideally one less prone to breaking changes), or by adapting our options to match the new arrangements. This forms part of our on-gong maintenance of the Rock-ons repository:

See the following doc section regarding Rock-on contribution/updating:

https://rockstor.com/docs/contribute/contribute_rockons.html#contributerockons

I’ve created the following issue to address this maintenance need, along with some historic changes to this Rock-on:

Our Rock-ons are where community contirbution is easiest and also most vital. With the large number of upstream ‘shifting sands’ we depend on reports such as this and community contributions by way of maintenance pull requests. If a Rock-on fall too far out of maintenance we simply delete as it is obviously not of enough community interest to receive the required maintenance.

Hope that helps, and again thanks for detailed feedback on this Rock-on.

@Hooverdan When you get a moment could you double check my above issue and change/close as you see fit of course. I just wanted to get @GoreMaker feedback to the issue stage. But I have not looked closely enough at what may have happened there. The issue summarises via links to that Rock-ons history of major changes.

2 Likes
3

Thank you for that explanation, makes all the sense. Based on more recent howtos for that docker image, it seems they basically require Config and Storage volumes, and the Watch and Output volumes are optional (they could just exist as part of the main Storage volume rather than being separate). You’re right, things evolve and it’s impossible to keep track of all the changes that happen upstream, especially for something as fluid as docker images. But I don’t think there are any better options for that particular functionality at this time.

3 Likes
4

@phillxnet thanks for opening the github issue. Good summary of the evolution.

@GoreMaker, the one functionality that we want to implement is the ability to define optional parameters, that don’t have to be populated. However, at this time, with the refactoring of the entire Rockstor platform to newer versions of pretty much everything, capacity has been lacking to do so.

To your points:

The Movies location shown during the configuration:

image
image963×346 6.4 KB

does actually refer to the /storage mapping, mentioned here:

We can at least change the label to Storage and use the description from github to more clearly have the reference. Again, currently can’t be made optional.

the /watch and /output mapping was proposed in an earlier forum post, as it would fulfill the need of that user. Again, this would be a candidate for an optional mapping, but since we don’t have that as of yet, it still remains mandatory.

The AUTOMATED_CONVERSION_PRESET and AUTOMATED_CONVERSION_FORMAT actually do still exist as parameters that can be passed. Unfortunately, also something that can’t be made optional at this time, and also not prepopulate (so, if one doesn’t want to change the defaults, they currently, unfortunately, have to be typed in, which you did).

As for the DVD device missing, that’s a good point. I just think “back then” it was not requested as one of the fields to be exposed. I believe, we can remedy that one fairly easily in a new PR, updating the current Rockon.

I’d prefer not to remove the working options at this time (if they still work), as we likely have some users of those, and since they are still available to be used in the image, but we can certainly work on updating and adding the other fields (i.e. storage label and add the DVD as a device that can be added).

Of course, the ultimate goal is to adjust the Rockon framework to allow for optional parameters (since we have a couple of other Rockons waiting in the wings for that functionality).

3 Likes
5

@GoreMaker, I have created a draft PR for the updates on the Rockon. If you’re game for testing, you could copy the json contents from the PR into a local file and temporarily install it on your instance to see whether it works (following the help on how to install your “own” Rockon). I have to rebuild one of my test VMs and won’t get to that until tomorrow or so.

You can find it here:

Ideally, it has no syntax errors, but you never know … it won’t conflict with the existing Rockon, as I’ve added a “test” to the Rockon name.

3 Likes
6

It installs and runs fine, and makes more sense than it did before :grinning: Being able to leave some volumes optional would definitely be a big improvement in the future once that’s supported. I noticed there’s a lot of leftover html tags visible in the info for the optical drive field.

Thank you for the quick action! I also learned a fair bit about creating and using custom Rockons.

Side note: the documentation states that custom Rockon json files should be stored in /opt/rockstor/rockons-metastore. The documentation at Rock-ons (Docker Plugins) — Rockstor documentation is written in such a way that it implies that directory should already exist, but it did not on my server (version 5.0.8 on Leap 15.5). Creating the directory and putting that json file in there worked fine, but it would be nice if either the documentation was updated to reflect that the directory needs to be created, or even better if the directory was there to begin with.

3 Likes
7

Thanks for the feedback. I will clean it up some more once I have my instance back. I think the point of not having the directory is to follow the approach to not have anything superfluous created until/unless it’s needed. The expectation has been that most users would not create custom Rockons. But I see your point, too. Maybe we can do something with the documentation to make it more clear.

2 Likes
8

@Hooverdan & @GoreMaker Just a note that @Hooverdan’s PR is now merged and in production.

Thanks to both of you for all your efforts here. Always nice to get some Rock-on maintenance/updates.

We could really do with more folks chipping in with well formed contributions such as these.

I’ve detailed in the review what I looked at: and it was only a basic review. But it’s now out-there and we can always revise as and when there is interest.

Hope that helps.

3 Likes