batch connect apps to use a per cluster dataroot (#1409)

apps/dashboard/app/models/batch_connect/session.rb

@@ -97,8 +97,8 @@ class << self
       # The data root directory for this namespace
       # @param token [#to_s] The data root directory for a given app token
       # @return [Pathname] data root directory
-      def dataroot(token = "")
-        OodAppkit.dataroot.join("batch_connect", token.to_s)
+      def dataroot(token = "", cluster: nil)
+        OodAppkit.dataroot.join('batch_connect').join(cluster.to_s).join(token.to_s)
       end
 
       # Root directory for file system database
@@ -215,11 +215,12 @@ def save(app:, context:, format: nil)
       self.title      = app.title
       self.view       = app.session_view
       self.created_at = Time.now.to_i
+      self.cluster_id = context.try(:cluster).to_s
 
       submit_script = app.submit_opts(context, fmt: format, staged_root: staged_root) # could raise an exception
 
-      self.cluster_id = submit_script.fetch(:cluster, context.try(:cluster)).to_s
-      raise(ClusterNotFound, I18n.t('dashboard.batch_connect_missing_cluster')) unless self.cluster_id.present?
+      self.cluster_id = submit_script.fetch(:cluster, cluster_id).to_s
+      raise(ClusterNotFound, I18n.t('dashboard.batch_connect_missing_cluster')) unless cluster_id.present?
 
       stage(app.root.join("template"), context: context) && submit(submit_script)
     rescue => e   # rescue from all standard exceptions (app never crashes)
@@ -418,7 +419,8 @@ def completed?
     # Root directory where a job is staged and run in
     # @return [Pathname] staged root directory
     def staged_root
-      self.class.dataroot(token).join("output", id)
+      c = Configuration.per_cluster_dataroot? ? cluster_id : nil
+      self.class.dataroot(token, cluster: c).join("output", id)
     end
 
     # List of template files that need to be rendered
@@ -544,7 +546,7 @@ def to_hash
       def job_name
         [
           ENV["OOD_PORTAL"],    # the OOD portal id
-          ENV["RAILS_RELATIVE_URL_ROOT"].sub(/^\/[^\/]+\//, ""),  # the OOD app
+          ENV["RAILS_RELATIVE_URL_ROOT"].to_s.sub(/^\/[^\/]+\//, ""),  # the OOD app
           token                 # the Batch Connect app
         ].reject(&:blank?).join("/")
       end

apps/dashboard/config/configuration_singleton.rb

@@ -386,6 +386,14 @@ def bc_dynamic_js?
     end
   end
 
+  def per_cluster_dataroot?
+    if ENV['OOD_PER_CLUSTER_DATAROOT']
+      to_bool(ENV['OOD_PER_CLUSTER_DATAROOT'])
+    else
+      to_bool(config.fetch(:per_cluster_dataroot, false))
+    end
+  end
+
   private
 
   def can_access_core_app?(name)

apps/dashboard/test/models/batch_connect/session_test.rb

@@ -3,7 +3,7 @@
 class BatchConnect::SessionTest < ActiveSupport::TestCase
 
   def bc_jupyter_app
-    r = PathRouter.new("test/fixtures/sys_with_interactive_apps/bc_jupyter")
+    r = PathRouter.new("test/fixtures/sys_with_gateway_apps/bc_jupyter")
     BatchConnect::App.new(router: r)
   end
 
@@ -348,7 +348,7 @@ def completed?
         id: 'test-123',
         status: :running
       )
-      BatchConnect::Session.stubs(:dataroot).returns(Pathname.new(dir))
+      OodAppkit.stubs(:dataroot).returns(Pathname.new(dir))
       BatchConnect::Session.any_instance.stubs(:id).returns('test-id')
       BatchConnect::Session.any_instance.stubs(:info).returns(info)
       session = BatchConnect::Session.new
@@ -371,7 +371,7 @@ def completed?
         'port' => 8080,
         'password' => 'superSecretPassword'
       }
-      BatchConnect::Session.stubs(:dataroot).returns(Pathname.new(dir.to_s))
+      OodAppkit.stubs(:dataroot).returns(Pathname.new(dir.to_s))
       BatchConnect::Session.any_instance.stubs(:id).returns('test-id')
       BatchConnect::Session.any_instance.stubs(:info).returns(info)
       session = BatchConnect::Session.new
@@ -386,7 +386,7 @@ def completed?
 
   test "staged_root does not exist until we call session.stage" do
     Dir.mktmpdir("staged_root") do |dir|
-      BatchConnect::Session.stubs(:dataroot).returns(Pathname.new(dir))
+      OodAppkit.stubs(:dataroot).returns(Pathname.new(dir))
       BatchConnect::Session.any_instance.stubs(:id).returns('test-id')
       OodAppkit.stubs(:clusters).returns([OodCore::Cluster.new({id: 'owens', job: {foo: 'bar'}})])
       session = BatchConnect::Session.new
@@ -430,4 +430,53 @@ def completed?
     Configuration.stubs(:ood_bc_ssh_to_compute_node).returns(false)
     refute session.ssh_to_compute_node?
   end
+
+  test 'saves the cluser id in the staged_root path' do
+    # stub open3 and system apps because :save stages and submits the job
+    # and expects certain things - like a valid cluster.d directory
+    stub_sys_apps
+    Open3.stubs(:capture3).returns(['the-job-id', '', exit_success])
+
+    with_modified_env({ OOD_PER_CLUSTER_DATAROOT: 'true' }) do
+      Dir.mktmpdir('staged_root') do |dir|
+        OodAppkit.stubs(:dataroot).returns(Pathname.new(dir))
+
+        session = BatchConnect::Session.new
+        ctx = bc_jupyter_app.build_session_context
+        ctx.attributes = { 'cluster' => 'owens' }
+
+        assert session.save(app: bc_jupyter_app, context: ctx), session.errors.each { |e| e.to_s }.to_s
+        base_dir = Pathname.new("#{dir}/batch_connect/owens/bc_jupyter/output")
+        assert base_dir.directory?
+        assert_equal 1, base_dir.children.size
+        refute File.directory?("#{dir}/batch_connect/oakley/bc_jupyter/output")
+
+        # now let's switch to the oakley cluster
+        ctx.attributes = { 'cluster' => 'oakley' }
+        assert session.save(app: bc_jupyter_app, context: ctx)
+        base_dir = Pathname.new("#{dir}/batch_connect/oakley/bc_jupyter/output")
+        assert base_dir.directory?
+        assert_equal 1, base_dir.children.size
+      end
+    end
+  end
+
+  test 'no cluster in dataroot by default' do
+    stub_sys_apps
+    Open3.stubs(:capture3).returns(['the-job-id', '', exit_success])
+
+    Dir.mktmpdir('staged_root') do |dir|
+      OodAppkit.stubs(:dataroot).returns(Pathname.new(dir))
+
+      session = BatchConnect::Session.new
+      ctx = bc_jupyter_app.build_session_context
+      ctx.attributes = { 'cluster' => 'owens' }
+
+      assert session.save(app: bc_jupyter_app, context: ctx), session.errors.each { |e| e.to_s }.to_s
+      # owens is not here in the path
+      base_dir = Pathname.new("#{dir}/batch_connect/bc_jupyter/output")
+      assert base_dir.directory?
+      assert_equal 1, base_dir.children.size
+    end
+  end
 end