[ARVADOS] updated: da0d61f9a95848d47d28462d89e0a2070c39d587

git at public.curoverse.com git at public.curoverse.com
Wed Sep 24 15:32:44 EDT 2014


Summary of changes:
 doc/api/methods/nodes.html.textile.liquid          |   2 +
 doc/api/schema/Job.html.textile.liquid             |   1 +
 doc/api/schema/Node.html.textile.liquid            |   1 +
 .../app/controllers/arvados/v1/nodes_controller.rb |  18 ++++
 services/api/app/models/job.rb                     |   6 ++
 services/api/app/models/node.rb                    |  11 +++
 .../20140924091559_add_job_uuid_to_nodes.rb        |  13 +++
 services/api/db/structure.sql                      |   9 +-
 services/api/script/crunch-dispatch.rb             |  81 ++++++++--------
 services/api/script/update_node_attributes.rb      | 106 ---------------------
 services/api/test/fixtures/nodes.yml               |   4 +
 .../functional/arvados/v1/jobs_controller_test.rb  |   6 ++
 .../functional/arvados/v1/nodes_controller_test.rb |  72 ++++++++++++++
 13 files changed, 183 insertions(+), 147 deletions(-)
 create mode 100644 services/api/db/migrate/20140924091559_add_job_uuid_to_nodes.rb
 delete mode 100755 services/api/script/update_node_attributes.rb

       via  da0d61f9a95848d47d28462d89e0a2070c39d587 (commit)
       via  9340ba16925c4288e6de8a457b74ae9524b04213 (commit)
       via  e350e7a5a074d6666f60b0a1789dce8da3037d8f (commit)
      from  37404e821668b6b9b1952a0a5a3b28901835884b (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit da0d61f9a95848d47d28462d89e0a2070c39d587
Merge: 37404e8 9340ba1
Author: Brett Smith <brett at curoverse.com>
Date:   Wed Sep 24 15:34:21 2014 -0400

    Merge branch '2881-node-has-job-wip'
    
    Refs #2881.  Closes #3944.


commit 9340ba16925c4288e6de8a457b74ae9524b04213
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..45fb6dd 100755
--- a/services/api/script/crunch-dispatch.rb
+++ b/services/api/script/crunch-dispatch.rb
@@ -61,63 +61,68 @@ class Dispatcher
     end
   end
 
-  def sinfo
+  def each_slurm_line(cmd, outfmt, max_fields=nil)
+    max_fields ||= outfmt.split(":").size
+    max_fields += 1  # To accommodate the node field we add
     @@slurm_version ||= Gem::Version.new(`sinfo --version`.match(/\b[\d\.]+\b/)[0])
     if Gem::Version.new('2.3') <= @@slurm_version
-      `sinfo --noheader -o '%n:%t'`.strip
+      `#{cmd} --noheader -o '%n:#{outfmt}'`.each_line do |line|
+        yield line.chomp.split(":", max_fields)
+      end
     else
       # Expand rows with hostname ranges (like "foo[1-3,5,9-12]:idle")
       # into multiple rows with one hostname each.
-      `sinfo --noheader -o '%N:%t'`.split("\n").collect do |line|
-        tokens = line.split ":"
+      `#{cmd} --noheader -o '%N:#{outfmt}'`.each_line do |line|
+        tokens = line.chomp.split(":", max_fields)
         if (re = tokens[0].match /^(.*?)\[([-,\d]+)\]$/)
-          re[2].split(",").collect do |range|
+          tokens.shift
+          re[2].split(",").each do |range|
             range = range.split("-").collect(&:to_i)
-            (range[0]..range[-1]).collect do |n|
-              [re[1] + n.to_s, tokens[1..-1]].join ":"
+            (range[0]..range[-1]).each do |n|
+              yield [re[1] + n.to_s] + tokens
             end
           end
         else
-          tokens.join ":"
+          yield tokens
         end
-      end.flatten.join "\n"
+      end
+    end
+  end
+
+  def slurm_status
+    slurm_nodes = {}
+    each_slurm_line("sinfo", "%t") do |hostname, state|
+      state.sub!(/\W+$/, "")
+      state = "down" unless %w(idle alloc down).include?(state)
+      slurm_nodes[hostname] = {state: state, job: nil}
+    end
+    each_slurm_line("squeue", "%j") do |hostname, job_uuid|
+      slurm_nodes[hostname][:job] = job_uuid if slurm_nodes[hostname]
     end
+    slurm_nodes
   end
 
   def update_node_status
-    if Server::Application.config.crunch_job_wrapper.to_s.match /^slurm/
-      @node_state ||= {}
-      node_seen = {}
+    return unless Server::Application.config.crunch_job_wrapper.to_s.match /^slurm/
+    @node_state ||= {}
+    slurm_status.each_pair do |hostname, slurmdata|
+      next if @node_state[hostname] == slurmdata
       begin
-        sinfo.split("\n").
-          each do |line|
-          re = line.match /(\S+?):+(idle|alloc|down)?/
-          next if !re
-
-          _, node_name, node_state = *re
-          node_state = 'down' unless %w(idle alloc down).include? node_state
-
-          # sinfo tells us about a node N times if it is shared by N partitions
-          next if node_seen[node_name]
-          node_seen[node_name] = true
-
-          # update our database (and cache) when a node's state changes
-          if @node_state[node_name] != node_state
-            @node_state[node_name] = node_state
-            node = Node.where('hostname=?', node_name).order(:last_ping_at).last
-            if node
-              $stderr.puts "dispatch: update #{node_name} state to #{node_state}"
-              node.info['slurm_state'] = node_state
-              if not node.save
-                $stderr.puts "dispatch: failed to update #{node.uuid}: #{node.errors.messages}"
-              end
-            elsif node_state != 'down'
-              $stderr.puts "dispatch: sinfo reports '#{node_name}' is not down, but no node has that name"
-            end
+        node = Node.where('hostname=?', hostname).order(:last_ping_at).last
+        if node
+          $stderr.puts "dispatch: update #{hostname} state to #{slurmdata}"
+          node.info["slurm_state"] = slurmdata[:state]
+          node.job_uuid = slurmdata[:job]
+          if node.save
+            @node_state[hostname] = slurmdata
+          else
+            $stderr.puts "dispatch: failed to update #{node.uuid}: #{node.errors.messages}"
           end
+        elsif slurmdata[:state] != 'down'
+          $stderr.puts "dispatch: SLURM reports '#{hostname}' is not down, but no node has that name"
         end
       rescue => error
-        $stderr.puts "dispatch: error updating node status: #{error}"
+        $stderr.puts "dispatch: error updating #{hostname} node status: #{error}"
       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 e350e7a5a074d6666f60b0a1789dce8da3037d8f
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..b2c7924 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 remove a node's job assignment, update the node object's @job_uuid@ to null.
diff --git a/doc/api/schema/Job.html.textile.liquid b/doc/api/schema/Job.html.textile.liquid
index 513c105..f94f093 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.|
+|node_uuids|array|List of UUID strings for node objects that have been assigned to this job||
 |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..ff9e882 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_uuid|string|The UUID of the job that this node is assigned to work on.  If you do not have permission to read the job, this will be null.||
 |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..c9ac096 100644
--- a/services/api/app/controllers/arvados/v1/nodes_controller.rb
+++ b/services/api/app/controllers/arvados/v1/nodes_controller.rb
@@ -10,6 +10,13 @@ class Arvados::V1::NodesController < ApplicationController
     show
   end
 
+  def update
+    if resource_attrs[:job_uuid]
+      @object.job_readable = readable_job_uuids(resource_attrs[:job_uuid]).any?
+    end
+    super
+  end
+
   def self._ping_requires_parameters
     { ping_secret: true }
   end
@@ -45,5 +52,16 @@ class Arvados::V1::NodesController < ApplicationController
       # recently) working
       @objects = model_class.where('last_ping_at >= ?', Time.now - 1.hours)
     end
+    assigned_nodes = @objects.select(&:job_uuid)
+    assoc_jobs = readable_job_uuids(*assigned_nodes.map(&:job_uuid))
+    assigned_nodes.each do |node|
+      node.job_readable = assoc_jobs.include?(node.job_uuid)
+    end
+  end
+
+  protected
+
+  def readable_job_uuids(*uuids)
+    Job.readable_by(*@read_users).select(:uuid).where(uuid: uuids).map(&:uuid)
   end
 end
diff --git a/services/api/app/models/job.rb b/services/api/app/models/job.rb
index b4aa625..7da6852 100644
--- a/services/api/app/models/job.rb
+++ b/services/api/app/models/job.rb
@@ -16,6 +16,7 @@ class Job < ArvadosModel
   validate :validate_status
 
   has_many :commit_ancestors, :foreign_key => :descendant, :primary_key => :script_version
+  has_many(:nodes, foreign_key: :job_uuid, primary_key: :uuid)
 
   class SubmitIdReused < StandardError
   end
@@ -45,6 +46,7 @@ class Job < ArvadosModel
     t.add :supplied_script_version
     t.add :docker_image_locator
     t.add :queue_position
+    t.add :node_uuids
     t.add :description
   end
 
@@ -63,6 +65,10 @@ class Job < ArvadosModel
                       running: false)
   end
 
+  def node_uuids
+    nodes.map(&:uuid)
+  end
+
   def self.queue
     self.where('started_at is ? and is_locked_by_uuid is ? and cancelled_at is ? and success is ?',
                nil, nil, nil, nil).
diff --git a/services/api/app/models/node.rb b/services/api/app/models/node.rb
index 119a0ed..c791f8e 100644
--- a/services/api/app/models/node.rb
+++ b/services/api/app/models/node.rb
@@ -7,6 +7,12 @@ 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 the Job UUID can be included in the API response.
+  belongs_to(:job, foreign_key: :job_uuid, primary_key: :uuid)
+  attr_accessor :job_readable
+
   MAX_SLOTS = 64
 
   @@confdir = Rails.configuration.dnsmasq_conf_dir
@@ -20,6 +26,7 @@ class Node < ArvadosModel
     t.add :last_ping_at
     t.add :slot_number
     t.add :status
+    t.add :api_job_uuid, as: :job_uuid
     t.add :crunch_worker_state
     t.add :properties
   end
@@ -33,6 +40,10 @@ class Node < ArvadosModel
     super || @@domain
   end
 
+  def api_job_uuid
+    job_readable ? job_uuid : nil
+  end
+
   def crunch_worker_state
     return 'down' if slot_number.nil?
     case self.info.andand['slurm_state']
diff --git a/services/api/db/migrate/20140924091559_add_job_uuid_to_nodes.rb b/services/api/db/migrate/20140924091559_add_job_uuid_to_nodes.rb
new file mode 100644
index 0000000..d8ec20f
--- /dev/null
+++ b/services/api/db/migrate/20140924091559_add_job_uuid_to_nodes.rb
@@ -0,0 +1,13 @@
+class AddJobUuidToNodes < ActiveRecord::Migration
+  def up
+    change_table :nodes do |t|
+      t.column :job_uuid, :string
+    end
+  end
+
+  def down
+    change_table :nodes do |t|
+      t.remove :job_uuid
+    end
+  end
+end
diff --git a/services/api/db/structure.sql b/services/api/db/structure.sql
index ca87b1b..48bc57a 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_uuid character varying(255)
 );
 
 
@@ -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 ('20140924091559');
\ No newline at end of file
diff --git a/services/api/test/fixtures/nodes.yml b/services/api/test/fixtures/nodes.yml
index f282e7e..1511501 100644
--- a/services/api/test/fixtures/nodes.yml
+++ b/services/api/test/fixtures/nodes.yml
@@ -7,6 +7,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_uuid: zzzzz-8i9sb-2gx6rz0pjl033w3  # nearly_finished_job
   info:
     ping_secret: "48dpm3b8ijyj3jkr2yczxw0844dqd2752bhll7klodvgz9bg80"
     slurm_state: "alloc"
@@ -20,6 +21,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_uuid: ~
   info:
     ping_secret: "2k3i71depad36ugwmlgzilbi4e8n0illb2r8l4efg9mzkb3a1k"
 
@@ -32,6 +34,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_uuid: ~
   info:
     ping_secret: "69udawxvn3zzj45hs8bumvndricrha4lcpi23pd69e44soanc0"
     slurm_state: "idle"
@@ -46,6 +49,7 @@ was_idle_now_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_uuid: ~
   info:
     ping_secret: "1bd1yi0x4lb5q4gzqqtrnq30oyj08r8dtdimmanbqw49z1anz2"
     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..a9a49a0 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,10 @@ class Arvados::V1::JobsControllerTest < ActionController::TestCase
     end
   end
 
+  test "job includes assigned nodes" do
+    authorize_with :active
+    get :show, {id: jobs(:nearly_finished_job).uuid}
+    assert_response :success
+    assert_equal([nodes(:busy).uuid], json_response["node_uuids"])
+  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..dd942b6 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,76 @@ 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_uuid"])
+  end
+
+  test "user without job read permission can't see job" do
+    authorize_with :spectator
+    get :show, {id: nodes(:busy).uuid}
+    assert_response :success
+    assert_nil(json_response["job"], "spectator can see 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_uuid: 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_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_uuid: 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_uuid: nil},
+    }
+    assert_response :success
+    assert_equal(changed_node.hostname, json_response["hostname"],
+                 "hostname mismatch after defining job")
+    assert_nil(json_response["job_uuid"],
+               "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_uuid: nil},
+    }
+    assert_response 403
+  end
+
+  test "job readable after updating other attributes" do
+    authorize_with :admin
+    post :update, {
+      id: nodes(:busy).uuid,
+      node: {last_ping_at: 1.second.ago},
+    }
+    assert_response :success
+    assert_equal(jobs(:nearly_finished_job).uuid, json_response["job_uuid"],
+                 "mismatched job UUID after ping update")
+  end
 end

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list