Skip to content

Conversation

@jnoss
Copy link
Contributor

@jnoss jnoss commented Dec 8, 2016

...should be a space separated list

This fixes an issue where modifying safe_dirs results in an error from the provider:

Error: /Stage[main]/Main/Onedatastore[cephds]: Could not evaluate: Execution of '/usr/bin/onedatastore update cephds /tmp/onedatastore-cephds20161208-13878-9oyenf --append' returned 255: [DatastoreUpdateTemplate] Cannot update template. Parse error: syntax error, unexpected VARIABLE, expecting EQUAL or EQUAL_EMPTY at line 6, columns 90:94

.. which is caused by this line in the nebula datastore template:

SAFE_DIRS = "["/tmp/dir1 /tmp/dir1"]"

should be

SAFE_DIRS = "/tmp/dir1 /tmp/dir2"

This is fixed by making safe_dirs no longer an array, and specifying as a space separated list (similar to ceph_host and bridge_list)

...should be a space separated list
@tuxmea
Copy link
Contributor

tuxmea commented Dec 9, 2016

I would recommend to keep the array in the type and do data munging on provider level.
This would keep the type declaration as is.

@jnoss
Copy link
Contributor Author

jnoss commented Dec 9, 2016

I considered that, but decided to make it a space separated list to be the same as ceph_host and bridge_list; this way all the onedatastore parameters that can take multiple values are specified the same way.

(Note also that specifying safe_dirs on a onedatastore doesn't work as it is currently (either passing an array or a string to the type causes the provider to fail with the error above) so I don't think changing safe_dirs from array to space separated string would be breaking any config for anyone.)

If we want to make safe_dirs an array I think we'd need to make ceph_host and bridge_list also arrays, which would be a breaking change for current onedatastore configs. (This would bring this in line with the other types, though -- onecluster uses arrays for hosts/vnets/etc and onetemplate uses arrays for disks/nics/etc.)

Let me know what you think--

@Xylakant
Copy link
Contributor

I'm all in favor of unifying parameter types and since we've just done a new major version which hasn't yet been pushed to the forge, this would be the time to make a BC-breaking change if we want to.

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.

3 participants