-
Notifications
You must be signed in to change notification settings - Fork 327
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Config-Based Module Support #275
Conversation
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. |
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. |
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?
|
|
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=docker This 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. |
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.
6c3fb0e
to
7388d25
Compare
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
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-admin
default-docker
Default modules can be overridden in a custom configuration file by simply using the same names as above.
If the
host
orspec
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: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 thedefault-docker
module'shost
andspec
properties as well. The difference is that thedefault-docker
module'shost
andspec
properties will default to values based not on the module name butunix:///run/docker/plugins/rexray
and/etc/docker/plugins/rexray.spec
.