Skip to content

Commit

Permalink
DAOS-16282 control: Fix some test assumptions about JSON (#14832)
Browse files Browse the repository at this point in the history
The commit landed for DAOS-16127 resulted in some differences in
JSON output for pool query. Several tests were written in such a
way that the code expected the (en|dis)abled_ranks keys to always
be set, even if those arrays were NULL. This isn't very idiomatic
and is awkward to work with. The test code has been updated to
instead use the get() operator which will return None if the
response dict does not have the requested key.

Also fixes a problem reported in DAOS-16283, where the JSON output
of `dmg pool query` differed from the JSON output of `daos pool query`
because it didn't include the usage array.

Signed-off-by: Michael MacDonald <[email protected]>
  • Loading branch information
mjmac authored Aug 6, 2024
1 parent a30bdb6 commit 8896868
Show file tree
Hide file tree
Showing 10 changed files with 43 additions and 41 deletions.
7 changes: 5 additions & 2 deletions src/control/cmd/dmg/pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -638,9 +638,12 @@ func (cmd *PoolQueryCmd) Execute(args []string) error {
}

resp, err := control.PoolQuery(cmd.MustLogCtx(), cmd.ctlInvoker, req)

if cmd.JSONOutputEnabled() {
return cmd.OutputJSON(resp, err)
var poolInfo *daos.PoolInfo
if resp != nil {
poolInfo = &resp.PoolInfo
}
return cmd.OutputJSON(poolInfo, err)
}

if err != nil {
Expand Down
48 changes: 24 additions & 24 deletions src/tests/ftest/control/dmg_pool_query_ranks.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,46 +41,46 @@ def test_pool_query_ranks_basic(self):
self.log.debug("Checking without ranks state information")
data = self.dmg.pool_query(self.pool.identifier)
self.assertIsNone(
data['response']['enabled_ranks'],
data['response'].get('enabled_ranks'),
"Invalid enabled_ranks field: want=None, got={}".format(
data['response']['enabled_ranks']))
data['response'].get('enabled_ranks')))
self.assertIsNone(
data['response']['disabled_ranks'],
data['response'].get('disabled_ranks'),
"Invalid disabled_ranks field: want=None, got={}".format(
data['response']['disabled_ranks']))
data['response'].get('disabled_ranks')))

self.log.debug("Checking enabled ranks state information")
data = self.dmg.pool_query(self.pool.identifier, show_enabled=True)
self.assertListEqual(
data['response']['enabled_ranks'], [0, 1, 2],
data['response'].get('enabled_ranks'), [0, 1, 2],
"Invalid enabled_ranks field: want=[0, 1, 2], got={}".format(
data['response']['enabled_ranks']))
data['response'].get('enabled_ranks')))
self.assertIsNone(
data['response']['disabled_ranks'],
data['response'].get('disabled_ranks'),
"Invalid disabled_ranks field: want=None, got={}".format(
data['response']['disabled_ranks']))
data['response'].get('disabled_ranks')))

self.log.debug("Checking disabled ranks state information")
data = self.dmg.pool_query(self.pool.identifier, show_disabled=True)
self.assertIsNone(
data['response']['enabled_ranks'],
data['response'].get('enabled_ranks'),
"Invalid enabled_ranks field: want=None, got={}".format(
data['response']['enabled_ranks']))
data['response'].get('enabled_ranks')))
self.assertListEqual(
data['response']['disabled_ranks'], [],
data['response'].get('disabled_ranks'), [],
"Invalid disabled_ranks field: want=[], got={}".format(
data['response']['disabled_ranks']))
data['response'].get('disabled_ranks')))

self.log.debug("Checking enabled and disabled ranks state information")
data = self.dmg.pool_query(self.pool.identifier, show_enabled=True, show_disabled=True)
self.assertListEqual(
data['response']['enabled_ranks'], [0, 1, 2],
data['response'].get('enabled_ranks'), [0, 1, 2],
"Invalid enabled_ranks field: want=[0, 1, 2], got={}".format(
data['response']['enabled_ranks']))
data['response'].get('enabled_ranks')))
self.assertListEqual(
data['response']['disabled_ranks'], [],
data['response'].get('disabled_ranks'), [],
"Invalid disabled_ranks field: want=[], got={}".format(
data['response']['disabled_ranks']))
data['response'].get('disabled_ranks')))

def test_pool_query_ranks_mgmt(self):
"""Test the state of ranks after excluding and reintegrate them.
Expand Down Expand Up @@ -113,13 +113,13 @@ def test_pool_query_ranks_mgmt(self):
data = self.dmg.pool_query(
self.pool.identifier, show_enabled=True, show_disabled=True)
self.assertListEqual(
data['response']['enabled_ranks'], enabled_ranks,
data['response'].get('enabled_ranks'), enabled_ranks,
"Invalid enabled_ranks field: want={}, got={}".format(
enabled_ranks, data['response']['enabled_ranks']))
enabled_ranks, data['response'].get('enabled_ranks')))
self.assertListEqual(
data['response']['disabled_ranks'], disabled_ranks,
data['response'].get('disabled_ranks'), disabled_ranks,
"Invalid disabled_ranks field: want={}, got={}".format(
disabled_ranks, data['response']['disabled_ranks']))
disabled_ranks, data['response'].get('disabled_ranks')))

self.log.debug("Waiting for pool to be rebuild")
self.pool.wait_for_rebuild_to_start()
Expand Down Expand Up @@ -147,13 +147,13 @@ def test_pool_query_ranks_mgmt(self):
self.log.debug("Checking enabled ranks state information")
data = self.dmg.pool_query(self.pool.identifier, show_enabled=True, show_disabled=True)
self.assertListEqual(
data['response']['enabled_ranks'], enabled_ranks,
data['response'].get('enabled_ranks'), enabled_ranks,
"Invalid enabled_ranks field: want={}, got={}".format(
enabled_ranks, data['response']['enabled_ranks']))
enabled_ranks, data['response'].get('enabled_ranks')))
self.assertListEqual(
data['response']['disabled_ranks'], disabled_ranks,
data['response'].get('disabled_ranks'), disabled_ranks,
"Invalid disabled_ranks field: want={}, got={}".format(
disabled_ranks, data['response']['disabled_ranks']))
disabled_ranks, data['response'].get('disabled_ranks')))

self.log.debug("Waiting for pool to be rebuild")
self.pool.wait_for_rebuild_to_start()
Expand Down
8 changes: 3 additions & 5 deletions src/tests/ftest/control/dmg_pool_query_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,6 @@ def test_pool_query_basic(self):
# exp_info = self.params.get("exp_vals", path="/run/*", default={})
# but this yields an empty dictionary (the default), so it needs to be defined manually:
exp_info = {
"enabled_ranks": None,
"disabled_ranks": None,
"status": self.params.get("pool_status", path="/run/exp_vals/*"),
"state": self.params.get("pool_state", path="/run/exp_vals/*"),
"uuid": self.pool.uuid.lower(),
"total_targets": self.params.get("total_targets", path="/run/exp_vals/*"),
Expand All @@ -74,7 +71,8 @@ def test_pool_query_basic(self):
"status": self.params.get("rebuild_status", path="/run/exp_vals/rebuild/*"),
"state": self.params.get("state", path="/run/exp_vals/rebuild/*"),
"objects": self.params.get("objects", path="/run/exp_vals/rebuild/*"),
"records": self.params.get("records", path="/run/exp_vals/rebuild/*")
"records": self.params.get("records", path="/run/exp_vals/rebuild/*"),
"total_objects": self.params.get("total_objects", path="/run/exp_vals/rebuild/*")
},
"tier_stats": [
{
Expand Down Expand Up @@ -102,7 +100,7 @@ def test_pool_query_basic(self):
}

self.assertDictEqual(
self.pool.query_data["response"], exp_info,
exp_info, self.pool.query_data["response"],
"Found difference in dmg pool query output and the expected values")

self.log.info("All expect values found in dmg pool query output.")
Expand Down
2 changes: 1 addition & 1 deletion src/tests/ftest/control/dmg_pool_query_test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ ior:
dfs_destroy: False

exp_vals:
pool_status: 0
pool_state: "Ready"
total_targets: 4
active_targets: 4
Expand All @@ -44,6 +43,7 @@ exp_vals:
state: "idle"
objects: 0
records: 0
total_objects: 0

pool_uuids:
uuids:
Expand Down
2 changes: 1 addition & 1 deletion src/tests/ftest/deployment/network_failure.py
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ def verify_network_failure(self, ior_namespace, container_namespace):
# 6. Call dmg pool query -b to find the disabled ranks.
self.log_step("Find the disabled ranks.")
output = dmg_cmd.pool_query(pool=self.pool.identifier, show_disabled=True)
disabled_ranks = output["response"]["disabled_ranks"]
disabled_ranks = output["response"].get("disabled_ranks")
self.log.info("Disabled ranks = %s", disabled_ranks)

# 7. Call dmg pool reintegrate one rank at a time to enable all ranks.
Expand Down
4 changes: 2 additions & 2 deletions src/tests/ftest/deployment/server_rank_failure.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
"""
(C) Copyright 2022-2023 Intel Corporation.
(C) Copyright 2022-2024 Intel Corporation.
SPDX-License-Identifier: BSD-2-Clause-Patent
"""
Expand Down Expand Up @@ -184,7 +184,7 @@ def verify_rank_failure(self, ior_namespace):

# 9. Call dmg pool query -b to find the disabled ranks.
output = self.get_dmg_command().pool_query(pool=self.pool.identifier, show_disabled=True)
disabled_ranks = output["response"]["disabled_ranks"]
disabled_ranks = output["response"].get("disabled_ranks")
self.log.info("Disabled ranks = %s", disabled_ranks)

# 10. Call dmg pool reintegrate one rank at a time to enable all ranks.
Expand Down
4 changes: 2 additions & 2 deletions src/tests/ftest/pool/create_query.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
"""
(C) Copyright 2021-2023 Intel Corporation.
(C) Copyright 2021-2024 Intel Corporation.
SPDX-License-Identifier: BSD-2-Clause-Patent
"""
Expand Down Expand Up @@ -51,7 +51,7 @@ def test_create_and_query(self):

# Query pool created
resp = self.get_dmg_command().pool_query(pool.identifier, show_enabled=True)["response"]
nb_ranks = len(resp["enabled_ranks"])
nb_ranks = len(resp.get("enabled_ranks"))
tier_stats = resp["tier_stats"]
self.assertEqual("SCM", tier_stats[0]["media_type"].upper(), "Unexpected tier media type")
self.assertEqual("NVME", tier_stats[1]["media_type"].upper(), "Unexpected tier media type")
Expand Down
3 changes: 2 additions & 1 deletion src/tests/ftest/pool/list_verbose.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ def create_expected(self, pool, scm_free, nvme_free, scm_imbalance,
"status": 0,
"state": rebuild_state,
"objects": 0,
"records": 0
"records": 0,
"total_objects": 0
},
# NB: tests should not expect min/max/mean values
"tier_stats": [
Expand Down
4 changes: 2 additions & 2 deletions src/tests/ftest/util/dmg_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -669,8 +669,8 @@ def pool_query(self, pool, show_enabled=False, show_disabled=False):
# "max": 3999993856,
# "mean": 3999993856
# },
# "enabled_ranks": None,
# "disabled_ranks": None
# "enabled_ranks": [0,1,3],
# "disabled_ranks": [2]
# },
# "error": null,
# "status": 0
Expand Down
2 changes: 1 addition & 1 deletion src/tests/ftest/util/pool_create_all_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ def check_pool_full_storage(self, scm_delta_bytes, nvme_delta_bytes=None, ranks=
if ranks is not None:
wait_ranks = sorted(ranks)
data = self.dmg.pool_query(self.pool[pool_idx].identifier, show_enabled=True)
got_ranks = sorted(data['response']['enabled_ranks'])
got_ranks = sorted(data['response'].get('enabled_ranks'))
self.assertListEqual(
wait_ranks,
got_ranks,
Expand Down

0 comments on commit 8896868

Please sign in to comment.