From 0a62e9787194ee68aab80b5723d7de2c69d1965c Mon Sep 17 00:00:00 2001 From: Simon Leary Date: Wed, 16 Jul 2025 15:42:22 +0000 Subject: [PATCH] remove queue from AccountInfo --- lib/ood_core/job/account_info.rb | 5 -- lib/ood_core/job/adapter.rb | 3 +- lib/ood_core/job/adapters/slurm.rb | 19 ++++--- .../output/slurm/sacctmgr_show_accts.txt | 49 +++++++++---------- spec/job/adapters/slurm_spec.rb | 28 +++-------- 5 files changed, 42 insertions(+), 62 deletions(-) diff --git a/lib/ood_core/job/account_info.rb b/lib/ood_core/job/account_info.rb index e6ae36c00..b13ae4971 100644 --- a/lib/ood_core/job/account_info.rb +++ b/lib/ood_core/job/account_info.rb @@ -15,16 +15,11 @@ class AccountInfo # The cluster this account is associated with. attr_reader :cluster - # The queue this account can use. nil means there is no queue info - # for this account. - attr_reader :queue - def initialize(**opts) orig_name = opts.fetch(:name, 'unknown') @name = upcase_accounts? ? orig_name.upcase : orig_name @qos = opts.fetch(:qos, []) @cluster = opts.fetch(:cluster, nil) - @queue = opts.fetch(:queue, nil) end def to_h diff --git a/lib/ood_core/job/adapter.rb b/lib/ood_core/job/adapter.rb index a12f89a7e..6cc21c856 100644 --- a/lib/ood_core/job/adapter.rb +++ b/lib/ood_core/job/adapter.rb @@ -210,7 +210,8 @@ def job_name_illegal_chars ENV["OOD_JOB_NAME_ILLEGAL_CHARS"].to_s end - # Retrieve the accounts available to use for the current user. + # Retrieve the accounts available to use for the current user. + # The same account might appear muiltiple times if it has access to multiple clusters. # # Subclasses that do not implement this will return empty arrays. # @return [Array] the accounts available to the user. diff --git a/lib/ood_core/job/adapters/slurm.rb b/lib/ood_core/job/adapters/slurm.rb index bdd4b7663..16be2d46a 100644 --- a/lib/ood_core/job/adapters/slurm.rb +++ b/lib/ood_core/job/adapters/slurm.rb @@ -182,22 +182,27 @@ def get_jobs(id: "", owner: nil, attrs: nil) def accounts user = Etc.getlogin - args = ['-nP', 'show', 'users', 'withassoc', 'format=account,cluster,partition,qos', 'where', "user=#{user}"] + args = ['-nP', 'show', 'users', 'withassoc', 'format=account,cluster,qos', 'where', "user=#{user}"] - [].tap do |accts| + [].tap do |associations| call('sacctmgr', *args).each_line do |line| - acct, cluster, queue, qos = line.split('|') + acct, cluster, qos = line.split('|') next if acct.nil? || acct.chomp.empty? - args = { + associations << { name: acct, qos: qos.to_s.chomp.split(','), cluster: cluster, - queue: queue.to_s.empty? ? nil : queue } - info = OodCore::Job::AccountInfo.new(**args) unless acct.nil? - accts << info unless acct.nil? end + end.group_by do |x| + [x[:name], x[:cluster]] + end.map do |(name, cluster), assocs| + OodCore::Job::AccountInfo.new( + name: name, + cluster: cluster, + qos: (assocs.flat_map { |x| x[:qos] }).uniq, + ) end end diff --git a/spec/fixtures/output/slurm/sacctmgr_show_accts.txt b/spec/fixtures/output/slurm/sacctmgr_show_accts.txt index 54c780646..5644315b1 100644 --- a/spec/fixtures/output/slurm/sacctmgr_show_accts.txt +++ b/spec/fixtures/output/slurm/sacctmgr_show_accts.txt @@ -1,27 +1,22 @@ -pzs0715|ascend|partition_a|ascend-default -pzs0714|ascend|partition_b|ascend-default -pzs1124|owens||owens-default,staff,phoenix,geophys,hal,gpt -pzs1118|owens||owens-default -pzs1117|owens||owens-default -pzs1010|owens||owens-default -pzs0715|owens||owens-default -pzs0714|owens||owens-default -pde0006|owens||owens-default -pas2051|owens||owens-default -pas1871|owens||owens-default -pas1754|owens||owens-default -pas1604|owens||owens-default -pzs1124|pitzer||pitzer-default -pzs1118|pitzer||pitzer-default -pzs1117|pitzer||pitzer-default -pzs1010|pitzer||pitzer-default -pzs0715|pitzer||pitzer-default -pzs0714|pitzer||pitzer-default -pde0006|pitzer||pitzer-default -pas2051|pitzer||pitzer-default -pas1871|pitzer||pitzer-default -pas1754|pitzer||pitzer-default -pas1604|pitzer||pitzer-default - - - +pzs1124|owens|owens-default,staff,phoenix,geophys,hal,gpt +pzs1118|owens|owens-default +pzs1117|owens|owens-default +pzs1010|owens|owens-default +pzs0715|owens|owens-default +pzs0714|owens|owens-default +pde0006|owens|owens-default +pas2051|owens|owens-default +pas1871|owens|owens-default +pas1754|owens|owens-default +pas1604|owens|owens-default +pzs1124|pitzer|pitzer-default +pzs1118|pitzer|pitzer-default +pzs1117|pitzer|pitzer-default +pzs1010|pitzer|pitzer-default +pzs0715|pitzer|pitzer-default +pzs0714|pitzer|pitzer-default +pde0006|pitzer|pitzer-default +pas2051|pitzer|pitzer-default +pas1871|pitzer|pitzer-default +pas1754|pitzer|pitzer-default +pas1604|pitzer|pitzer-default diff --git a/spec/job/adapters/slurm_spec.rb b/spec/job/adapters/slurm_spec.rb index 55fce9755..95b851498 100644 --- a/spec/job/adapters/slurm_spec.rb +++ b/spec/job/adapters/slurm_spec.rb @@ -1305,17 +1305,17 @@ def job_info(opts = {}) it 'returns the correct accounts names' do allow(Etc).to receive(:getlogin).and_return('me') allow(Open3).to receive(:capture3) - .with({}, 'sacctmgr', '-nP', 'show', 'users', 'withassoc', 'format=account,cluster,partition,qos', 'where', 'user=me', {stdin_data: ''}) + .with({}, 'sacctmgr', '-nP', 'show', 'users', 'withassoc', 'format=account,cluster,qos', 'where', 'user=me', {stdin_data: ''}) .and_return([File.read('spec/fixtures/output/slurm/sacctmgr_show_accts.txt'), '', double("success?" => true)]) - expect(subject.accounts.map(&:to_s).uniq).to eq(expected_accounts) + expect(subject.accounts.map(&:to_s).uniq.to_set).to eq(expected_accounts.to_set) end # TODO test for qos & cluster once the API solidifies it 'parses qos correctly' do allow(Etc).to receive(:getlogin).and_return('me') allow(Open3).to receive(:capture3) - .with({}, 'sacctmgr', '-nP', 'show', 'users', 'withassoc', 'format=account,cluster,partition,qos', 'where', 'user=me', {stdin_data: ''}) + .with({}, 'sacctmgr', '-nP', 'show', 'users', 'withassoc', 'format=account,cluster,qos', 'where', 'user=me', {stdin_data: ''}) .and_return([File.read('spec/fixtures/output/slurm/sacctmgr_show_accts.txt'), '', double("success?" => true)]) accts = subject.accounts @@ -1327,22 +1327,6 @@ def job_info(opts = {}) expect(acct.qos).to eq(["#{acct.cluster}-default"]) end end - - it 'parses partition correctly' do - allow(Etc).to receive(:getlogin).and_return('me') - allow(Open3).to receive(:capture3) - .with({}, 'sacctmgr', '-nP', 'show', 'users', 'withassoc', 'format=account,cluster,partition,qos', 'where', 'user=me', {stdin_data: ''}) - .and_return([File.read('spec/fixtures/output/slurm/sacctmgr_show_accts.txt'), '', double("success?" => true)]) - - accts = subject.accounts - acct_w_partitions = accts.select { |a| a.cluster == 'ascend' } - acct_w_no_partitions = accts.select { |a| a.queue.nil? } - - expect(acct_w_partitions.size).to eq(2) - expect(accts - acct_w_no_partitions).to eq(acct_w_partitions) - expect(acct_w_partitions.select {|a| a.name == 'pzs0715'}.first.queue).to eq('partition_a') - expect(acct_w_partitions.select {|a| a.name == 'pzs0714'}.first.queue).to eq('partition_b') - end end context 'when sacctmgr fails' do @@ -1351,7 +1335,7 @@ def job_info(opts = {}) it 'raises the error' do allow(Etc).to receive(:getlogin).and_return('me') allow(Open3).to receive(:capture3) - .with({}, 'sacctmgr', '-nP', 'show', 'users', 'withassoc', 'format=account,cluster,partition,qos', 'where', 'user=me', {stdin_data: ''}) + .with({}, 'sacctmgr', '-nP', 'show', 'users', 'withassoc', 'format=account,cluster,qos', 'where', 'user=me', {stdin_data: ''}) .and_return(['', 'the error message', double("success?" => false)]) expect { subject.accounts }.to raise_error(OodCore::Job::Adapters::Slurm::Batch::Error, 'the error message') @@ -1365,11 +1349,11 @@ def job_info(opts = {}) it 'returns the correct accounts' do allow(Etc).to receive(:getlogin).and_return('me') allow(Open3).to receive(:capture3) - .with({}, 'sacctmgr', '-nP', 'show', 'users', 'withassoc', 'format=account,cluster,partition,qos', 'where', 'user=me', {stdin_data: ''}) + .with({}, 'sacctmgr', '-nP', 'show', 'users', 'withassoc', 'format=account,cluster,qos', 'where', 'user=me', {stdin_data: ''}) .and_return([File.read('spec/fixtures/output/slurm/sacctmgr_show_accts.txt'), '', double("success?" => true)]) with_modified_env({ OOD_UPCASE_ACCOUNTS: 'true'}) do - expect(subject.accounts.map(&:to_s).uniq).to eq(expected_accounts) + expect(subject.accounts.map(&:to_s).uniq.to_set).to eq(expected_accounts.to_set) end end end