Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade kubernetes-client-api to 6.8.1 #1342

Merged
merged 21 commits into from
Aug 29, 2023

Conversation

Vlatombe
Copy link
Member

@Vlatombe Vlatombe commented Mar 27, 2023

Downstream of jenkinsci/kubernetes-client-api-plugin#221

Breaking changes (from library):

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

Vlatombe added 3 commits June 23, 2023 17:02
Remove usage of HttpClientUtils.getProxyUrl as it got removed in newer
version of kubernetes-client
@Vlatombe Vlatombe changed the title Upgrade kubernetes-client-api to 6.5.1 Upgrade kubernetes-client-api to 6.8.1 Aug 16, 2023
@julieheard
Copy link

I notice that this plugin has a dependency on kubernetes-credentials-plugin. Do we need to make sure jenkinsci/kubernetes-credentials-plugin#39 is merged and released first, so we can use the release version for that in this plugin?

@Vlatombe
Copy link
Member Author

It's probably safer.

pom.xml Outdated
Comment on lines 82 to 85
<version>0.10.0</version>
<!-- TODO https://github.com/jenkinsci/kubernetes-credentials-plugin/pull/39 @ f62dc9ee13617d84ec899274ed897668a4703242 -->
<version>0.10.1-rc150.f62dc9ee1361</version>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pom.xml Outdated
Comment on lines 273 to 274
<!-- TODO https://github.com/jenkinsci/snakeyaml-api-plugin/pull/94 @ 4960f40749788fcfe6ddc7def8e0bbc0ce937046-->
<version>2.1-110.v4960f4074978</version>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pom.xml Outdated
Comment on lines 61 to 63
<version>6.4.1-215.v2ed17097a_8e9</version>
<version>6.8.1-220.v1f66736f9a_42</version>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Vlatombe
Copy link
Member Author

Vlatombe commented Aug 25, 2023

@jglick Any clue about this new warning raised in org.csanchez.jenkins.plugins.kubernetes.pipeline.KubernetesPipelineTest.runInPod ?

   7.725 [id=1175]	WARNING	o.j.p.w.flow.FlowExecutionList#unregister: Owner[run In Pod/1:run In Pod #1] was not in the list to begin with: []

Looks like it's called twice, and it seems like a normal condition? In that case it should be logged with level FINE ?!

Stacktraces
Breakpoint reached
at org.jenkinsci.plugins.workflow.flow.FlowExecutionList.unregister(FlowExecutionList.java:148)
at org.jenkinsci.plugins.workflow.flow.FlowExecutionList$1.computeNext(FlowExecutionList.java:93)
at org.jenkinsci.plugins.workflow.flow.FlowExecutionList$1.computeNext(FlowExecutionList.java:83)
at com.google.common.collect.AbstractIterator.tryToComputeNext(AbstractIterator.java:146)
at com.google.common.collect.AbstractIterator.hasNext(AbstractIterator.java:141)
at org.jenkinsci.plugins.workflow.flow.FlowExecutionList$StepExecutionIteratorImpl.apply(FlowExecutionList.java:229)
at org.jenkinsci.plugins.workflow.steps.StepExecution.applyAll(StepExecution.java:179)
at org.jenkinsci.plugins.workflow.steps.StepExecution.applyAll(StepExecution.java:187)
at org.jenkinsci.plugins.workflow.steps.durable_task.DurableTaskStep$AgentReconnectionListener.check(DurableTaskStep.java:761)
at org.jenkinsci.plugins.workflow.steps.durable_task.DurableTaskStep$AgentReconnectionListener.onOffline(DurableTaskStep.java:748)
at hudson.slaves.SlaveComputer.lambda$closeChannel$1(SlaveComputer.java:944)
at hudson.slaves.SlaveComputer$$Lambda$1255/0x0000009000dafa28.accept(Unknown Source:-1)
at jenkins.util.Listeners.lambda$notify$0(Listeners.java:59)
at jenkins.util.Listeners$$Lambda$825/0x0000009000a1eef0.run(Unknown Source:-1)
at jenkins.util.Listeners.notify(Listeners.java:67)
at hudson.slaves.SlaveComputer.closeChannel(SlaveComputer.java:944)
at hudson.slaves.SlaveComputer$2.run(SlaveComputer.java:820)
at jenkins.util.ContextResettingExecutorService$1.run(ContextResettingExecutorService.java:28)
at jenkins.security.ImpersonatingExecutorService$1.run(ImpersonatingExecutorService.java:68)
at jenkins.util.ErrorLoggingExecutorService.lambda$wrap$0(ErrorLoggingExecutorService.java:51)
at jenkins.util.ErrorLoggingExecutorService$$Lambda$997/0x0000009000c05238.run(Unknown Source:-1)
at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:539)
at java.util.concurrent.FutureTask.run(FutureTask.java:264)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
at java.lang.Thread.run(Thread.java:833)
Unregistering Owner[run In Pod/1:run In Pod #1]
Breakpoint reached
at org.jenkinsci.plugins.workflow.flow.FlowExecutionList.unregister(FlowExecutionList.java:148)
at org.jenkinsci.plugins.workflow.job.WorkflowRun.finish(WorkflowRun.java:673)
at org.jenkinsci.plugins.workflow.job.WorkflowRun$GraphL.onNewHead(WorkflowRun.java:1069)
at org.jenkinsci.plugins.workflow.cps.CpsFlowExecution.notifyListeners(CpsFlowExecution.java:1530)
at org.jenkinsci.plugins.workflow.cps.CpsThreadGroup$3.run(CpsThreadGroup.java:511)
at org.jenkinsci.plugins.workflow.cps.CpsVmExecutorService$1.run(CpsVmExecutorService.java:38)
at hudson.remoting.SingleLaneExecutorService$1.run(SingleLaneExecutorService.java:139)
at jenkins.util.ContextResettingExecutorService$1.run(ContextResettingExecutorService.java:28)
at jenkins.security.ImpersonatingExecutorService$1.run(ImpersonatingExecutorService.java:68)
at jenkins.util.ErrorLoggingExecutorService.lambda$wrap$0(ErrorLoggingExecutorService.java:51)
at jenkins.util.ErrorLoggingExecutorService$$Lambda$997/0x0000009000c05238.run(Unknown Source:-1)
at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:539)
at java.util.concurrent.FutureTask.run(FutureTask.java:264)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
at java.lang.Thread.run(Thread.java:833)
Unregistering Owner[run In Pod/1:run In Pod #1]

vs. only one call in master

Stacktrace
Breakpoint reached
	at org.jenkinsci.plugins.workflow.flow.FlowExecutionList.unregister(FlowExecutionList.java:133)
	at org.jenkinsci.plugins.workflow.job.WorkflowRun.finish(WorkflowRun.java:673)
	at org.jenkinsci.plugins.workflow.job.WorkflowRun$GraphL.onNewHead(WorkflowRun.java:1069)
	at org.jenkinsci.plugins.workflow.cps.CpsFlowExecution.notifyListeners(CpsFlowExecution.java:1587)
	at org.jenkinsci.plugins.workflow.cps.CpsThreadGroup$3.run(CpsThreadGroup.java:509)
	at org.jenkinsci.plugins.workflow.cps.CpsVmExecutorService$1.run(CpsVmExecutorService.java:38)
	at hudson.remoting.SingleLaneExecutorService$1.run(SingleLaneExecutorService.java:139)
	at jenkins.util.ContextResettingExecutorService$1.run(ContextResettingExecutorService.java:28)
	at jenkins.security.ImpersonatingExecutorService$1.run(ImpersonatingExecutorService.java:68)
	at jenkins.util.ErrorLoggingExecutorService.lambda$wrap$0(ErrorLoggingExecutorService.java:51)
	at jenkins.util.ErrorLoggingExecutorService$$Lambda$945/0x000000b000c12550.run(Unknown Source:-1)
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:539)
	at java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
	at java.lang.Thread.run(Thread.java:833)

