Skip to content
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

Proposal: standardize file locations & conventions for supporting files #91

Open
jcgraybill opened this issue Oct 12, 2021 · 8 comments
Labels
discuss enhancement New feature or request

Comments

@jcgraybill
Copy link

jcgraybill commented Oct 12, 2021

Most of the OpenSearch plugins in the OpenSearch project include necessary or helpful things such as config files, tools and scripts, security certs, and postinstall/postremove scripts for some deployment environments. From plugin to plugin, these aren't in consistent locations: some are in subdirectories of the plugin itself, some are scattered around other directories in the distribution. Looking just at supporting scripts, they behave differently from each other. Some need $JAVA_HOME and $OPENSEARCH_HOME environment variables set, some autodetect them, some are Linux only, some are Linux/Windows.

This is worth standardizing. For somebody administering an OpenSearch cluster, they shouldn't need to figure out different adminstrator UX conventions feature by feature.

Looking at the current plugins, a few standards I would propose:

  1. Plugins should be entirely self-contained. All files they use should be located inside the /plugins/plugin-name/ directory. None should be in /bin/, /config/, /, etc.
  2. Within a plugin directory, supplementary files should be in directories that match the layout of OpenSearch itself. So e.g. config files should be in /plugins/plugin-name/config/, scripts should be in /plugin/plugin-name/bin/
  3. All scripts should come in *nix and .bat flavors, should use OpenSearch's defaults for environment variables like $JAVA_HOME, so users can override these but don't need to specify them by default. Scripts should have 0755 permissions by default.
  4. Additional directories to think about:
  • lib - for plugins that use native libraries (e.g. libKNNIndexV2_0_11.so)
  • certs - certificates for the security plugin
  • install - scripts called to bootstrap the plugin before first use. These need to be optional, since there's currently no mechanism to guarantee they get called, although perhaps opensearch-plugin could be extended to do this. Should there also be a cleanup/remove directory?
  • docs - generally docs should be on the documentation site, but I see that some plugins bundle documentation with their installable artifact.

Interested in thoughts / feedback on this.

@jcgraybill jcgraybill added enhancement New feature or request discuss labels Oct 12, 2021
@stockholmux
Copy link
Member

A question and two comments:

  • 'Scripts should have 0755 permissions by default.' How is Windows handled? Excuse my ignorance here as I know very little about non-*nix style permissions: Does Window require elevating the permissions of anything in this case? If so then if the package does 0755 for unix permissions, anything in windows should also be set appropriately too.
  • I think the bundled-in docs are an anti-pattern with a couple of exceptions:
    • Docs that are exposed in Dashboards
    • Developer docs should be packed into the repos (although I do question if they should be distributed in the runnable zips)
  • Seems like there should be a manifest of scripts instead of just doing file system discovery. Standard location of scripts is fine, but a manifest would make more sense if you're trying to build automation - it's a one stop file to determine what needs to be executed under what conditions. This could also be used to do validation at build time (check manifest vs what's actually present, etc.)

@dblock
Copy link
Member

dblock commented Oct 19, 2021

  • 'Scripts should have 0755 permissions by default.' How is Windows handled? Excuse my ignorance here as I know very little about non-*nix style permissions: Does Window require elevating the permissions of anything in this case? If so then if the package does 0755 for unix permissions, anything in windows should also be set appropriately too.

Windows has .bat files and doesn't use permissions for what's an executable and what is not AFAIK.

@dblock
Copy link
Member

dblock commented Oct 19, 2021

@dlvenable
Copy link
Member

Overall, I like this proposal and having consistent directory structures.

I have a few comments on specific suggestions:

lib directory

lib - for plugins that use native libraries (e.g. libKNNIndexV2_0_11.so)

I would think that if the plugin structures are being cleaned up, it might be nice to move the Jar files as well. OpenSearch's directory structure has a lib directory which is used for Java jar files. I think it would be more consistent to use lib for Jar files. Maybe it could have both jar and native libraries. But, if the recommendation is to keep jar and native libraries separated, I believe reserving lib for jars keeps consistency with the OpenSearch application.

Scripts

None should be in /bin/, /config/, /, etc.

I agree on keeping the bin files self contained. But, I also wonder if there is room for symlinks in the OpenSearch bin directory. I'm unsure how often scripts are run, but if I were administering a system, I'd generally rather type bin/myplugin.sh than plugins/my-plugin/bin/myplugin.sh.

Config/Data Directories

None should be in /bin/, /config/, /, etc.

I do have some reservations on the config and data directories. If configurations and data were saved in config/plugins/plugin-name and data/plugins/plugin-name, then backing up OpenSearch content is as easy as copying over the config and data directories. And restoring, or scripting a new installation would be just as easy since one would need to copy those directories over.

This approach also helps when mounting volumes in Docker as well: -v /path/to/my/config:/opensearch/config.

That being said, a consistent directory structure can still afford for copying these config/data files. It would require a little more work to get the paths correct.

Other Files

Similar to the OpenSearch application, perhaps the standard can encourage a LICENSE.txt, NOTICE.txt, and README.md in the root of the plugin.

@jcgraybill
Copy link
Author

Scripts

None should be in /bin/, /config/, /, etc.

I agree on keeping the bin files self contained. But, I also wonder if there is room for symlinks in the OpenSearch bin directory. I'm unsure how often scripts are run, but if I were administering a system, I'd generally rather type bin/myplugin.sh than plugins/my-plugin/bin/myplugin.sh.

Any thoughts about how to avoid name collision if we were to do something like this? It's unlikely plugin authors could keep track of script names used in all other plugins. My first thought was subdirectories e.g. bin/my-plugin/do-something.sh but that's not feeling substantially shorter than plugins/my-plugin/bin/do-something.sh

@dlvenable
Copy link
Member

@jcgraybill , I do agree that the collisions can be a problem. And I also think that bin/my-plugin/do-something.sh is not much of an improvement.

I'm not necessarily saying that all scripts should be symlinked into bin. But, that this can be optional for plugins for some scripts when the author believes it may be valuable to run somewhat frequently. So hopefully, few scripts do end up in bin, but it could be allowed.

In regards to collisions, they are ultimately symlinks. So it is more of a convenience for plugin authors. I'd expect that once a script is symlinked that remains the symlink and they are not overridden. Thus, only the first installed plugin gets that name.

@dblock
Copy link
Member

dblock commented Nov 3, 2021

Add load library paths to this, see opensearch-project/opensearch-build#879 (comment)

@jmazanec15
Copy link
Member

I wanted to move this comment from here over to here:

I believe that "config" and "bin" convention already exists: https://github.com/opensearch-project/OpenSearch/blob/1.3.0/distribution/tools/plugin-cli/src/main/java/org/opensearch/plugins/InstallPluginCommand.java#L128-L135.

I am wondering if a better way of going about this would be for the plugin cli to enforce these conventions (as it does for bin and config)?

I think native libraries are a bit challenging. All JNI libraries need to be in the "java.library.path" which we do not set by default. Also, "java.library.path" cannot be set at runtime. I think we could get creative with this, but I think it will require us to set a default "java.library.path" at the very least.

For LD_LIBRARY_PATH, it is a little bit tricky. Setting "java.library.path" will make JNI libraries discoverable, but not their dependencies (even if they are in the same directory). This is where LD_LIBRARY_PATH needs to be set -- unless the JNI libraries can be compiled in such a way as to set RPATH to the directory they are in -- but this would make it more difficult to use 3rd party JNI libraries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants