openshift-evangelists / oc-cluster-wrapper

oc cluster up bash wrapper
Apache License 2.0
144 stars 72 forks source link

Make plugin system for additional commands #20

Closed jorgemoralespou closed 7 years ago

jorgemoralespou commented 8 years ago

We need to find a way to add additional commands to provision stuff (nexus, gitlab, tech-preview,...) that will get the commands from a directory. Can be current or can be in the profile, although I think current is better, since then oc-cluster can always load additional commands that can be pulled from git.

This way we can move some stuff into the plugin mechanism, so that users can create their own without needing to modify this script.

bjartek commented 8 years ago

The simplest I can think of here is doing like they do with /etc/profile.d.

That is inside the profile we make we create a plugins.d folder. Then in the appropriate place in the run script we do

for f in $__profile/plugins.d/*.sh; do
    [[ -r $f ]] || continue
    . $f
done

We have to create some guidelines for how to write plugins. They should basically do all code in a function of their own and prefix it with the name of the plugin.

So say we want a plugin that enables jenkins pipelines. We have flag that turns it ok. And if it is on create a files in plugins.d called jenkins.sh. That jenkins.sh file has a jenkins_main function that is called at the bottom of it.

bjartek commented 8 years ago

I guess the plugin system has to restart the cluster once it is done customizing it. So the flow is

oc cluster up do all the plugins oc cluster down oc cluster up

or something like that.

jorgemoralespou commented 8 years ago

Well, there will be different types of plugins, and we'll need to better describe them: There will be:

We need to define how these will be implemented and described. Sadly I'll be next 2 weeks at conferences, but I'll try to use some time during these to think/document this. Feel free to keep contributing/discussing.

And by the way, thanks for all what you're doing so far, it's been amazing.

jorgemoralespou commented 8 years ago

Restarting a cluster works with:

docker stop origin && docker start origin

jorgemoralespou commented 8 years ago

Completions can be optional. I don't want completions to stop us from doing this, so if it's complex then it should have no completions.

bjartek commented 8 years ago

Have a look in my branch plugin-system.

It is pretty rough right now but it works for enabling pipelines even though you get an error after restarting the origin container.

https://github.com/bjartek/oc-cluster-wrapper/blob/plugin-system/oc-cluster

This script has support for adding thew flags and new commands. See the test plugin for a command and the pipelines plugin for a flag.

I use the an directroy in the filesystem as a dispatch table. filenames are commands/flags and the contents of the files are the name of the function to run.

A plugin can set the varaible RESTART_CLUSTER to true and the origin container will be restarted at the end of the run command

jorgemoralespou commented 8 years ago

@bjartek I'm reviewing this. To be honest, seems too complex, also as there's very few comments and I'm no bash expert, I'll have to look into it throughly.

/cc @GrahamDumpleton can you review this and make comments as well?

bjartek commented 8 years ago

I was really unsure on how to approach this. I do not know what skill set you people have, and do not want to step on toes.

I can always add more comments if that is needed to understand what is going on here.

My personal opinion on this (having coded bash almost daily for a year) is that we are approaching the real pain point here. Bash is simply not the best tool for doing a plugin system or creating portable complex stuff. My reasonings are as follows

The best thing for complex logic here would be to send PR's to the oc cluster command directly IIRC. We can always add functionality, but doing it as a plugin system is hard.

bjartek commented 8 years ago

I pushed some comments to my code.

jorgemoralespou commented 8 years ago

@bjartek I have pushed a plugins branch, with what I envision as a simple plugin mechanism. It's not fully working but you can test doing:

oc-cluster --user oc-cluster --pipelines oc-cluster user-add (this doesn't do anything yet)

The rest of the stuff should be working as before

https://github.com/openshift-evangelists/oc-cluster-wrapper/tree/plugins

Just take a look and tell me whether this is too different, more complex, easier, or anything. I would like to have a super-simple approach. As I would like any non "basher" to easily craft plugins.

/cc @GrahamDumpleton You should also take a look

GrahamDumpleton commented 8 years ago

Not sure I like installation of a plugin being by a long opt.

Would perhaps prefer something like:

oc-cluster plugins enable user

This would enable/install the plugin.

In case it made sense in some cases to be able to rollback plugin, also:

oc-cluster plugins disable user

To get a list of plugins and whether currently enabled:

oc-cluster plugins list

and what the plugin is about:

oc-cluster plugins help user
bjartek commented 8 years ago

Sorry for not replying sooner, I have been busy with personal stuff.

After reviewing your code i can understand why you though my solution was complex. Yours works in a very different manner. It basically adds stuff to an already running profile.