-
Notifications
You must be signed in to change notification settings - Fork 670
weave script is becoming unmaintainable and limiting #307
Comments
I think that we can always bring back hackability by providing simple exec overrides to each of the main steps or even a some kind of interpreter (e.g. Lua or MicroPython). This is probably not of importance in the short term, but I just thought I'd make sure to mention what I've been thinking about this subject. I believe that moving from shell script to binary tool will eventually arise requests for configuration files, and I think we should discus hooks/scripting interfaces then. I think it would be much easier to provide a hook to override an entire step rather then provide configuration parameters for every single call within that step. This of course depends on what exactly we get asked for. |
I have forked the script once for Kubernetes, it has a few things that are pretty much same in upstream version and I can go through it in more detail if we want to, but it might be an extreme case of modification. |
Bear in mind that the role of the weave script (or whatever supersedes it) could change significantly when the new docker networking stuff happens. That might still be some time away, but knowing that it will happen, perhaps we should try to live with the current weave script, rather than making big plans to replace it or enhance it in ways that might not make sense after that transition. If we can identify specific maintainability issues in the weave script, maybe there is something we can do to mitigate them? |
If we do end up moving to a binary CLI, the heroku toolbelt and cli would probably be a good model. |
While writing the plugin, I started moving implementations of common weave operations into a separate module. So far I've been able to replicate the important bits without needing the script at all, which is nice. As an extrapolation, we could
We lose |
That's a good point. Now that the new docker networking stuff has happened, we can review this. My feeling is that it's going to be a while before the docker plugin is able to have parity with the weave script; perhaps it will never be as flexible or have the same features. Meanwhile, we can share some code between the plugin, script replacement, and proxy -- and perhaps other integrations. |
Nice indeed.
So you are suggesting a gradual transition, where pieces of the script's functionality are migrated (with focus on the pieces needed by the proxy)? And at some point in the future, we can look at what's left and think about converting that? |
We can do that, yes. At some point (pretty quickly I expect), there would be no bits of the script needed by the proxy or plugin, and we could convert weaveexec over at our leisure. Or we can pursue this as an end in itself if we want. |
Well, the "end" would be significant benefits for maintainability and robustness. |
I like the "gradually expanding exe" approach. That will make it much easier to tackle this issue, since we can make incremental progress. Plus, it addresses the duplication of functionality between the script, proxy and plugin that is currently creeping in. Re the executable... we could make it a release artifact, so people running |
It would be great if user could compile the executable with go get too. It On Wed, 1 Jul 2015 12:15 Matthias Radestock [email protected]
|
The weave script is doing quite a lot of work and keeps growing. It's getting to the point where it becomes quite hard to maintain and extend. Also,
sh
is a rather limited environment for complex programming, and access to any kernel features is always mediated by tools, which introduces dependencies, versioning issues etc.On a positive note, the script has served us well as being the single thing that needs to be downloaded and installed - no need for packaging an OS specifics. Also, it has proved very hackable - we've received quite a few PRs for the script; far more than the go code.
Replacing the script with a statically linked exe would retain the "single download, no fuss install" of the current script.
The text was updated successfully, but these errors were encountered: