Config-Based Module Support #275
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hi @clintonskitson, I'm going to assign this to you for testing. I've tested it on my Linux VM and it behaves as I think it should -- modules are started via the configuration file as we discussed. The one difference is that I did use rooted scope-keys for modules. For example, in the PR YAML you'll note that Since they're required, I figured we'd make it easy. |
akutz
force-pushed
the
feature/config-backed-modules
branch
from
867769b
to
a7ada55
Compare
|
@akutz Am I missing something re how this would function? I am getting a driver init error from this. Just remembered the CLI functionality also would need to be addressed. I suppose we can define that the root level config is for the CLI, ie |
|
Hi @clintonskitson, Could you please include the error? Also, what CLI functionality? I already noted that the CLI commands would not be at all guaranteed to work for this RC. That's the tax on the configs-per-module in the file. That said, I did note in Slacker that the command do work for the most part. Again, I'm not sure what error you're seeing. |
|
Hi @clintonskitson, FWIW, I bet you have an errant tab or incorrect spacing in the above file. It's what bites me most often. At least that's my guess without more information. |
|
Hi @clintonskitson, I know you love to hear this, but... "it works for me" :) Here's my config file: |
|
The sock files are in place as well: |
|
And just to make sure, here it is started in the background: |
|
Your config is throwing this at me |
Then run it. Or use the env command. The mock drivers must be enabled to use them. |
|
Ok, the mocking worked. When I remove the mocking drivers I get this error. It looks like the config isn't making it to the driver init. |
|
Hi @clintonskitson, It would be helpful if you'd provide your altered config. Did you remember to add back in the storage drivers bit at the top? |
|
|
Hi @clintonskitson, Where is your config file? Is it in a home directory or a shared location? |
|
It is in |
|
Hi @clintonskitson, Well, I know it's reading the config correctly. Look at my new config: |
|
You should be able to use my example config and hit the same exact error. The following line shows that the configured variables aren't making it into the driver. |
|
Hi @clintonskitson, Works for me, and I'm no longer enabling the mock drivers (they are listed in the config, but they are useless sans the environment variable that enables them): Here's the REX-Ray log grepped down to just the INFO lines as I placed on in the ScaleIO driver to emit the URL. Notice that two of the ScaleIO modules do list the URL while one does not. Since you're only listing two modules, would you like to guess why a third is appearing? It's the default module we discussed. You're not using it, and therefore since nothing is defined as a default ScaleIO URL, it's trying to initialize to a default, empty value. It's working exactly as it's designed. Configure the default docker module with the name |
|
Are you sure your build isn't dirty? =) |
|
Hi @clintonskitson, It is, but only because I had to add logging statements & return early from the ScaleIO init method (since my ScaleIO driver will obv. not emit.). Want me to commit what I have exactly? I can revert it prior to the PR merge. |
|
Hi @clintonskitson, There ya go, I just pushed the changes I've been working with: Here's my config file as well:
I told you, it worked for me. As Dr. Crusher would say:
Or in this case, with your testing. It's working like we decided it should -- the default module loads right up as it did before, for better or worse, and you're no longer configuring it. You need to do so by adding a root |
|
Hi @clintonskitson, Please feel free to rethink/reconsider how default modules should behave. I'm fine with whatever. I like this method on which we agreed -- if the config is in error, things fail. We would have noticed it sooner as the logs note that it's the |
|
That worked for me. Is the problem that the default module initialization is blank so it is bombing out there before moving on? |
|
Hi @clintonskitson,
Yes, that's what I said above. And that's by design. If the configuration isn't valid, it fails REX-Ray. Users can either remove the invalid section or fix the problem. |
|
Hi @clintonskitson, Specifically, if you don't override it, REX-Ray defines two modules as I note in the commit message as well as the PR description. Well, note that there's nothing there that indicates what drivers to use, let alone how those drivers are configured. So just like before, if you fail to configure the default values, the service fails to start. The default modules are expected to be configured because you specifically didn't want to break the previous behavior. |
akutz
force-pushed
the
feature/config-backed-modules
branch
from
5c28e84
to
a7ada55
Compare
|
Hi @clintonskitson, So what's the verdict? |
|
Been delayed due to running into some other technical stuff. What I've narrowed it down to is the following three configurations. Trying to figure out for simplicity and consistency is we're in the right ball park. Let me play through a few mixtures and get your feedback. Just adding a module seems to be a bit confusing since
I think this is getting to be more desirable since all sockets can be customized. But the root I think this one would be ideal. I like it because it is very clear how to configure the modules. This one doesn't work. Thoughts? |
|
akutz
force-pushed
the
feature/config-backed-modules
branch
from
a7ada55
to
567d6f7
Compare
|
Hi @clintonskitson, Okay, I've updated the CLI per out conversation, and based on the logs it looks good to go. Give it a spin. Logs[0]root@poppy:bin$ ./rexray start -l debug
DEBU[0000] updated log level logLevel=debug
INFO[0000] [linux]
INFO[0000] [docker]
INFO[0000] [isilon]
DEBU[0000] core get drivers osDrivers=[linux] storageDrivers=[isilon] volumeDrivers=[docker]
INFO[0000] os driver initialized provider=linux
INFO[0000] volume driver initialized provider=dockerThis line One-Line Change to the CLI
|
|
Hi @clintonskitson, By the way, if you're happy with the pattern we discussed I'll go ahead and update the documentation explaining module support as well as how to override the default modules. |
|
@akutz Thumbs up, works now. This is an example of virtualbox with modules that works. And the contrasted one without modules that works. |
|
Hi @clintonskitson, Great. Please try this one though: This should have the same effect as the modular config you provided. It's a little closer to an in-between version of the two -- one with defaults (legacy) and one with modules. In the example I provided, only the |
|
Hi @clintonskitson, Also, we could get sneaky and decalre that aside from Then the |
|
If the module itself applied the base path, ie. |
|
Working on previous example now. |
|
@akutz the example you gave tests good. |
|
@akutz I made a comment to you on slack re the |
|
Hi @clintonskitson, Let's leave it as
|
|
Roger, agreed. |
akutz
force-pushed
the
feature/config-backed-modules
branch
from
567d6f7
to
6c3fb0e
Compare
|
Hi @clintonskitson, Please see the updated PR description as well as making sure the changes look good to you. I tested them, and they appear to work, but two sets of eyes...
|
|
Hi @clintonskitson, Did you confirm the inferred sock and spec file creation? |
This patch adds support for configuration modules via the REX-Ray
configuration source. For example, the following configuration is loaded
by default:
rexray:
modules:
default-admin:
type: admin
desc: The default admin module.
host: tcp://127.0.0.1:7979
disabled: false
default-docker:
type: docker
desc: The default docker module.
host: unix:///run/docker/plugins/rexray.sock
spec: /etc/docker/plugins/rexray.spec
disabled: false
The above configuration indicates that there are two modules that need
to be initialized and started:
1. default-admin
2. default-docker
Default modules can be overridden in a custom configuration file by
simply using the same names as above.
If the `host` or `spec` properties are not defined for a docker module,
then the sanitized name of the module is used to build the paths to a
socket and spec file. For example:
rexray:
modules:
"isilon 2":
type: docker
The above configuration would create a Docker module hosted using a
socket file at `unix:///run/docker/plugins/isilon-2.sock` and spec file
at `/etc/docker/plugins/isilon-2.spec`.
If the configuration file is used to override part of the
`default-docker` module's configuration, for example its description,
it's also possible to omit the `default-docker` module's `host` and
`spec` properties as well. The difference is that the `default-docker`
module's `host` and `spec` properties will default to values based not
on the module name but `unix:///run/docker/plugins/rexray` and
`/etc/docker/plugins/rexray.spec`.
Modules can also be disabled using `disabled: true` with any module,
including default modules. This is useful as it allows the disabling of
a module without completely removing its configuration from the file.
akutz
force-pushed
the
feature/config-backed-modules
branch
from
6c3fb0e
to
7388d25
Compare
akutz
added a commit
to akutz/rexray
that referenced
this pull request
Mar 4, 2016
This patch marks the release of REX-Ray 0.3.2! NEW FEATURES * Support for Docker 1.10 and Volume Plugin Interface 1.2 rexray#273 * Stale PID File Prevents Service Start rexray#258 * Module/Personality Support rexray#275 * Isilon Preemption rexray#231 * Isilon Snapshots rexray#260 * boot2Docker Support rexray#263 * ScaleIO Dynamic Storage Pool Support rexray#267 ENHANCEMENTS * Improved installation documentation rexray#331 * ScaleIO volume name limitation rexray#304 * Docker cache volumes for path operations rexray#306 * Config file validation rexray#312 * Better logging rexray#296 * Documentation Updates rexray#285 BUG FIXES * Fixes issue with daemon process getting cleaned as part of SystemD Cgroup rexray#327 * Fixes regression in 0.3.2 RC3/RC4 resulting in no log file rexray#319 * Fixes no volumes returned on empty list rexray#322 * Fixes "Unsupported FS" when mounting/unmounting with EC2 rexray#321 * ScaleIO re-authentication issue rexray#303 * Docker XtremIO create volume issue rexray#307 * Service status is reported correctly rexray#310 UPDATES * <del>Go 1.6 rexray#308</del> THANK YOU * Dan Forrest * Kapil Jain * Alex Kamalov
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch adds support for configuration modules via the REX-Ray configuration source. For example, the following configuration is loaded by default:
The above configuration indicates that there are two modules that need to be initialized and started:
default-admindefault-dockerDefault modules can be overridden in a custom configuration file by simply using the same names as above.
If the
hostorspecproperties are not defined for a docker module, then the sanitized name of the module is used to build the paths to a socket and spec file. For example:The above configuration would create a Docker module hosted using a socket file at
unix:///run/docker/plugins/isilon-2.sockand spec file at/etc/docker/plugins/isilon-2.spec.If the configuration file is used to override part of the
default-dockermodule's configuration, for example its description, it's also possible to omit thedefault-dockermodule'shostandspecproperties as well. The difference is that thedefault-dockermodule'shostandspecproperties will default to values based not on the module name butunix:///run/docker/plugins/rexrayand/etc/docker/plugins/rexray.spec.