[ARVADOS] created: f9b0d35bd35c3acb496d3a2afa1c1290f1ed7feb

git at public.curoverse.com git at public.curoverse.com
Mon Sep 22 11:11:12 EDT 2014


        at  f9b0d35bd35c3acb496d3a2afa1c1290f1ed7feb (commit)


commit f9b0d35bd35c3acb496d3a2afa1c1290f1ed7feb
Author: Brett Smith <brett at curoverse.com>
Date:   Mon Sep 22 11:11:28 2014 -0400

    2881: crunch-dispatch updates nodes' job assignments.
    
    With this commit, crunch-dispatch updates all the information that
    update_node_attributes used to, so it's removed.

diff --git a/services/api/script/crunch-dispatch.rb b/services/api/script/crunch-dispatch.rb
index 08ea229..44d8a0a 100755
--- a/services/api/script/crunch-dispatch.rb
+++ b/services/api/script/crunch-dispatch.rb
@@ -61,6 +61,10 @@ class Dispatcher
     end
   end
 
+  def crunch_wrapper_is_slurm?
+    Server::Application.config.crunch_job_wrapper.to_s.match /^slurm/
+  end
+
   def sinfo
     @@slurm_version ||= Gem::Version.new(`sinfo --version`.match(/\b[\d\.]+\b/)[0])
     if Gem::Version.new('2.3') <= @@slurm_version
@@ -85,7 +89,7 @@ class Dispatcher
   end
 
   def update_node_status
-    if Server::Application.config.crunch_job_wrapper.to_s.match /^slurm/
+    if crunch_wrapper_is_slurm?
       @node_state ||= {}
       node_seen = {}
       begin
@@ -108,6 +112,9 @@ class Dispatcher
             if node
               $stderr.puts "dispatch: update #{node_name} state to #{node_state}"
               node.info['slurm_state'] = node_state
+              if node_state == "idle"
+                node.job = nil
+              end
               if not node.save
                 $stderr.puts "dispatch: failed to update #{node.uuid}: #{node.errors.messages}"
               end
@@ -141,7 +148,7 @@ class Dispatcher
   def nodes_available_for_job_now(job)
     # Find Nodes that satisfy a Job's runtime constraints (by building
     # a list of Procs and using them to test each Node).  If there
-    # enough to run the Job, return an array of their names.
+    # enough to run the Job, return an array of usable node objects.
     # Otherwise, return nil.
     need_procs = NODE_CONSTRAINT_MAP.each_pair.map do |job_key, node_key|
       Proc.new do |node|
@@ -157,7 +164,7 @@ class Dispatcher
       if good_node
         usable_nodes << node
         if usable_nodes.count >= min_node_count
-          return usable_nodes.map { |node| node.hostname }
+          return usable_nodes
         end
       end
     end
@@ -185,6 +192,16 @@ class Dispatcher
     nodelist
   end
 
+  def assign_job_to_nodes(assigned_job, nodes)
+    nodes.each do |node|
+      node.job = assigned_job
+      if not node.save
+        $stderr.puts("dispatch: failed to save #{node.uuid} assignment to " +
+                     "job #{assigned_job.uuid}: #{node.errors.messages}")
+      end
+    end
+  end
+
   def start_jobs
     @todo.each do |job|
       next if @running[job.uuid]
@@ -212,7 +229,7 @@ class Dispatcher
                     "--exclusive",
                     "--no-kill",
                     "--job-name=#{job.uuid}",
-                    "--nodelist=#{nodelist.join(',')}"]
+                    "--nodelist=#{nodelist.map(&:hostname).join(',')}"]
       else
         raise "Unknown crunch_job_wrapper: #{Server::Application.config.crunch_job_wrapper}"
       end
@@ -306,7 +323,10 @@ class Dispatcher
         log_truncated: false
       }
       i.close
-      update_node_status
+      if crunch_wrapper_is_slurm?
+        assign_job_to_nodes(job, nodelist)
+        update_node_status
+      end
     end
   end
 
diff --git a/services/api/script/update_node_attributes.rb b/services/api/script/update_node_attributes.rb
deleted file mode 100755
index 2a0bccc..0000000
--- a/services/api/script/update_node_attributes.rb
+++ /dev/null
@@ -1,106 +0,0 @@
-#!/usr/bin/env ruby
-
-# Keep node.info[:running_job_uuid] and node.info[:slurm_state] up to date.
-#
-# use:     script/update_node_attributes.rb [rails_env] [update_interval]
-# example: script/update_node_attributes.rb production 10
-
-ENV["RAILS_ENV"] = ARGV[0] || "development"
- at update_interval = ARGV[1] ? ARGV[1].to_i : 5
-
-require File.dirname(__FILE__) + '/../config/boot'
-require File.dirname(__FILE__) + '/../config/environment'
-
-include ApplicationHelper
-act_as_system_user
-
- at slurm_state = {}
- at running_job_uuid = {}
-
-while true
-  IO.popen('sinfo --noheader --Node || true').readlines.each do |line|
-    tokens = line.strip.split
-    nodestate = tokens.last.downcase
-
-    nodenames = []
-    if (re = tokens.first.match /^([^\[]*)\[([-\d,]+)\]$/)
-      nodeprefix = re[1]
-      re[2].split(',').each do |number_range|
-        if number_range.index('-')
-          range = number_range.split('-').collect(&:to_i)
-          (range[0]..range[1]).each do |n|
-            nodenames << "#{nodeprefix}#{n}"
-          end
-        else
-          nodenames << "#{nodeprefix}#{number_range}"
-        end
-      end
-    else
-      nodenames << tokens.first
-    end
-
-    nodenames.each do |nodename|
-      if @slurm_state[nodename] != nodestate
-        has_no_job = ! ['alloc','comp'].index(nodestate)
-        node = Node.
-          where('slot_number=? and hostname=?',
-                nodename.match(/(\d+)$/)[1].to_i,
-                nodename).
-          first
-        raise "Fatal: Node does not exist: #{nodename}" if !node
-
-        puts "Node #{node.uuid} slot #{node.slot_number} name #{node.hostname} state #{nodestate}#{' (has_no_job)' if has_no_job}"
-        node_info_was = node.info.dup
-        node.info[:slurm_state] = nodestate
-        node.info[:running_job_uuid] = nil if has_no_job
-        if node_info_was != node.info and not node.save
-          raise "Fail: update node #{node.uuid} state #{nodestate}"
-        end
-        @slurm_state[nodename] = nodestate
-      end
-    end
-  end
-
-  IO.popen('squeue --noheader --format="%j %t %N" || true').readlines.each do |line|
-    tokens = line.strip.split
-    running_job_uuid = tokens.first
-
-    nodenames = []
-    if (re = tokens.last.match /^([^\[]*)\[([-\d,]+)\]$/)
-      nodeprefix = re[1]
-      re[2].split(',').each do |number_range|
-        if number_range.index('-')
-          range = number_range.split('-').collect(&:to_i)
-          (range[0]..range[1]).each do |n|
-            nodenames << "#{nodeprefix}#{n}"
-          end
-        else
-          nodenames << "#{nodeprefix}#{number_range}"
-        end
-      end
-    else
-      nodenames << tokens.first
-    end
-
-    nodenames.each do |nodename|
-      if @running_job_uuid[nodename] != running_job_uuid
-        node = Node.
-          where('slot_number=? and hostname=?',
-                nodename.match(/(\d+)$/)[1].to_i,
-                nodename).
-          first
-        raise "Fatal: Node does not exist: #{nodename}" if !node
-        puts "Node #{node.uuid} slot #{node.slot_number} name #{node.hostname} running_job_uuid #{running_job_uuid}"
-        if node.info[:running_job_uuid] != running_job_uuid
-          node.info[:running_job_uuid] = running_job_uuid
-          if not node.save
-            raise "Fail: update node #{node.uuid} running_job_uuid #{running_job_uuid}"
-          end
-        end
-        @running_job_uuid[nodename] = running_job_uuid
-      end
-    end
-  end
-
-  sleep @update_interval
-end

commit 161cd314791afe35fda63906755ad8375300e898
Author: Brett Smith <brett at curoverse.com>
Date:   Mon Sep 22 11:10:42 2014 -0400

    2881: API server associates nodes with their assigned jobs.
    
    This will enable us to write better administrative tools (like the
    Node Manager) and dashboards.

diff --git a/doc/api/methods/nodes.html.textile.liquid b/doc/api/methods/nodes.html.textile.liquid
index 2fb77e0..f846086 100644
--- a/doc/api/methods/nodes.html.textile.liquid
+++ b/doc/api/methods/nodes.html.textile.liquid
@@ -76,3 +76,5 @@ table(table table-bordered table-condensed).
 |_. Argument |_. Type |_. Description |_. Location |_. Example |
 {background:#ccffcc}.|uuid|string|The UUID of the Node in question.|path||
 |node|object||query||
+
+To update a node's job assignment, set the node object's @job_id@ to the UUID of the job, or null to clear the assignment.
diff --git a/doc/api/schema/Job.html.textile.liquid b/doc/api/schema/Job.html.textile.liquid
index 513c105..5418834 100644
--- a/doc/api/schema/Job.html.textile.liquid
+++ b/doc/api/schema/Job.html.textile.liquid
@@ -34,6 +34,7 @@ See "Script versions":#script_version below for more detail about acceptable way
 |running|boolean|Whether the job is running||
 |success|boolean|Whether the job indicated successful completion|Is null if job has not finished|
 |is_locked_by_uuid|string|UUID of the user who has locked this job|Is null if job is not locked. The system user locks the job when starting the job, in order to prevent job attributes from being altered.|
+|nodes|array|List of node objects that have been assigned to this job|Refer to the "Node schema":{{site.baseurl}}/api/schema/Node.html.|
 |log|string|Collection UUID|Is null if the job has not finished. After the job runs, the given collection contains a text file with log messages provided by the @arv-crunch-job@ task scheduler as well as the standard error streams provided by the task processes.|
 |tasks_summary|hash|Summary of task completion states.|Example: @{"done":0,"running":4,"todo":2,"failed":0}@|
 |output|string|Collection UUID|Is null if the job has not finished.|
diff --git a/doc/api/schema/Node.html.textile.liquid b/doc/api/schema/Node.html.textile.liquid
index 6f576bf..e373751 100644
--- a/doc/api/schema/Node.html.textile.liquid
+++ b/doc/api/schema/Node.html.textile.liquid
@@ -22,6 +22,7 @@ table(table table-bordered table-condensed).
 |hostname|string|||
 |domain|string|||
 |ip_address|string|||
+|job|hash|The job that this node is assigned to work on.  This field is null if the node is idle.  Refer to the "Job schema":{{site.baseurl}}/api/schema/Job.html for details.  If you do not have permission to read the job, you will only see its @running@ attribute.||
 |first_ping_at|datetime|||
 |last_ping_at|datetime|||
 |info|hash|||
diff --git a/services/api/app/controllers/arvados/v1/nodes_controller.rb b/services/api/app/controllers/arvados/v1/nodes_controller.rb
index 5bfeff0..7cb75f5 100644
--- a/services/api/app/controllers/arvados/v1/nodes_controller.rb
+++ b/services/api/app/controllers/arvados/v1/nodes_controller.rb
@@ -10,6 +10,20 @@ class Arvados::V1::NodesController < ApplicationController
     show
   end
 
+  def update
+    # Translate a job UUID to its numeric ID for the association,
+    # and mark the job readable if we found anything.
+    if resource_attrs[:job_id]
+      search_key = (resource_attrs[:job_id] =~ /\D/) ? :uuid : :id
+      resource_attrs[:job_id] = Job.
+        readable_by(*@read_users).
+        where(search_key => resource_attrs.delete(:job_id)).
+        first.andand.id
+      @object.job_readable = !!resource_attrs[:job_id]
+    end
+    super
+  end
+
   def self._ping_requires_parameters
     { ping_secret: true }
   end
@@ -45,5 +59,12 @@ class Arvados::V1::NodesController < ApplicationController
       # recently) working
       @objects = model_class.where('last_ping_at >= ?', Time.now - 1.hours)
     end
+    assoc_jobs = Job.
+      readable_by(*@read_users).
+      where(id: @objects.map(&:job_id)).
+      map(&:id)
+    @objects.select(&:job_id).each do |node|
+      node.job_readable = assoc_jobs.include?(node.job_id)
+    end
   end
 end
diff --git a/services/api/app/models/job.rb b/services/api/app/models/job.rb
index 12f82be..7b2942e 100644
--- a/services/api/app/models/job.rb
+++ b/services/api/app/models/job.rb
@@ -16,6 +16,7 @@ class Job < ArvadosModel
   before_save :set_state_before_save
 
   has_many :commit_ancestors, :foreign_key => :descendant, :primary_key => :script_version
+  has_many :nodes
 
   class SubmitIdReused < StandardError
   end
@@ -45,8 +46,12 @@ class Job < ArvadosModel
     t.add :supplied_script_version
     t.add :docker_image_locator
     t.add :queue_position
+    t.add :nodes
     t.add :description
   end
+  # Nodes have a superuser template for accessing sensitive information.
+  # We need one too so those requests can see associated Job information.
+  api_accessible :superuser, extend: :user
 
   # Supported states for a job
   States = [
diff --git a/services/api/app/models/node.rb b/services/api/app/models/node.rb
index 45fd3b1..0a8358c 100644
--- a/services/api/app/models/node.rb
+++ b/services/api/app/models/node.rb
@@ -7,6 +7,13 @@ class Node < ArvadosModel
   before_validation :ensure_ping_secret
   after_update :dnsmasq_update
 
+  # Only a controller can figure out whether or not the current API tokens
+  # have access to the associated Job.  They're expected to set
+  # job_readable=true if they want full Job information to be included in the
+  # API response.
+  belongs_to :job
+  attr_accessor :job_readable
+
   MAX_SLOTS = 64
 
   @@confdir = Rails.configuration.dnsmasq_conf_dir
@@ -20,6 +27,7 @@ class Node < ArvadosModel
     t.add :last_ping_at
     t.add :slot_number
     t.add :status
+    t.add :job
     t.add :crunch_worker_state
     t.add :properties
   end
@@ -33,6 +41,15 @@ class Node < ArvadosModel
     super || @@domain
   end
 
+  def job
+    db_job = super
+    if db_job and not job_readable
+      {"running" => true}
+    else
+      db_job
+    end
+  end
+
   def crunch_worker_state
     case self.info.andand['slurm_state']
     when 'alloc', 'comp'
diff --git a/services/api/db/migrate/20140919152716_add_job_id_to_nodes.rb b/services/api/db/migrate/20140919152716_add_job_id_to_nodes.rb
new file mode 100644
index 0000000..7fd8677
--- /dev/null
+++ b/services/api/db/migrate/20140919152716_add_job_id_to_nodes.rb
@@ -0,0 +1,7 @@
+class AddJobIdToNodes < ActiveRecord::Migration
+  def change
+    change_table :nodes do |t|
+      t.references :job
+    end
+  end
+end
diff --git a/services/api/db/structure.sql b/services/api/db/structure.sql
index ca87b1b..e927c51 100644
--- a/services/api/db/structure.sql
+++ b/services/api/db/structure.sql
@@ -641,7 +641,8 @@ CREATE TABLE nodes (
     last_ping_at timestamp without time zone,
     info text,
     updated_at timestamp without time zone NOT NULL,
-    properties text
+    properties text,
+    job_id integer
 );
 
 
@@ -683,9 +684,9 @@ CREATE TABLE pipeline_instances (
     properties text,
     state character varying(255),
     components_summary text,
-    description text,
     started_at timestamp without time zone,
-    finished_at timestamp without time zone
+    finished_at timestamp without time zone,
+    description text
 );
 
 
@@ -2027,3 +2028,5 @@ INSERT INTO schema_migrations (version) VALUES ('20140918141529');
 INSERT INTO schema_migrations (version) VALUES ('20140918153541');
 
 INSERT INTO schema_migrations (version) VALUES ('20140918153705');
+
+INSERT INTO schema_migrations (version) VALUES ('20140919152716');
\ No newline at end of file
diff --git a/services/api/test/fixtures/nodes.yml b/services/api/test/fixtures/nodes.yml
index 92e78da..bee7472 100644
--- a/services/api/test/fixtures/nodes.yml
+++ b/services/api/test/fixtures/nodes.yml
@@ -6,6 +6,7 @@ busy:
   ip_address: 172.17.2.172
   last_ping_at: <%= 1.minute.ago.to_s(:db) %>
   first_ping_at: <%= 23.hour.ago.to_s(:db) %>
+  job: nearly_finished_job
   info:
     :ping_secret: "48dpm3b8ijyj3jkr2yczxw0844dqd2752bhll7klodvgz9bg80"
     :slurm_state: "alloc"
@@ -18,6 +19,7 @@ down:
   ip_address: 172.17.2.173
   last_ping_at: <%= 1.hour.ago.to_s(:db) %>
   first_ping_at: <%= 23.hour.ago.to_s(:db) %>
+  job: nil
   info:
     :ping_secret: "2k3i71depad36ugwmlgzilbi4e8n0illb2r8l4efg9mzkb3a1k"
 
@@ -29,6 +31,7 @@ idle:
   ip_address: 172.17.2.174
   last_ping_at: <%= 2.minute.ago.to_s(:db) %>
   first_ping_at: <%= 23.hour.ago.to_s(:db) %>
+  job: nil
   info:
     :ping_secret: "69udawxvn3zzj45hs8bumvndricrha4lcpi23pd69e44soanc0"
     :slurm_state: "idle"
diff --git a/services/api/test/functional/arvados/v1/jobs_controller_test.rb b/services/api/test/functional/arvados/v1/jobs_controller_test.rb
index 546225e..914f365 100644
--- a/services/api/test/functional/arvados/v1/jobs_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/jobs_controller_test.rb
@@ -317,4 +317,27 @@ class Arvados::V1::JobsControllerTest < ActionController::TestCase
     end
   end
 
+  test "node assignments with basic info available for the job" do
+    authorize_with :active
+    get :show, {id: jobs(:nearly_finished_job).uuid}
+    assert_response :success
+    nodes = json_response["nodes"]
+    assert_equal([nodes(:busy).uuid], nodes.map { |n| n["uuid"] })
+    # Make sure the node information does not include superuser fields.
+    refute(nodes.any? { |n| n.has_key?("info") },
+           "non-admin can see privileged node information")
+  end
+
+  test "admin has access to superuser fields in node assignment list" do
+    authorize_with :admin
+    get :show, {id: jobs(:nearly_finished_job).uuid}
+    assert_response :success
+    node_fixture = nodes(:busy)
+    busy_node = json_response["nodes"].
+      select { |n| n["uuid"] == node_fixture.uuid }.first
+    assert_not_nil(busy_node, "assigned node not found in response")
+    assert_not_nil(busy_node["info"], "node info field missing in response")
+    assert_equal(node_fixture.info["ping_secret"],
+                 busy_node["info"]["ping_secret"])
+  end
 end
diff --git a/services/api/test/functional/arvados/v1/nodes_controller_test.rb b/services/api/test/functional/arvados/v1/nodes_controller_test.rb
index 3143a2e..fdea25b 100644
--- a/services/api/test/functional/arvados/v1/nodes_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/nodes_controller_test.rb
@@ -93,4 +93,70 @@ class Arvados::V1::NodesControllerTest < ActionController::TestCase
     assert_equal(1024, properties['total_ram_mb'].to_i)
     assert_equal(2048, properties['total_scratch_mb'].to_i)
   end
+
+  test "active user can see their assigned job" do
+    authorize_with :active
+    get :show, {id: nodes(:busy).uuid}
+    assert_response :success
+    assert_equal(jobs(:nearly_finished_job).uuid,
+                 json_response["job"].andand["uuid"])
+  end
+
+  test "user without job read permission only sees job running state" do
+    authorize_with :spectator
+    get :show, {id: nodes(:busy).uuid}
+    assert_response :success
+    node_job = json_response["job"] || {}
+    assert(node_job.delete("running"),
+           "spectator can't see that node has a job assigned")
+    assert_empty(node_job,
+                 "spectator can see details about node's assigned job")
+  end
+
+  test "admin can associate a job with a node" do
+    changed_node = nodes(:idle)
+    assigned_job = jobs(:queued)
+    authorize_with :admin
+    post :update, {
+      id: changed_node.uuid,
+      node: {job_id: assigned_job.uuid},
+    }
+    assert_response :success
+    assert_equal(changed_node.hostname, json_response["hostname"],
+                 "hostname mismatch after defining job")
+    assert_equal(assigned_job.uuid, json_response["job"].andand["uuid"],
+                 "mismatch in node's assigned job UUID")
+  end
+
+  test "non-admin can't associate a job with a node" do
+    authorize_with :active
+    post :update, {
+      id: nodes(:idle).uuid,
+      node: {job_id: jobs(:queued).uuid},
+    }
+    assert_response 403
+  end
+
+  test "admin can unassign a job from a node" do
+    changed_node = nodes(:busy)
+    authorize_with :admin
+    post :update, {
+      id: changed_node.uuid,
+      node: {job_id: nil},
+    }
+    assert_response :success
+    assert_equal(changed_node.hostname, json_response["hostname"],
+                 "hostname mismatch after defining job")
+    assert_nil(json_response["job"],
+               "node still has job assignment after update")
+  end
+
+  test "non-admin can't unassign a job from a node" do
+    authorize_with :project_viewer
+    post :update, {
+      id: nodes(:busy).uuid,
+      node: {job_id: nil},
+    }
+    assert_response 403
+  end
 end

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list