Rockons: Ability to set environment objects as optional

[Hooverdan96]

this is out of the discussion in this thread:

https://forum.rockstor.com/t/default-values-for-environment-variables-in-rockon-definition/6512

Currently environment variables are required, i.e. if they are defined as part of the json file, then during the installation/configuration of the RockOn, they have to be populated. Especially for Rockons based on docker images that use environment variables to pass specific options to the API of the image application, this would be useful.
@FroggyFlox already has mentioned an idea on how to represent this in the json file:
simple key:value under the specific environment object to set an "optional": true/false flag.
Behavior when the json file is ingested:
If set to true, the given environment variable can be left blank during the rock-on install wizard. And if it's left blank it will not be part of the docker run command that is generated to launch the RockOn.
In order to keep backwards compatibility, if this key:value pair doesn't exist (or is set to false), the original behavior of requiring to maintain the environment object during the install wizard.

[Hooverdan96]

This issue should be considered in conjunction with the issue #2012 (add env default option for rockons) for defining default values for the environment object.

[FroggyFlox]

Adding another forum thread requesting this feature:
https://forum.rockstor.com/t/optional-environment-variables-in-rock-ons/8756

[Hooverdan96]

While looking at this again, wanted to reported my observations, ideas:

Possibly add another field to the model in rockon.py to highlight that the environment variable is optional. Default would be false, so if it's not coming from the json file, then the downstream logic would not be affected (i.e. it's required):

class DContainerEnv(models.Model):
    container = models.ForeignKey(DContainer)
    key = models.CharField(max_length=1024)
    val = models.CharField(max_length=1024, null=True)
    description = models.CharField(max_length=2048, null=True)
    label = models.CharField(max_length=64, null=True)
    optional = models.BooleanField(default=false)

