Replace File operations with methods provided through the Jenkins context

[metmajer]

Using methods on java.io.File or java.nio.file.* makes pipeline code run exclusively on the Jenkins master. We hope that, by replacing all File operations, we can be more flexible in executing code on slaves as well. Arguments of type File will need to be replaced with a String path within the current workspace

@hrcornejo can you please investigate?

[hrcornejo]

@metmajer Ok.

[oalyman]

For more context I recommend this blogpost: https://jenkins.io/blog/2017/02/01/pipeline-scalability-best-practice/

Except for the steps themselves, all of the Pipeline logic, the Groovy conditionals, loops, etc execute on the master. Whether simple or complex! Even inside a node block!

In case of the MRO as of now it basically comes down to java.nio.file.Paths#get and java.io.File and more or less exists or read usage. Those should be replaceable by readFile, exists etc. from https://jenkins.io/doc/pipeline/steps/workflow-basic-steps/

One approach could be to build a File facade for those step functions similar as PipelineSteps to be able to unit test.

[clemensutschig]

This is fixed.. or?

[michaelsauter]

No this is not done yet. This requires lots of work (e.g. around document generation). We side-stepped the issue using the stashing approach (which results in having the same path structure on master and slave node).

I believe it to be really important to invest time in this though - the MRO should run its pipeline on a slave node. Otherwise there are lots of hacks and weird issues.

[clemensutschig]

@hrcornejo - are you on this hugo? is this a ODS 3 topic for you folks?

[martsec]

The big issue I see here @clemensutschig is that we do not know beforehand the file names for the junit-like test reports and the sonarqube file names. And then we unstash, filter the contents in the folder and get directly the files. So either we save as part of the context the name of those files and obtain them through readFiles or I can't see any other solution since Jenkins as far as I know, does not allow to list the stashed files.

FYI @jorge-romero @hrcornejo @metmajer

[jafarre-bi]

I'm working on this issue and find out that many cases would greatly benefit from using the File Operations Plugin, for instance, to create a directory or delete a file. The alternatives are cumbersome. What do you think, @metmajer ?

[clemensutschig]

I would strongly suggest that we use jenkins core operations - 'dir' will create a directory for example . and from what I remember it's just a very few places - makes testing also much easier

[michaelsauter]

@jafarre-viewnext Awesome that you are working on this. I believe that solving this issue will be a huge win. The current hacks in place with the jumping between slave and master will always cause issues. If you have any question about the current situation regarding this slave-master switching, the stashing/unstashing etc., please do not hesitate to contact me.

[jafarre-bi]

Thank you, @michaelsauter , I certainly will.

@clemensutschig Actually in the end I may need it only in one or two places, but it's very frustrating that for something as basic as deleting a single file, I either use java.io.File or run sh 'rm -f ...'.

Also, it seems like dir only creates the directory if you actually do something in the closure that requires the directory to exist. In order to implement PipelineUtil.createDirectory without java.io.File, again I either use sh or do something unclean to force dir creating the directory.

I honestly believe this plugin can a cleaner solution where it's needed, but I'll investigate how to circumvent the need.

[clemensutschig]

@jafarre-viewnext - we delete Directories.. so that should be easy.. writefile in dir closures should solve the rest.. Let me know if you see other issues..

[michaelsauter]

What's wrong with sh to remove / create dirs? I'd say that is simple enough, not something I would use a plugin for.

[jafarre-bi]

Ok @clemensutschig . @michaelsauter I have been told opening shells is not the preferred choice, but yes, it is a good solution in many cases. If using basic commands, it can even be written both for Unix and Windows, in case in the future we use Windows slaves.

[jafarre-bi]

For methods that took a File parameter, it is impossible to be backwards compatible. When a String with file name or path was enough, I used a String. For the cases where a String wasn't enough, I created the bean util.FileInfo, which is quite equivalent to org.jenkinsci.plugins.pipeline.utility.steps.fs.FileWrapper, used internally, for instance, in findFiles step. I haven't used FileWrapper directly, because it is not available in the classpath. Of course, it could be added.

[jafarre-bi]

I have found a clash between tests when creating directories and files. There are a couple of tests that create a directory and another test that create a file with exacly the same name. Depending on which one runs first, one or the others fail. Right now, all test specs set PipelineSteps.env.WORKSPACE to the default temp dir. One solution is to create a separate temp subdirectory for each test spec. Comments are welcome. In the meanwhile, I'm investigating why this clash doesn't seem to reproduce in the master branch.

[michaelsauter]

@jafarre-viewnext When you talk about "backwards compatible", what exactly do you mean? Are there any methods exposed to the user that take file parameters? I'm wondering as the only a very small surface is exposed to the end-user, and for methods hidden within the orchestration pipeline, I would deem backwards compatibility only a minor concern.

[jafarre-bi]

@michaelsauter As far as I'm aware, it's all internal API, so it should be a minor concern.

[jafarre-bi]

This issue has grown much bigger than it seemed initially, mostly to make unit tests work after the changes.

I have created a new issue that needs to be solved before this can be completed: https://github.com/opendevstack/ods-jenkins-shared-library/issues/379

[martsec]

As we have seen together with @jorge-romero and @stitakis this has broader ramifications and thus for now we are going to focus the efforts on e2e testing and leave this for another day. @jafarre-viewnext, since I think you have the code, if someone wants to check-it-out could reach out to you, right?