Add an interactive CLI mode (--cli argument)

[literalplus]

Ellipsis

As suggested by #178, this PR adds a way to access the Jenkins CLI from jenkinsfile-runner.

Technical Overview

To support the new mode of execution, a new JenkinsLauncher superclass has been extracted from the existing JenkinsfileRunnerLauncher, to abstract Jenkins setup capabilities. A new subclass CliLauncher has been added that provides a loop forwarding commands via CliCommand.clone(). App now has a switch on the new Bootstrap.cliOnly flag, and decides which implementation to use.

This flag can be set via --cli or -c, and additionally via a FORCE_JENKINS_CLI environment variable. The latter is mainly intended for use with Docker (otherwise we'd need a custom entrypoint script or a separate Dockerfile just for CLI).

Usage

$ docker run --rm -it -e FORCE_JENKINS_CLI=true jenkinsfile-runner:dev
Connected to Jenkins!
Type 'help' for a list of available commands, or 'exit' to quit.
 > show-plugins
No such command, try 'help'.
 > list-plugins
ace-editor                         JavaScript GUI Lib: ACE Editor bundle plugin                     1.1
...
workflow-multibranch               Pipeline: Multibranch                                            2.21
 > who-am-i
Authenticated as: anonymous
Authorities:
  anonymous
 > bye
$ app/target/appassembler/bin/jenkinsfile-runner -w /tmp/jenkins -p /tmp/jhome/plugins --cli
...

I hope this is somewhat similar to what was meant with "new engine" in #178.

Fixes #178

[oleg-nenashev]

Having a bootstrap would be a cleaner approach IMHO. Jenkins official Docker images do that, and we may need it here to provide better compatibility with classic images. E.g. JAVA_OPTS, DEBUG, JENKINS_OPTS have a real use-case in JFR.

But it is fine to do it as a follow-up

[oleg-nenashev]

Could it be generalized in JenkinsLauncher

[literalplus]

I can move it there, but I don't see how this internal utility method belongs there semantically - the only other existing implementation doesn't do command-line parsing and this use-case seems very specific to this one mode.

[literalplus]

Thanks for the review @oleg-nenashev!

With "bootstrap" you mean the Docker image and providing a docker-entrypoint.sh script that removes the need to handle the env var in the application itself, right? If so, that makes sense & I can do that. Can you think of anything else such a script should do besides JENKINS_OPTS and DEBUG? Otherwise, can you please elaborate?

I just noticed that the integration tests are failing due to an IllegalAccessError which sounds...interesting, I'll take a look at it & push the solution together with the two resolved discussions if I find one.

Update: The IllegalAccessError seems unrelated, also occurs on master. The tests also fail there and in other PRs. Haven't yet figured out which of the tests is failing and it is related to my changes (I find the shunit output a little unintuitive to read)

Update: The failing test is test_pipeline_execution_hangs in tests-negative-scenarios.sh. I'm not sure why this fails.

Update: Failures seem to be random. I first thought it was caused by my earlier PR, but running it a second time with the master before that also failed it. Possibly related: Error response from daemon: conflict: unable to delete 044193f3a355 (cannot be forced) - image is being used by running container 42ad8f3a0ce7

Update: I got the tests to pass locally by re-trying in a loop: while ! bash ./tests-negative-scenarios.sh; do; echo "try again"; done: I guess we could re-try them on the CI multiple times as well, but this seems more like a hack than a solution.

[oleg-nenashev]

With "bootstrap" you mean the Docker image and providing a docker-entrypoint.sh script that removes the need to handle the env var in the application itself, right?

Yes

Update: The failing test is test_pipeline_execution_hangs in tests-negative-scenarios.sh. I'm not sure why this fails.

Just ignore it for now, please. CI is still not very stable. I will test it manually before merging

[oleg-nenashev]

Retriggering CI

[literalplus]

merge conflicts resolved

[literalplus]

In the meantime, another flag with the -c shorthand was added (for setting the cause), so I have removed that alias.

[oleg-nenashev]

I am not a big fan of shortcuts at all :( Thanks for adjusting the PR!

…

On Sat, Oct 26, 2019, 19:52 Philipp Nowak ***@***.***> wrote: In the meantime, another flag with the -c shorthand was added (for setting the cause), so I have removed that alias. — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#207?email_source=notifications&email_token=AAW4RIB76VGGEHXQGG2X6N3QQRYU3A5CNFSM4JCSOE5KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECKMGAI#issuecomment-546620161>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAW4RIBY34SLF4J5CWFEKJ3QQRYU3ANCNFSM4JCSOE5A> .

[oleg-nenashev]

I tested it manually, and it works like a charm. Thanks!
I will go ahead and merge it without #213, we can integrate it in a follow-up