deviceinsight / helm-maven-plugin

Maven Plugin for Helm Charts
Apache License 2.0
46 stars 22 forks source link

[Enhancement] - Make helm plugin thread safe #87

Closed kobynet closed 3 years ago

kobynet commented 3 years ago

Since we use maven parallel build to speed up our builds, this is kind of a blocker due to errors unzipping the helm binary in parallel ("text file in use" kind of errors).

This is more of a brainstorming issue and i was wondering if there are other places which arent thread safe, and how would you attack this? Maybe global lock(process level) for the downloadFileAndExtractBinary method ?

pvorb commented 3 years ago

Hi Koby, I see the problem here.

How do you configure your parallel builds? Do you configure multiple plugin executions and let Maven do it for you? I'd guess that's not the case, since it's a bit harder than just running Maven in separate processes.

If the latter is the case, a traditional synchronized or some other in-process lock wouldn't help.

We could maybe write a .lock file and look for that before extracting the zip file, but I'm not looking forward to implement this for such an edge case.

Are there possible workarounds? Maybe you could use different values for MAVEN_HOME, but this might eliminate the benefit of running it in parallel. Or you could download the Helm binary as part of you build job and place it in the right location before running Maven at all?

But it would certainly help if you could describe your setup a little more.

kobynet commented 3 years ago

Hi @pvorb , We are actually running maven parallel builds using the flag "-T 1C" which means 1 thread for each CPU. We have a repository with few services, which each of them uses the helm-maven-plugin. Since they are not a dependency of each other, maven runs it's lifecycle phase (install/deploy, etc..) for each of them in parallel.

Are there possible workarounds? Maybe you could use different values for MAVEN_HOME, but this might eliminate the benefit of running it in parallel. Or you could download the Helm binary as part of you build job and place it in the right location before running Maven at all?

We are actually running builds on jenkins with kubernetes plugin which is pretty common use-case. We can download the binary before, but that's a bit cumbersome since we like the idea of having most of the configurations in maven and not in jenkins job. Since maven runs in single process with parallel threads, process-lock level would probably solve the issue, regarding synchronize in the code, i'm not sure if maven creates new instance of the plugin for each module or it uses the same instance (my guess is it creates new instance so configuration won't get mixed up).

kobynet commented 3 years ago

After doing a bit of investigation on this, using inter-process locks might pollute the running env in case of failures, etc. I've come to the conclusion that it's best to add another goal ("resolve" goal) that isn't bound into any lifecycle phase, and it is just a split of the resolve phase from AbstractHelmMojo. All the previous behaviour stays the same.

This way we can run something like
mvn com.deviceinsight.helm:helm-maven-plugin:2.9.0:resolve -DhelmVersion=2.17.0 just before our build, and when running in parallel or not, the binary will already be there and therefor won't have a clash in downloading it.

Made a pr for this

pvorb commented 3 years ago

Resolved by #88