griddynamics / mpl

[IT-36925] Jenkins Shared Modular Pipeline Library
https://blog.griddynamics.com/developing-a-modular-pipeline-library-to-improve-devops-collaboration/
Apache License 2.0
157 stars 97 forks source link

Suggestion for configuration #21

Open digulla opened 5 years ago

digulla commented 5 years ago

This is another approach to enhance the config (similar to #10).

I would suggest a "default config" object which contains all possible config keys and defaults.

Modules must have a method to add to this default config. Adding the same "path" twice is an error but it should be possible to check for an existing path (so two modules can cooperate and still be optional).

This way, modules don't need code like

CFG.'maven.tool_version' ?: 'Maven 3'

because 'Maven 3' will be the default value. Reading unknown keys should throw an exception (-> typo in library)

The step which parses the project's config options from the should throw exceptions for unknown keys/paths to catch typos.

After merging the project and default config, the result should be logged in Jenkins to allow to find mistakes.

I'm also not fond of the CFG.'xxx.yyy' syntax. With the default config as path supplier, it should be possible to implement a nested config using nested Maps which would then allow to pass inner maps around so the code would become independent of the path.

In my builds, I'm calling Maven several times (build with unit tests, site, deploy without tests). Using nested configs, I can create a shared Maven config (default JDK & Maven tool) but I can also tweak the options passed to each individual Maven invocation like additional profiles to activate or system properties. As an example, I can:

        config {
            build {
                addAfter 'install', 'foo'
            }
        }

to get mvn [...] clean install foo sonar:sonar as command line or I can

        config {
            maven {
                property('foo', params['foo'])
            }
        }

to add -Dfoo=[...] to every Maven invocation.

My current code depends on a hand-written config builder class for each nested config object. That way, I can support things like addAfter(). I'm looking into ways to change this so modules can easily define their default config.

sparshev commented 5 years ago

Hello @digulla,

Happy to see you thinking about the improvements for MPL configuration) Unfortunately it's quite hard to understand the idea without a code, but seems like you proposing for some huge interface changes. I'm wondering to see the fork / nested shared lib / PR with some simple examples to compare the approach with already provided one, how this will affect the interfaces and to imagine the future of proposed configuration schema. So we can discuss the changes and choose the right way of the further improvements.

Thank you

digulla commented 5 years ago

I've created a gist with unit tests: https://gist.github.com/digulla/15a55d8a5cefb2bda1e2da1fb6dae4f7

I'd like to first discuss this code and then start with a fork.

sparshev commented 5 years ago

Thank you @digulla , we will check it shortly.

sparshev commented 5 years ago

Hi @digulla,

I checked the source code and from the tests I see, that the configuration becomes quite bulky. Also it's hard to predict where this will go. I think it just will move a part of the module logic to the configs, that seems not good... You saying, that there will be no need of CFG.'maven.tool_version' ?: 'Maven 3' - but I think it will be just moved somewhere else... Anyway it's still hard to say for sure using this code, because probably I don't understand it correctly. So it will be good to see the fork with your idea implementation to compare with the existing solution.

Thank you

digulla commented 5 years ago

My goals are:

You saying, that there will be no need of CFG.'maven.tool_version' ?: 'Maven 3' - but I think it will be just moved somewhere else...

It has to be somewhere. With your design, all plugins accessing this config option must use the same pattern to read it. Specifically, the default value must be the same everywhere or weird things will happen ("hey, why is it doing this here and that there????").

In my design, the default config does it for everyone, so everyone can simply use CFG.maven.tool_version and they will get a non-null String. Defaults don't have to be kept in sync in several places.

I think we should talk about my design goals. I think they are worth the effort because they help to catch a lot of bugs.

sparshev commented 5 years ago

Yeah, agree. We just need to compare the differences of approaches, the interface changes, required efforts to implement and backward compatibility to make some other decisions. Right now I think that probably it's possible to give a way for the end users to test this new way. I think it will be much easier to play with the fork and on practice see the benefits and maybe the negative points.

digulla commented 5 years ago

I ran into an interesting question while trying to implement my idea. Where to put the default config? I see threse options:

I don't like option 1 very much. Only if we're very desperate.

With option 2, I expect name collisions when we merge the override configs (configs in the projects).

With option 3, a lot of code would have to be changed. Also, CPS might cause problems. Lastly, people might try to put state into those classes and we would have to decide how to handle that.

Option 4 sounds doable but I would like to have the config and the code in a single file. Otherwise, we might set a precedent and end up with tons of file fragments.

That aside, you currently have some logic to prevent endless loops when a module calls it's parent. With option 2 and 3, the parent code could be passed to the override module as closure.

sparshev commented 5 years ago

Probably it will be suitable to create vars/MPLModuleDefaults.groovy with logic to set module defaults and prepend each module with such call with specifying defaults map... It will move defaults to the module top that is pretty close to the rest of the module logic, will slightly increase complexity, but will be backward-compatible with the old way...

digulla commented 5 years ago

I would really like to collect all config options before starting to build.

Maybe a solution would be to use a different approach for the pipeline: Instead of using explicit steps, we could create the steps dynamically (see https://stackoverflow.com/questions/42837066/can-i-create-dynamically-stages-in-a-jenkins-pipeline).

That way, we could collect all the modules which are part of the build beforehand. With a flag, we could disable executing the "build code" module. Then we run all the modules to collect all config options.

With that, we can merge the overrides for the current build, log the result, enable execution again, disable the code in MPLModuleDefaults (otherwise, we'd get duplicate key errors) and do the actual build.

The other solution would be to run all modules that we can find but that would run Checkout.groovy, DefaultCheckout.groovy and GitCheckout.groovy. I'm uneasy about this.

Or is there a way to discover the stages in a Jenkinsfile while running it? Isn't Jenkins showing all stages before it starts executing the first one?

sparshev commented 5 years ago

Hi @digulla

You can use any pipelines you want - default MPLPipeline is just to show a simple example pipeline. For my project I'm using the same structure, but with the custom logic and agents per stage (because we using input for redeploy and just to save some money).

Sorry, but it's still overcomplicated to just understand what you saying without some logic to play with... I'm usually just trying one way, that if something is not working perfect enough - another and so on.

There for sure some ways (at least you can use vars steps and going deep into the jenkins objects to locate what you need) - but be careful, noone likes too much complicated logic...