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

Restore JCasC compatibility for JNLPLauncher.tunnel #8793

Merged
merged 2 commits into from Dec 23, 2023

Conversation

jglick
Copy link
Member

@jglick jglick commented Dec 19, 2023

#8762 seems to have broken JCasC compatibility for agent definitions using tunnel, which is probably unusual but happened to be used by an integration test in CloudBees CI.

I suspect the reason is that DataBoundConfigurator is buggy and does not support @DataBoundSetter on fields, which despite the name is supported by Jenkins core: it never actually looks for this annotation, and perhaps just assumes that getter/setter pairs not covered by the @DataBoundConstructor are properties.

Testing done

jenkinsci/configuration-as-code-plugin#2441

Removal of @Deprecated seems to only be necessary for compatibility with CloudBees CI, which has its own CasC reflection code not based on JCasC and which appears to check for the @Deprecated annotation on fields rather than on setters. (For @cloudbees only: verified running selected tests in the com.cloudbees.casc.server.items package.)

Proposed changelog entries

  • The tunnel property on an inbound agent was inadvertently broken for JCasC usage in 2.437. It remains deprecated and usages should be deleted (regression in 2.437).

Proposed upgrade guidelines

N/A

Maintainer checklist

  • There are at least two (2) approvals for the pull request and no outstanding requests for change.
  • Conversations in the pull request are over, or it is explicit that a reviewer is not blocking the change.
  • Changelog entries in the pull request title and/or Proposed changelog entries are accurate, human-readable, and in the imperative mood.
  • Proper changelog labels are set so that the changelog can be generated automatically.
  • If the change needs additional upgrade steps from users, the upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the pull request title (see example).
  • If it would make sense to backport the change to LTS, a Jira issue must exist, be a Bug or Improvement, and be labeled as lts-candidate to be considered (see query).

timja
timja previously approved these changes Dec 19, 2023
MarkEWaite
MarkEWaite previously approved these changes Dec 19, 2023
@MarkEWaite MarkEWaite added bug For changelog: Minor bug. Will be listed after features ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback labels Dec 19, 2023
@MarkEWaite
Copy link
Contributor

This PR is now ready for merge. We will merge it after approximately 24 hours if there is no negative feedback.

@jglick jglick dismissed stale reviews from MarkEWaite and timja December 20, 2023 18:41

stale

@jglick jglick removed the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Dec 20, 2023
@jglick
Copy link
Member Author

jglick commented Dec 20, 2023

java.lang.OutOfMemoryError: Java heap space on jenkins-war again

@MarkEWaite
Copy link
Contributor

java.lang.OutOfMemoryError: Java heap space on jenkins-war again

Reported to the infra help desk as:

@timja
Copy link
Member

timja commented Dec 22, 2023

/label ready-for-merge


This PR is now ready for merge, after ~24 hours, we will merge it if there's no negative feedback.

Thanks!

@comment-ops-bot comment-ops-bot bot added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Dec 22, 2023
@MarkEWaite MarkEWaite merged commit df03159 into jenkinsci:master Dec 23, 2023
17 checks passed
@jglick jglick deleted the JNLPLauncher.tunnel branch January 2, 2024 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug For changelog: Minor bug. Will be listed after features ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback
Projects
None yet
4 participants