In the cc_form.jst this could then conditionally change the display from required (*) to optional:

	<div class="messages"></div>
	{{#each cc}}
        <div class="form-group">
		    {{#if this.optional}}
			    <label class="control-label col-sm-3" for="cc">{{this.label}}<span class="optional"></span></label>
		    {{else}}
			    <label class="control-label col-sm-3" for="cc">{{this.label}}<span class="required">*</span></label>
		    {{/if}}
        <div class="controls">
        <div class="col-sm-6">

Of course, since that form is shared with the custom control (for OwnCloud, etc.), would have to see whether that will be a conflict.

Finally, I don't know (since I am not a js or handlebars expert) how the validation needs to change to differentiate between optional and required fields in the rockons.js . Does that require a new helper?

rockstor-core/src/rockstor/storageadmin/static/storageadmin/js/views/rockons.js

Lines 774 to 785 in 3ad7eee

	RockonEnvironment = RockonCustomChoice.extend({
	initialize: function() {
	RockonCustomChoice.prototype.initialize.apply(this, arguments);
	this.custom_config = this.model.get('environment');
	},
	save: function() {
	if (!this.cc_form.valid()) {
	this.validator.showErrors();
	return $.Deferred().reject();
	}
	var env_map = {};

Also, if optional environment variables are not filled in (i.e. not request/used) those entries would not be saved into the database, or would they?

[phillxnet]

@Hooverdan96 Great to see some progress on this one by the way.
Re:

Also, if optional environment variables are not filled in (i.e. not request/used) those entries would not be saved into the database, or would they?

Each db record must be consistent so it's constituent fields must contain something, default null if null=True for example. Pretty sure I've not picked up your point here however. Also js is not my favourite and I really don't like the kind of code that almost always seems to be associated with it (that would be a digression) :) .

I think @FroggyFlox may be of more help on this one, although he is busy elsewhere it seems.

I think ultimately the proof is in the pudding as it were. Go ahead and implement your idea in a draft pr and take a look at exactly what actually happens in the database. That is the true of the matter and will likely uncover other knock-on effects that are not always easy to predict (at least for me).

I'm really trying to avoid 'hard' db field additions currently (i.e. not just properties) as we are so close the next stable release, but given #2529 there may soon be the option to pop this into the next testing channel where a db addition is more than welcome.

[FroggyFlox]

Very briefly as I can't remember the details exactly and I can't look at it right now but:

If I remember correctly, the requirement we have is purely on the frontend side (js validation) as I think we have all of those fields set to null=True in their respective models.
I would have a look at the DContainerDevice model and how we parse the respective object in the JSON: devices in the Rock-On installation wizard are optional, and when I implemented those, I basically took example on the environment object so it should be very close.

Sorry again for the excessive briefness here; I'll try to have a better look some time later.

[Hooverdan96]

Thanks both. @phillxnet I was thinking about the saving in the object, and how it would look like in the installation summary page (i.e. all non-maintained optional fields would also show up, or only the ones for which values had been entered) - but @FroggyFlox comment brings back that when one doesn't maintain a device entry, it will also show up in the summary page. So, we would still see a long list of environment entries (like in the Unpackerr Rockon example from another thread), but they would show empty values. Which, I think is fine, especially if it doesn't require major changes to the way entries are stored, etc.

As for the js piece, I think if for this topic we used a different approach - reducing reliance on js further - then we would want to eventually propagate this for all objects, too (if feasible), which is probably a "slightly" bigger undertaking (and definitely only after stable release). So, it might be easier to follow a similar approach as for the other objects that relies on some js logic interaction to be consistent, and then contemplate a wholesale move to another approach later?

As for the adding of new fields to the database, I didn't expect that this type of a change would make it in before the stable release to avoid another variable that could further delay it.

If I have time, I might try a PoC, since, as you said, the proof will be in the pudding :). But certainly interested if @FroggyFlox has some more insights to consider (though, no rush, I know you're busy on many fronts, of which Rockstor is only one).

[kanecko]

Thank you all for the discussion and the insights it has offered.

I am working on a PoC as we speak:

I have added "optional":bool and "default_value":string into the JSON and to the DContainerEnv. Additionally, I will add another boolean to the model in order to discern between a blank value vs a NULL value (=do not define an env var).
In order to recognize when the user wants to use a blank value vs. a NULL value in a optional field, I will now proceed to add a checkbox to the form. The checkbox will disable the text field when checked. I think it would be best to save user's "val" value, in case the user wants to use it again in the future.

[Hooverdan96]

Good stuff.
On this:

I think it would be best to save user's "val" value, in case the user wants to use it again in the future.

Do you mean here, that if they entered a value in an optional field and then subsequently decide not to use it, they can essentially disable the usage of that value by setting the check mark?

• The system would save whatever is in that field, but only use it if the checkbox has not been set.
• This would then allow in the future during a "reinstall" to use the value and leave the checkbox unchecked.
• One thing to remember is that we can currently only make "changes" after uninstalling and reinstalling, however a reinstall is facilitated by the fact that the wizard remembers the values from the previous installation.

[kanecko]

That's correct.
The user value would (I presume) only first be saved to DB after a successful install. No matter if the checkbox is checked or not.

The checkbox itself is represented by its own boolean in the model.

If a default value has been defined in the JSON, then the default value will be shown. User value obviously replaces/overrides the default value.
The default value is used only in the html attribute placeholder.

Do I need to save the default value to DB, or should I just read it from the JSON? I could still pass it to Handlebars, if it isn't in the model, right?

[Hooverdan96]

@FroggyFlox probably has to answer that, since he's intimately familiar (and designer of the most recent architectural changes) of that.

[FroggyFlox]

@kanecko, awesome to see you working on this, thank you!
And thank you, @Hooverdan96 for guiding this as well.

At first look, I think you may not even need to add anything to the model, but just how Rockstor parses and deals with what the is given to it. The docker devices are optional that wat, for instance.

I'm unfortunately unable to have a proper look at this right now but will try to give a better answer with examples as soon as possible.

Thank you so much!

[kanecko]

I have imagined such a UI change. Comments are welcome. :)

[Hooverdan96]

Thanks for the visual.

• When the field has only the asterisk, it's a required field (as before, so no change)
• When the check mark is set, then the field's default value (originally derived from the json that was loaded for that particular Rockon; it could be blank or some actual default value) will be used?
• When the check mark is not set, then the field will become editable and the user can add a value?
• Not sure that's a valid scenario in any docker container setup: there is a default value that comes with the Rockon's json definition, the user leaves the parameter unchecked and also decides no value is needed (because, say, options are in this case that a default value exists, but is still an optional entry, would one just blank out the field and the subsequently build docker run command would not contain that parameter?

Would that be the planned behavior?

Thinking about the use cases in my mind:
Use Cases:
Required Parameter

• standard already established the behavior/requirement

Optional Parameter:

• with default value - user decides to use it
• with default value - user decides not to use the parameter at all
• with default value - user decides not to use default, but maintain their own value
• without default value - user decides to use it, enters a value (assumed valid, otherwise install will fail anyway, current behavior for required parameters
• without default value - user decides not to use the parameter at all

[kanecko]

When the checkbox is checked, the variable will not be defined at the run command.
The greyed out value has no function in this use case.

Maybe it's better UX if I replace the text field with a "variable will not be defined" instead?
I'll try out a few different ways.

Another thing to consider is the "index" attribute in JSON and how should it play together with optional/required fields.

When no index attr is defined on any of the env vars: place required vars at the top, optional below the required.
When an index attr is defined on some of the env vars:??? Do the same placement as above, but handle the individual index attrs seperately?
When an index attr is defined in all vars: take the index vals

[kanecko]

I've tried using a switch:

A little bit too wide for my taste, but at least it conveys more meaning than just a checkbox.
Maybe a single normal button would be enough, since we are disabling the text form in the case of forgoing an optional variable. I will try that next.

I am having trouble finding the right words to put on the button. So any help here is appreciated. But it can wait until the code-review stage.

[Hooverdan96]

Inactive/Active maybe? single button might be nicer, as you've said, needing less space ... Or stick with the checkbox but have it as a column with the header of "Inactive"? All required items would not have one obviously....

@FroggyFlox if you do get a minute to chime in, since you also had a vision on how this can be communicated/handled.

[FroggyFlox]

@kanecko , @Hooverdan96 , thank you so much for all your work there, that's awesome!

I indeed would like to chime in here but I unfortunately do not have the time at this very moment to clearly communicate what I have in mind; I'll do my best to take the time to do so shortly, however. My sincere apologies for that :-(

[kanecko]

The code changes of the prototype so far: https://github.com/kanecko/rockstor-core
EDIT: progress: first successful installation