Skip to content

RUN-4780: Clarify Groovy plugin hot reload requires plugin.refreshDelay - #1952

Merged
elquimeras merged 6 commits into
4.0.xfrom
RUN-4780-doc-hot-reload-flag
Sep 3, 2026
Merged

RUN-4780: Clarify Groovy plugin hot reload requires plugin.refreshDelay#1952
elquimeras merged 6 commits into
4.0.xfrom
RUN-4780-doc-hot-reload-flag

Conversation

@elquimeras

@elquimeras elquimeras commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes RUN-4780 ("Hot reloading groovy plugin is not working"). The docs claimed Groovy plugins hot-reload automatically after their initial load; in practice Rundeck never re-checks a loaded Groovy plugin unless plugin.refreshDelay is explicitly set (root cause: resources.groovy defaults the Spring refresh-check-delay to -1, disabling the refresh check entirely — this has been the behavior since the feature was introduced, not a regression).

  • Hot reloading is now documented as opt-in via the plugin.refreshDelay JVM system property.
  • Added an "Enabling Hot Reloading" subsection pointing to the canonical System Properties Configuration page for RPM/DEB setup, plus a Docker note.
  • Added a caution that enabling this makes Rundeck watch every Groovy plugin in libext, not just the one being edited, and can break startup if any of them are empty or invalid.

Changes

  • docs/developer/groovy-plugin-development.md — corrected the Overview bullet and rewrote the "Iterative Development" section under Development Workflow.
  • .gitignore — ignore local .atl/ directory.
image

…eshDelay

RUN-4780: Rundeck never reloads a Groovy plugin after initial load by
default. Hot reloading requires setting plugin.refreshDelay explicitly;
document activation per install type (RPM, DEB, Docker, bootRun) and
warn that enabling it watches every plugin in libext.
…eshDelay

RUN-4780: Rundeck never reloads a Groovy plugin after initial load by
default. Hot reloading requires setting plugin.refreshDelay explicitly;
link to the canonical System Properties Configuration page for
install-specific setup, and warn that enabling it watches every plugin
in libext.
@elquimeras
elquimeras requested review from a team and a lite review from Copilot September 2, 2026 18:05
@elquimeras elquimeras self-assigned this Sep 2, 2026
@elquimeras elquimeras added this to the 6.2.0 milestone Sep 2, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The new documentation includes ambiguous Docker guidance and a potentially unsafe RPM/DEB example that can overwrite existing RDECK_JVM_OPTS settings.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR updates the Rundeck documentation to clarify that Groovy plugin hot reloading is not automatic after initial load, and requires explicitly enabling refresh checks via the plugin.refreshDelay JVM system property. It also adds a local ignore entry for a .atl/ directory.

Changes:

  • Clarifies Groovy plugin “Hot Reloading” as an opt-in capability in the Overview section.
  • Reworks the iterative development workflow to document enabling hot reload via -Dplugin.refreshDelay=..., including cautions about watching all Groovy plugins in libext.
  • Updates .gitignore to ignore a local .atl/ directory.
File summaries
File Description
docs/developer/groovy-plugin-development.md Clarifies hot reload behavior and adds an opt-in configuration subsection + caveats for Groovy plugin development.
.gitignore Ignores local .atl/ directory artifacts.
Review details
  • Files reviewed: 1/2 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread docs/developer/groovy-plugin-development.md Outdated
Comment thread docs/developer/groovy-plugin-development.md Outdated
@elquimeras elquimeras modified the milestones: 6.2.0, 6.3.0 Sep 2, 2026
Use the append pattern for RDECK_JVM_OPTS to avoid overwriting an
existing value, and replace the unverified Docker command flag with a
link to Extending Docker Configuration since the official images
configure the JVM via env vars/templates, not direct -D passthrough.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The documentation updates align with existing configuration guidance in the repo and introduce no apparent inconsistencies or incomplete changes.

Review details
  • Files reviewed: 1/2 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@luismalamoc

Copy link
Copy Markdown

A few follow-ups from review (claims verified against resources.groovy in rundeck/rundeck — refresh-check-delay defaults to -1 and applies to every .groovy in libext, so the core correction in this PR is accurate):

  1. Config-file alternative to JVM opts: the value is read via application.config.getProperty("plugin.refreshDelay", ...), i.e. the merged Grails config — so a plain plugin.refreshDelay=5000 line in rundeck-config.properties should work too. Worth a quick local test; if it works, documenting it as the primary method would simplify the Docker note considerably (no derived image needed).
  2. Startup caveat precision: a broken or empty .groovy file in libext may fail startup even without plugin.refreshDelay set (the script beans are defined at startup either way). If you verified breakage only happens with refresh enabled, ignore this; otherwise consider softening the warning to "broken or empty .groovy files in libext can prevent startup".
  3. Snyk check failing: diff is markdown-only, so this is almost certainly repo baseline rather than this PR — but it leaves the PR blocked. Worth confirming against another recent PR before merge.

…e hot reload

Verified against rundeckapp source: resources.groovy reads
plugin.refreshDelay via application.config.getProperty, and
Application.groovy registers rundeck-config.properties as a
Spring PropertySource with addFirst, giving it higher precedence
than JVM system properties. It's also the one config file present
across every install type, simplifying the Docker case raised in
review without needing a derived image.
RUN-4780 review (luismalamoc, section 2): resources.groovy defines the
lang:groovy bean for every .groovy in libext unconditionally, and
Spring eagerly instantiates non-lazy singletons at context startup
regardless of refresh-check-delay. A broken/empty .groovy file can
break startup whether or not plugin.refreshDelay is set - the warning
no longer implies the risk is exclusive to enabling it.
@elquimeras

elquimeras commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

@luismalamoc Thanks for the detailed review. Verified all three against rundeckapp source (local checkout of rundeck/rundeck):

  1. Config-file alternative: confirmed — Application.groovy registers rundeck-config.properties as a Spring PropertySource with addFirst, so it has higher precedence than JVM system properties, and it's the config file present in every install type. Documented it as the primary method in 68cb382 (JVM opts kept as an alternative).
  2. Startup caveat precision: confirmed via source, not empirical testing — resources.groovy defines the lang:groovy bean for every .groovy file in libext unconditionally, and these are eagerly-instantiated singletons (no lazy-init), so a broken/empty file can break startup regardless of plugin.refreshDelay. Softened the warning in cf0587b to not imply the risk is exclusive to enabling it.
  3. Snyk check: leaving this out of scope for this PR per the team's call — will be looked at separately.

Ready for another pass whenever you have a chance.

@luismalamoc luismalamoc left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verified both follow-ups are addressed:

  1. rundeck-config.properties documented as the primary method (68cb382) — confirmed against Application.groovy (addFirst property source, takes precedence) and the linked Config File Reference page exists on 4.0.x.
  2. Startup warning softened (cf0587b) to reflect that broken/empty .groovy files in libext are a startup risk regardless of plugin.refreshDelay.

Snyk failure is repo baseline, tracked separately per team's call. LGTM.

@elquimeras
elquimeras merged commit 607de12 into 4.0.x Sep 3, 2026
2 of 4 checks passed
@elquimeras
elquimeras deleted the RUN-4780-doc-hot-reload-flag branch September 3, 2026 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants