Skip to content

CTM-414: Fix RStudio image detection when JUPYTER_HOME is also present#4907

Open
LizBaldo wants to merge 1 commit intodevelopfrom
CTM-414/enable-custom-rstudio-images
Open

CTM-414: Fix RStudio image detection when JUPYTER_HOME is also present#4907
LizBaldo wants to merge 1 commit intodevelopfrom
CTM-414/enable-custom-rstudio-images

Conversation

@LizBaldo
Copy link
Copy Markdown
Collaborator

@LizBaldo LizBaldo commented May 8, 2026

Jira ticket: https://broadworkbench.atlassian.net/browse/CTM-413

Problem

Custom RStudio environments built on a Jupyter base image fail to launch because Leonardo misclassifies them as Jupyter runtimes.
Leonardo's HttpDockerDAO.detectTool() determines the image type by scanning the container's env vars for either JUPYTER_HOME (Jupyter) or RSTUDIO_HOME (RStudio). The lookup used Map(Jupyter -> "JUPYTER_HOME", RStudio -> "RSTUDIO_HOME") with .find, which in Scala 2.13 iterates in insertion order — meaning Jupyter was always checked first.

When an image inherits JUPYTER_HOME from a Jupyter base image and defines RSTUDIO_HOME in its own Dockerfile, the old code classified it as Jupyter. The misclassified image type was persisted to the DB at creation time, so startup.sh received a populated JUPYTER_DOCKER_IMAGE and an empty RSTUDIO_DOCKER_IMAGE, causing the Jupyter launch path to run against an image that isn't a Jupyter image — resulting in a timeout.

Fix

Change clusterToolEnv from a Map to an ordered List with RStudio first. This makes the priority explicit and independent of any Map implementation details: if both env vars are present, RStudio wins as the more specific signal.

@LizBaldo LizBaldo requested a review from a team as a code owner May 8, 2026 18:06
@LizBaldo LizBaldo requested a review from lucymcnatt May 8, 2026 18:06
@codecov
Copy link
Copy Markdown

codecov Bot commented May 8, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.08%. Comparing base (61186a3) to head (f2e5463).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #4907   +/-   ##
========================================
  Coverage    74.08%   74.08%           
========================================
  Files          131      131           
  Lines        11100    11100           
  Branches       895      895           
========================================
  Hits          8223     8223           
  Misses        2877     2877           
Files with missing lines Coverage Δ
...te/dsde/workbench/leonardo/dao/HttpDockerDAO.scala 88.77% <100.00%> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 61186a3...f2e5463. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Collaborator

@lucymcnatt lucymcnatt left a comment

Choose a reason for hiding this comment

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

makes sense to me!

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