Fix global instance singleton resolver

[kuzaxak]

According to the doc to get an instance of a config we should use ExtensionList.lookupSingleton(<your GlobalConfiguration subclass>.class)

Initially started to check the code because we got exceptions on agents when using Git checkout:

java.lang.IllegalStateException: Jenkins.instance is missing. Read the documentation of Jenkins.getInstanceOrNull to see what you are doing wrong.
	at jenkins.model.Jenkins.get(Jenkins.java:816)
	at jenkins.model.GlobalConfiguration.all(GlobalConfiguration.java:75)
	at jenkins.plugins.logstash.LogstashConfiguration.getInstance(LogstashConfiguration.java:289)
	at jenkins.plugins.logstash.LogstashWriter.getIndexerDao(LogstashWriter.java:162)
	at jenkins.plugins.logstash.LogstashWriter.getDaoOrNull(LogstashWriter.java:201)
	at jenkins.plugins.logstash.LogstashWriter.<init>(LogstashWriter.java:82)
	at jenkins.plugins.logstash.pipeline.GlobalDecorator.decorate(GlobalDecorator.java:41)
	at org.jenkinsci.plugins.workflow.log.TaskListenerDecorator.decorateAll(TaskListenerDecorator.java:230)
	at org.jenkinsci.plugins.workflow.log.TaskListenerDecorator$DecoratedTaskListener.getOutputStream(TaskListenerDecorator.java:267)
	at org.jenkinsci.plugins.workflow.log.OutputStreamTaskListener$Default.getLogger(OutputStreamTaskListener.java:116)
	at org.jenkinsci.plugins.workflow.log.TaskListenerDecorator$CloseableTaskListener.getLogger(TaskListenerDecorator.java:307)
	at org.jenkinsci.plugins.gitclient.CliGitAPIImpl.launchCommandIn(CliGitAPIImpl.java:2801)
	at org.jenkinsci.plugins.gitclient.CliGitAPIImpl.launchCommandIn(CliGitAPIImpl.java:2762)
	at org.jenkinsci.plugins.gitclient.CliGitAPIImpl.launchCommandIn(CliGitAPIImpl.java:2757)
	at org.jenkinsci.plugins.gitclient.CliGitAPIImpl.launchCommand(CliGitAPIImpl.java:2051)
	at org.jenkinsci.plugins.gitclient.CliGitAPIImpl.launchCommand(CliGitAPIImpl.java:2063)
	at org.jenkinsci.plugins.gitclient.CliGitAPIImpl.revParse(CliGitAPIImpl.java:1115)
	at hudson.plugins.git.GitAPI.revParse(GitAPI.java:419)

Testing done

Added additional test to handle a remote logging from workflow pipeline job executed on agent.

Don't know how to test Git plugin within tests of this plugin.

https://issues.jenkins.io/browse/JENKINS-71693

Submitter checklist

Beta Give feedback

1. Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
2. Ensure that the pull request title represents the desired changelog entry
3. Please describe what you did
4. Link to relevant issues in GitHub or Jira
5. Link to relevant pull requests, esp. upstream and downstream changes
6. Ensure you have provided tests - that demonstrates feature works or fixes the issue

[jakub-bochenski]

LGTM, thanks a lot!

[jakub-bochenski]

BTW I think it was already reported on jira

[jakub-bochenski]

yep, https://issues.jenkins.io/browse/JENKINS-71693

[kuzaxak]

LGTM, thanks a lot!

I didn't tested this yet on real Jenkins instance. Will post an update here.

[kuzaxak]

Nope, different error, but still an exception:

java.lang.IllegalStateException: Expected 1 instance of jenkins.plugins.logstash.LogstashConfiguration but got 0
	at hudson.ExtensionList.lookupSingleton(ExtensionList.java:454)
	at jenkins.plugins.logstash.LogstashConfiguration.getInstance(LogstashConfiguration.java:290)
	at jenkins.plugins.logstash.LogstashWriter.getIndexerDao(LogstashWriter.java:162)
	at jenkins.plugins.logstash.LogstashWriter.getDaoOrNull(LogstashWriter.java:201)
	at jenkins.plugins.logstash.LogstashWriter.<init>(LogstashWriter.java:82)
	at jenkins.plugins.logstash.pipeline.GlobalDecorator.decorate(GlobalDecorator.java:41)
	at org.jenkinsci.plugins.workflow.log.TaskListenerDecorator.decorateAll(TaskListenerDecorator.java:230)
	at org.jenkinsci.plugins.workflow.log.TaskListenerDecorator$DecoratedTaskListener.getOutputStream(TaskListenerDecorator.java:267)
	at org.jenkinsci.plugins.workflow.log.OutputStreamTaskListener$Default.getLogger(OutputStreamTaskListener.java:116)
	at org.jenkinsci.plugins.workflow.log.TaskListenerDecorator$CloseableTaskListener.getLogger(TaskListenerDecorator.java:307)
	at org.jenkinsci.plugins.gitclient.CliGitAPIImpl.launchCommandIn(CliGitAPIImpl.java:2801)
	at org.jenkinsci.plugins.gitclient.CliGitAPIImpl.launchCommandIn(CliGitAPIImpl.java:2762)
	at org.jenkinsci.plugins.gitclient.CliGitAPIImpl.launchCommandIn(CliGitAPIImpl.java:2757)
	at org.jenkinsci.plugins.gitclient.CliGitAPIImpl.launchCommand(CliGitAPIImpl.java:2051)
	at org.jenkinsci.plugins.gitclient.CliGitAPIImpl.launchCommand(CliGitAPIImpl.java:2063)
	at org.jenkinsci.plugins.gitclient.CliGitAPIImpl.getGitVersion(CliGitAPIImpl.java:259)
	at org.jenkinsci.plugins.gitclient.CliGitAPIImpl.isAtLeastVersion(CliGitAPIImpl.java:301)
	at org.jenkinsci.plugins.gitclient.CliGitAPIImpl$6.execute(CliGitAPIImpl.java:1292)
	at org.jenkinsci.plugins.gitclient.RemoteGitImpl$CommandInvocationHandler$GitCommandMasterToSlaveCallable.call(RemoteGitImpl.java:170)
	at org.jenkinsci.plugins.gitclient.RemoteGitImpl$CommandInvocationHandler$GitCommandMasterToSlaveCallable.call(RemoteGitImpl.java:161)
	at hudson.remoting.UserRequest.perform(UserRequest.java:211)
	at hudson.remoting.UserRequest.perform(UserRequest.java:54)
	at hudson.remoting.Request$2.run(Request.java:377)
	at hudson.remoting.InterceptingExecutorService.lambda$wrap$0(InterceptingExecutorService.java:78)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at hudson.remoting.Engine$1.lambda$newThread$0(Engine.java:125)
	at java.base/java.lang.Thread.run(Thread.java:829)

Seems need to add a filter or a handler for that case...

[kuzaxak]

After latest fix issue got fixed. I tested it in Jenkins and issue seems to be resolved.

[2023-10-16T19:23:41.871Z] Running on pipeline-build-29c37b56-0c94-4871-8bd9-341bd15dbd9d-g82xg-cgzdk in /root/workspace/PR-4385
[2023-10-16T19:23:41.875Z] [Pipeline] {
[2023-10-16T19:23:41.915Z] [Pipeline] ansiColor
[2023-10-16T19:23:41.919Z] [Pipeline] {
[2023-10-16T19:23:41.923Z] 
[2023-10-16T19:23:41.943Z] [Pipeline] stage
[2023-10-16T19:23:41.947Z] [Pipeline] { (Checkout)
[2023-10-16T19:23:41.971Z] [Pipeline] checkout
[2023-10-16T19:23:41.982Z] The recommended git tool is: git
[2023-10-16T19:23:43.144Z] using credential ID
[2023-10-16T19:23:43.153Z] Cloning the remote Git repository
[2023-10-16T19:23:43.154Z] Cloning with configured refspecs honoured and without tags
[logstash-plugin]: Unable to instantiate LogstashIndexerDao with current configuration.
[2023-10-16T19:23:43.368Z] Cloning repository git@github.com:
[2023-10-16T19:23:43.385Z]  > git init /root/workspace/PR-4385 # timeout=10
[2023-10-16T19:23:43.412Z] Fetching upstream changes from git@github.com:
[2023-10-16T19:23:43.413Z]  > git --version # timeout=10
[2023-10-16T19:23:43.417Z]  > git --version # 'git version 2.38.4'
[2023-10-16T19:23:43.417Z] using GIT_SSH to set credentials key for the github account

Now it is reporting only [logstash-plugin]: Unable to instantiate LogstashIndexerDao with current configuration. which is fine I think.

[jakub-bochenski]

Now it is reporting only [logstash-plugin]: Unable to instantiate LogstashIndexerDao with current configuration. which is fine I think.

that doesn't look good, LogstashIndexerDao is the interface used to actually push the log data out

are you getting the job logs still?

[icep87]

@kuzaxak Is is working even if you receive this error message? [logstash-plugin]: Unable to instantiate LogstashIndexerDao with current configuration.

[IlyaFs1]

Hi, we are also suffering from this issue. Is there a plan to fix it?

[duemir]

are you getting the job logs still?

I am guessing most of the logs will still be sent. Only the Git Checkout will be missed since Git Client plugin is a bit special, see https://issues.jenkins.io/browse/JENKINS-30600.

Can this be merged to at least stop the stack traces?

[jakub-bochenski]

Would be nice to hear from OP @kuzaxak ?

[kuzaxak]

@kuzaxak Is is working even if you receive this error message? [logstash-plugin]: Unable to instantiate LogstashIndexerDao with current configuration.

Yes, the plugin works, but it is printing an error every time Jenkins starts the k8s agent

And as mentioned Git Checkout too. So, we are using it in our Jenkins.

[rda1ton]

Any updates on this one? :D

[jakub-bochenski]

Any updates on this one? :D

Sorry, I went on vacation after I wrote the last message and missed the reply.

The error message still worries me, but I don't have time to investigate this.
On the other hand this change seems to help people.

I believe this MR should be available as an incremental release already.
Can more people interested in this change try it out?

[tranceporter]

Hello and thanks for the fix @kuzaxak @jakub-bochenski
I added a comment on the issue above. I think we need to check if the agent is JenkinsJVM, before we call getInstanceOrNull. I might be totally misunderstanding the issue, but just wanted to check. Screenshot below.

[jakub-bochenski]

I don't think it will make the warning go away

Yet per the Javadoc description you quoted:

"If you must do a runtime check whether you are in the controller or agent, use JenkinsJVM rather than this method, as merely loading the Jenkins class file into an agent JVM can cause linkage errors under some conditions."

I think we should be using JenkinsJVM here instead of the current call

[tranceporter]

@jakub-bochenski Agreed. Is it possible to change the code and test? I do not have intimate knowledge of the inner workings of this filter, and as such would prefer not to mess things up.

[jakub-bochenski]

If I'm not mistaken we just need to swap this line https://github.com/jenkinsci/logstash-plugin/pull/138/files#diff-788d74b65c8f1ef69c0b62956c4fa34d8d61dd1e572b388af767de1376e01a45R162

We should probably include this before merging to master.
But for now, can you file a separate MR with this applied so that we have 2 incremental builds to try out?

[tranceporter]

I have created this PR. If we are happy with the change, we can merge it into existing fork, which should update the old PR salemove#1

[jakub-bochenski]

I have created this PR. If we are happy with the change, we can merge it into existing fork, which should update the old PR salemove#1

@kuzaxak can you take a look?

[tranceporter]

@jakub-bochenski @kuzaxak Any update on this please? Would like to get it merged and try it out. Worst case we rollback to previous version of the plugin..

[jakub-bochenski]

@tranceporter I can't merge that PR because it's targeting another repository. Feel free to submit you changes here directly

[tranceporter]

@jakub-bochenski Do you want just my change in the fork PR or do you want me to include @kuzaxak changes as well?

EDIT:

I have created a PR with just my code fix. #147
Seems like build failed and I will need to add the imports statement at the top after all. Fixing.