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

[JENKINS-72441] Prevent lock contention on KubernetesLauncher#isLaunchSupported #1479

Merged
merged 1 commit into from
Dec 8, 2023

Conversation

Dohbedoh
Copy link
Contributor

@Dohbedoh Dohbedoh commented Dec 8, 2023

JENKINS-72441

As per #1392 (comment)

Remove the synchronized - that was added to fix a spotbug warning - causing lock contention on accessors. And use an AtomicBoolean instead to make spotbugs happy.

Testing done

Testing agent low to launch

  • Create a pipeline:
pipeline {
    agent none
    stages {
        stage('Hello') {
            steps {
                podTemplate(envVars: [envVar(key: 'JENKINS_TUNNEL', value: ':50999')]) {
                    node(POD_LABEL) {
                        // some block
                        sh "echo 'Hello World'"
                    }
                }
            }
        }
    }
}
  • Run it
  • Follow the link to the agent created from the console out
  • Validate that the agent page is accessible

Testing normal launch

  • Create a pipeline:
pipeline {
    agent none
    stages {
        stage('Hello') {
            steps {
                podTemplate {
                    node(POD_LABEL) {
                        // some block
                        sh "echo 'Hello World'"
                    }
                }
            }
        }
    }
}
  • Run it
  • Follow the link to the agent created from the console out
  • Validate that the agent page is accessible and that the build completes

Submitter checklist

Preview Give feedback

@Dohbedoh Dohbedoh requested a review from a team as a code owner December 8, 2023 12:41
@Vlatombe Vlatombe changed the title [JENKINS-72441] Prevent lock contention on KubernetesLauncher#isLaunc… [JENKINS-72441] Prevent lock contention on KubernetesLauncher#isLaunchSupported Dec 8, 2023
@Vlatombe Vlatombe added the bug Bug Fixes label Dec 8, 2023
@Vlatombe Vlatombe enabled auto-merge December 8, 2023 14:05
@Vlatombe Vlatombe merged commit 6faf0fb into jenkinsci:master Dec 8, 2023
5 checks passed
@Dohbedoh Dohbedoh deleted the JENKINS-72441 branch December 8, 2023 15:28
@@ -69,7 +70,7 @@ public class KubernetesLauncher extends JNLPLauncher {

private static final Logger LOGGER = Logger.getLogger(KubernetesLauncher.class.getName());

private boolean launched;
private final AtomicBoolean launched = new AtomicBoolean(false);
Copy link
Member

Choose a reason for hiding this comment

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

This is not OK—the field is not transient so it would be persisted in the XStream form of the node config. Granted these agents are usually ephemeral, but they do survive controller restarts and plugin upgrades.

I would suggest restoring it to boolean and just make it volatile to shut up SpotBugs.

@jglick
Copy link
Member

jglick commented May 9, 2024

What does this have to do with https://issues.jenkins.io/browse/JENKINS-72441 in artifact-manager-s3? Mistyped issue number?

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

Successfully merging this pull request may close these issues.

3 participants