Seems related to jenkinsci/workflow-durable-task-step-plugin#323, combined with https://github.com/jenkinsci/workflow-api-plugin/blob/2c63e4d244199a60ac7003c9b8813f36aad5f638/src/main/java/org/jenkinsci/plugins/workflow/flow/FlowExecutionList.java#L83-L104 which has a side-effect of calling FlowExecutionList#unregister in some cases.

@Vlatombe
Copy link
Member Author

jenkinsci/workflow-api-plugin#304 seems to fix it

@jglick
Copy link
Member

jglick commented Aug 25, 2023

Judging by the stack traces, it looks like a race condition: depending on timing, the AgentReconnectionListener may or may not call StepExecution.applyAll after the FlowExecution.complete but before WorkflowRun.finish gets to that point. Indeed we do not need to unregister anything from StepExecution.applyAll.

pom.xml Outdated Show resolved Hide resolved
pom.xml Outdated Show resolved Hide resolved
@Vlatombe Vlatombe marked this pull request as ready for review August 29, 2023 13:50
@Vlatombe Vlatombe requested a review from a team as a code owner August 29, 2023 13:50
@Vlatombe Vlatombe added the enhancement Improvements label Aug 29, 2023
@Vlatombe Vlatombe enabled auto-merge August 29, 2023 14:00
@Vlatombe Vlatombe merged commit 5712230 into jenkinsci:master Aug 29, 2023
@Vlatombe Vlatombe deleted the kubernetes-client-api-6.5.1 branch August 29, 2023 14:18
@amuniz
Copy link
Member

amuniz commented Oct 10, 2023

As part of the snakeyaml (transitive) update a breaking change was introduced. The new snakeyaml-engine is implementing YAML 1.2.2, in which the merge operator (<<) is not defined anymore (it was removed from the spec). So any yaml using it will fail to parse.

I think it's worth noting in release notes @Vlatombe?

@Vlatombe
Copy link
Member Author

Do you have a concrete use case for it ? I've never seen this operator being used in the context of this plugin

@Vlatombe
Copy link
Member Author

Probably a general mention that snakeyaml-engine implements a new version of the yaml spec, and so anything that was relying on specifics of yaml 1.1 that got changed or removed in yaml 1.2 no longer works.

@Vlatombe
Copy link
Member Author

@vicecea
Copy link

vicecea commented Jan 18, 2024

Facing the same issues as https://issues.jenkins.io/browse/JENKINS-71956
Any chance we can get an additional option that is backwards compatible?

Would there be a way to fix this in snakeyaml-engine ?

Thanks for any response!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants