diff --git a/src/bindings/python/fluxacct/accounting/bank_subcommands.py b/src/bindings/python/fluxacct/accounting/bank_subcommands.py index 8e9b4d36..c5349c7b 100644 --- a/src/bindings/python/fluxacct/accounting/bank_subcommands.py +++ b/src/bindings/python/fluxacct/accounting/bank_subcommands.py @@ -219,6 +219,12 @@ def print_parsable_hierarchy(cur, bank, hierarchy_str, indent=""): def add_bank(conn, bank, shares, parent_bank=""): cur = conn.cursor() + if parent_bank == "": + # a root bank is trying to be added; check that one does not already exist + cur.execute("SELECT * FROM bank_table WHERE parent_bank=''") + if len(cur.fetchall()) > 0: + raise ValueError(f"bank_table already has a root bank") + # if the parent bank is not "", that means the bank trying # to be added wants to be placed under an existing parent bank try: diff --git a/t/Makefile.am b/t/Makefile.am index 392e0f7c..923026eb 100644 --- a/t/Makefile.am +++ b/t/Makefile.am @@ -41,6 +41,7 @@ TESTSCRIPTS = \ t1039-issue476.t \ t1040-mf-priority-projects.t \ t1041-view-jobs-by-project.t \ + t1042-issue508.t \ t5000-valgrind.t \ python/t1000-example.py \ python/t1001_db.py \ diff --git a/t/python/t1002_user_cmds.py b/t/python/t1002_user_cmds.py index a721c840..4b023f20 100755 --- a/t/python/t1002_user_cmds.py +++ b/t/python/t1002_user_cmds.py @@ -78,7 +78,7 @@ def test_02_add_duplicate_primary_key(self): # add a user with the same username but a different bank def test_03_add_duplicate_user(self): - b.add_bank(acct_conn, bank="other_acct", shares=10) + b.add_bank(acct_conn, bank="other_acct", parent_bank="acct", shares=10) u.add_user( acct_conn, username="dup_user", @@ -151,7 +151,7 @@ def test_06_delete_user(self): # check for a new user's default bank def test_07_check_default_bank_new_user(self): - b.add_bank(acct_conn, bank="test_bank", shares=10) + b.add_bank(acct_conn, bank="test_bank", parent_bank="acct", shares=10) u.add_user( acct_conn, username="test_user1", @@ -167,7 +167,7 @@ def test_07_check_default_bank_new_user(self): # check for an existing user's default bank def test_08_check_default_bank_existing_user(self): - b.add_bank(acct_conn, bank="other_test_bank", shares=10) + b.add_bank(acct_conn, bank="other_test_bank", parent_bank="acct", shares=10) u.add_user( acct_conn, username="test_user1", @@ -202,8 +202,8 @@ def test_10_view_nonexistent_user(self): # disable a user who belongs to multiple banks; make sure that the default_bank # is updated to the next earliest associated bank def test_11_disable_user_default_bank_row(self): - b.add_bank(acct_conn, bank="A", shares=1) - b.add_bank(acct_conn, bank="B", shares=1) + b.add_bank(acct_conn, bank="A", parent_bank="acct", shares=1) + b.add_bank(acct_conn, bank="B", parent_bank="acct", shares=1) u.add_user(acct_conn, username="test_user2", bank="A") u.add_user(acct_conn, username="test_user2", bank="B") cur = acct_conn.cursor() diff --git a/t/python/t1003_bank_cmds.py b/t/python/t1003_bank_cmds.py index a49de79d..b38f9e88 100755 --- a/t/python/t1003_bank_cmds.py +++ b/t/python/t1003_bank_cmds.py @@ -39,10 +39,10 @@ def test_01_add_bank_success(self): rows = cur.fetchall() self.assertEqual(len(rows), 1) - # check for an IntegrityError when trying to add a duplicate bank + # a root bank already exists, so a ValueError will be raised def test_02_add_dup_bank(self): - with self.assertRaises(sqlite3.IntegrityError): - b.add_bank(acct_conn, bank="root", shares=100) + with self.assertRaises(ValueError): + b.add_bank(acct_conn, bank="second_root", shares=100) # trying to add a sub account with an invalid parent bank # name should result in a ValueError @@ -76,19 +76,15 @@ def test_05_disable_bank_success(self): # disabling a parent bank should disable all of its sub banks def test_06_disable_parent_bank(self): - b.delete_bank(acct_conn, bank="root") - b.delete_bank(acct_conn, bank="sub_account_2") - - b.add_bank(acct_conn, bank="A", shares=1) - b.add_bank(acct_conn, bank="B", parent_bank="A", shares=1) + b.add_bank(acct_conn, bank="B", parent_bank="root", shares=1) b.add_bank(acct_conn, bank="D", parent_bank="B", shares=1) b.add_bank(acct_conn, bank="E", parent_bank="B", shares=1) - b.add_bank(acct_conn, bank="C", parent_bank="A", shares=1) + b.add_bank(acct_conn, bank="C", parent_bank="root", shares=1) b.add_bank(acct_conn, bank="F", parent_bank="C", shares=1) b.add_bank(acct_conn, bank="G", parent_bank="C", shares=1) - b.delete_bank(acct_conn, bank="A") - cur.execute("SELECT active FROM bank_table WHERE bank='A'") + b.delete_bank(acct_conn, bank="root") + cur.execute("SELECT active FROM bank_table WHERE bank='root'") rows = cur.fetchall() self.assertEqual(rows[0][0], 0) @@ -102,7 +98,6 @@ def test_06_disable_parent_bank(self): # edit a bank value def test_07_edit_bank_value(self): - b.add_bank(acct_conn, bank="root", shares=100) b.edit_bank(acct_conn, bank="root", shares=50) cursor = acct_conn.cursor() cursor.execute("SELECT shares FROM bank_table where bank='root'") diff --git a/t/t1042-issue508.t b/t/t1042-issue508.t new file mode 100755 index 00000000..76588cd7 --- /dev/null +++ b/t/t1042-issue508.t @@ -0,0 +1,38 @@ +#!/bin/bash + +test_description='test trying to add more than one root bank to the bank_table' + +. `dirname $0`/sharness.sh + +mkdir -p conf.d + +ACCOUNTING_DB=$(pwd)/FluxAccountingTest.db + +export TEST_UNDER_FLUX_SCHED_SIMPLE_MODE="limited=1" +test_under_flux 1 job -o,--config-path=$(pwd)/conf.d + +flux setattr log-stderr-level 1 + +test_expect_success 'create flux-accounting DB, start flux-accounting service' ' + flux account -p ${ACCOUNTING_DB} create-db && + flux account-service -p ${ACCOUNTING_DB} -t +' + +test_expect_success 'add a root bank' ' + flux account add-bank root 1 +' + +test_expect_success 'adding a second root bank will raise a ValueError' ' + test_must_fail flux account add-bank second_root 1 > error.out 2>&1 && + grep "bank_table already has a root bank" error.out +' + +test_expect_success 'shut down flux-accounting service' ' + flux python -c "import flux; flux.Flux().rpc(\"accounting.shutdown_service\").get()" +' + +test_expect_success 'remove flux-accounting DB' ' + rm ${ACCOUNTING_DB} +' + +test_done