From 955b96370c38817ddaa394e6a2fb70b9771516c4 Mon Sep 17 00:00:00 2001 From: Cedric Koch-Hofer <94527853+knard-intel@users.noreply.github.com> Date: Thu, 3 Oct 2024 15:22:56 +0200 Subject: [PATCH] DAOS-14419 control: Display disabled ranks by default (#15112) Always display the disabled targets and remove the old associated options. Required-githooks: true Signed-off-by: Cedric Koch-Hofer --- src/control/cmd/daos/pool.go | 12 ++--- src/control/cmd/daos/pretty/pool.go | 2 +- src/control/cmd/dmg/pool.go | 9 ++-- src/control/cmd/dmg/pool_test.go | 48 ------------------- src/control/lib/control/pool_test.go | 4 +- src/control/lib/daos/pool.go | 2 +- src/control/lib/daos/pool_test.go | 2 +- .../ftest/control/dmg_pool_query_ranks.py | 45 ++--------------- .../ftest/control/dmg_pool_query_test.py | 1 + .../ftest/control/dmg_pool_query_test.yaml | 3 +- src/tests/ftest/deployment/network_failure.py | 2 +- .../ftest/deployment/server_rank_failure.py | 2 +- src/tests/ftest/util/dmg_utils.py | 6 +-- src/tests/ftest/util/dmg_utils_base.py | 1 - src/tests/ftest/util/test_utils_pool.py | 10 ++-- 15 files changed, 26 insertions(+), 123 deletions(-) diff --git a/src/control/cmd/daos/pool.go b/src/control/cmd/daos/pool.go index 2aae717766e8..7a917b1a9e44 100644 --- a/src/control/cmd/daos/pool.go +++ b/src/control/cmd/daos/pool.go @@ -224,9 +224,8 @@ type poolCmd struct { type poolQueryCmd struct { poolBaseCmd - ShowEnabledRanks bool `short:"e" long:"show-enabled" description:"Show engine unique identifiers (ranks) which are enabled"` - ShowDisabledRanks bool `short:"b" long:"show-disabled" description:"Show engine unique identifiers (ranks) which are disabled"` - HealthOnly bool `short:"t" long:"health-only" description:"Only perform pool health related queries"` + ShowEnabledRanks bool `short:"e" long:"show-enabled" description:"Show engine unique identifiers (ranks) which are enabled"` + HealthOnly bool `short:"t" long:"health-only" description:"Only perform pool health related queries"` } func convertPoolSpaceInfo(in *C.struct_daos_pool_space, mt C.uint) *daos.StorageUsageStats { @@ -340,15 +339,10 @@ func (cmd *poolQueryCmd) Execute(_ []string) error { if cmd.HealthOnly { queryMask = daos.HealthOnlyPoolQueryMask } - if cmd.ShowEnabledRanks && cmd.ShowDisabledRanks { - return errors.New("show-enabled and show-disabled can't be used at the same time.") - } if cmd.ShowEnabledRanks { queryMask.SetOptions(daos.PoolQueryOptionEnabledEngines) } - if cmd.ShowDisabledRanks { - queryMask.SetOptions(daos.PoolQueryOptionDisabledEngines) - } + queryMask.SetOptions(daos.PoolQueryOptionDisabledEngines) cleanup, err := cmd.resolveAndConnect(C.DAOS_PC_RO, nil) if err != nil { diff --git a/src/control/cmd/daos/pretty/pool.go b/src/control/cmd/daos/pretty/pool.go index aa70115152df..a9f685b536f6 100644 --- a/src/control/cmd/daos/pretty/pool.go +++ b/src/control/cmd/daos/pretty/pool.go @@ -50,7 +50,7 @@ func PrintPoolInfo(pi *daos.PoolInfo, out io.Writer) error { if pi.EnabledRanks != nil && pi.EnabledRanks.Count() > 0 { fmt.Fprintf(w, "- Enabled ranks: %s\n", pi.EnabledRanks) } - if pi.DisabledRanks != nil && pi.DisabledRanks.Count() > 0 { + if pi.DisabledRanks.Count() > 0 { fmt.Fprintf(w, "- Disabled ranks: %s\n", pi.DisabledRanks) } if pi.Rebuild != nil { diff --git a/src/control/cmd/dmg/pool.go b/src/control/cmd/dmg/pool.go index 7edf2c6652f2..400ef237ca01 100644 --- a/src/control/cmd/dmg/pool.go +++ b/src/control/cmd/dmg/pool.go @@ -606,9 +606,8 @@ func (cmd *PoolReintegrateCmd) Execute(args []string) error { // PoolQueryCmd is the struct representing the command to query a DAOS pool. type PoolQueryCmd struct { poolCmd - ShowEnabledRanks bool `short:"e" long:"show-enabled" description:"Show engine unique identifiers (ranks) which are enabled"` - ShowDisabledRanks bool `short:"b" long:"show-disabled" description:"Show engine unique identifiers (ranks) which are disabled"` - HealthOnly bool `short:"t" long:"health-only" description:"Only perform pool health related queries"` + ShowEnabledRanks bool `short:"e" long:"show-enabled" description:"Show engine unique identifiers (ranks) which are enabled"` + HealthOnly bool `short:"t" long:"health-only" description:"Only perform pool health related queries"` } // Execute is run when PoolQueryCmd subcommand is activated @@ -624,9 +623,7 @@ func (cmd *PoolQueryCmd) Execute(args []string) error { if cmd.ShowEnabledRanks { req.QueryMask.SetOptions(daos.PoolQueryOptionEnabledEngines) } - if cmd.ShowDisabledRanks { - req.QueryMask.SetOptions(daos.PoolQueryOptionDisabledEngines) - } + req.QueryMask.SetOptions(daos.PoolQueryOptionDisabledEngines) resp, err := control.PoolQuery(cmd.MustLogCtx(), cmd.ctlInvoker, req) if cmd.JSONOutputEnabled() { diff --git a/src/control/cmd/dmg/pool_test.go b/src/control/cmd/dmg/pool_test.go index 67bf72bf54e8..84e5dda95218 100644 --- a/src/control/cmd/dmg/pool_test.go +++ b/src/control/cmd/dmg/pool_test.go @@ -1025,54 +1025,6 @@ func TestPoolCommands(t *testing.T) { }, " "), nil, }, - { - "Query pool with UUID and disabled ranks", - "pool query --show-disabled 12345678-1234-1234-1234-1234567890ab", - strings.Join([]string{ - printRequest(t, &control.PoolQueryReq{ - ID: "12345678-1234-1234-1234-1234567890ab", - QueryMask: setQueryMask(func(qm *daos.PoolQueryMask) { qm.SetOptions(daos.PoolQueryOptionDisabledEngines) }), - }), - }, " "), - nil, - }, - { - "Query pool with UUID and disabled ranks", - "pool query -b 12345678-1234-1234-1234-1234567890ab", - strings.Join([]string{ - printRequest(t, &control.PoolQueryReq{ - ID: "12345678-1234-1234-1234-1234567890ab", - QueryMask: setQueryMask(func(qm *daos.PoolQueryMask) { qm.SetOptions(daos.PoolQueryOptionDisabledEngines) }), - }), - }, " "), - nil, - }, - { - "Query pool with UUID, enabled ranks and disabled ranks", - "pool query --show-disabled --show-enabled 12345678-1234-1234-1234-1234567890ab", - strings.Join([]string{ - printRequest(t, &control.PoolQueryReq{ - ID: "12345678-1234-1234-1234-1234567890ab", - QueryMask: setQueryMask(func(qm *daos.PoolQueryMask) { - qm.SetOptions(daos.PoolQueryOptionEnabledEngines, daos.PoolQueryOptionDisabledEngines) - }), - }), - }, " "), - nil, - }, - { - "Query pool with UUID, enabled ranks and disabled ranks", - "pool query -b -e 12345678-1234-1234-1234-1234567890ab", - strings.Join([]string{ - printRequest(t, &control.PoolQueryReq{ - ID: "12345678-1234-1234-1234-1234567890ab", - QueryMask: setQueryMask(func(qm *daos.PoolQueryMask) { - qm.SetOptions(daos.PoolQueryOptionEnabledEngines, daos.PoolQueryOptionDisabledEngines) - }), - }), - }, " "), - nil, - }, { "Query pool for health only", "pool query --health-only 12345678-1234-1234-1234-1234567890ab", diff --git a/src/control/lib/control/pool_test.go b/src/control/lib/control/pool_test.go index 6849b88b0536..c111ba183cca 100644 --- a/src/control/lib/control/pool_test.go +++ b/src/control/lib/control/pool_test.go @@ -807,7 +807,7 @@ func TestControl_PoolQueryResp_MarshalJSON(t *testing.T) { UpgradeLayoutVer: 8, }, }, - exp: `{"query_mask":"rebuild,space","state":"Ready","uuid":"` + poolUUID.String() + `","total_targets":1,"active_targets":2,"total_engines":3,"disabled_targets":4,"version":5,"svc_ldr":6,"svc_reps":[0,1,2],"rebuild":null,"tier_stats":null,"pool_layout_ver":7,"upgrade_layout_ver":8,"status":42}`, + exp: `{"query_mask":"disabled_engines,rebuild,space","state":"Ready","uuid":"` + poolUUID.String() + `","total_targets":1,"active_targets":2,"total_engines":3,"disabled_targets":4,"version":5,"svc_ldr":6,"svc_reps":[0,1,2],"rebuild":null,"tier_stats":null,"pool_layout_ver":7,"upgrade_layout_ver":8,"status":42}`, }, "valid rankset": { pqr: &PoolQueryResp{ @@ -829,7 +829,7 @@ func TestControl_PoolQueryResp_MarshalJSON(t *testing.T) { UpgradeLayoutVer: 8, }, }, - exp: `{"query_mask":"rebuild,space","state":"Ready","uuid":"` + poolUUID.String() + `","total_targets":1,"active_targets":2,"total_engines":3,"disabled_targets":4,"version":5,"svc_ldr":6,"svc_reps":[0,1,2],"rebuild":null,"tier_stats":null,"enabled_ranks":[0,1,2,3,5],"disabled_ranks":[],"pool_layout_ver":7,"upgrade_layout_ver":8,"status":42}`, + exp: `{"query_mask":"disabled_engines,rebuild,space","state":"Ready","uuid":"` + poolUUID.String() + `","total_targets":1,"active_targets":2,"total_engines":3,"disabled_targets":4,"version":5,"svc_ldr":6,"svc_reps":[0,1,2],"rebuild":null,"tier_stats":null,"enabled_ranks":[0,1,2,3,5],"disabled_ranks":[],"pool_layout_ver":7,"upgrade_layout_ver":8,"status":42}`, }, } { t.Run(name, func(t *testing.T) { diff --git a/src/control/lib/daos/pool.go b/src/control/lib/daos/pool.go index fe44a00e210a..e47e6e2b23df 100644 --- a/src/control/lib/daos/pool.go +++ b/src/control/lib/daos/pool.go @@ -104,7 +104,7 @@ type ( const ( // DefaultPoolQueryMask defines the default pool query mask. - DefaultPoolQueryMask = PoolQueryMask(^uint64(0) &^ (C.DPI_ENGINES_ENABLED | C.DPI_ENGINES_DISABLED)) + DefaultPoolQueryMask = PoolQueryMask(^uint64(0) &^ C.DPI_ENGINES_ENABLED) // HealthOnlyPoolQueryMask defines the mask for health-only queries. HealthOnlyPoolQueryMask = PoolQueryMask(^uint64(0) &^ (C.DPI_ENGINES_ENABLED | C.DPI_SPACE)) diff --git a/src/control/lib/daos/pool_test.go b/src/control/lib/daos/pool_test.go index e76f33f4c250..3ff1ca098d29 100644 --- a/src/control/lib/daos/pool_test.go +++ b/src/control/lib/daos/pool_test.go @@ -130,7 +130,7 @@ func TestDaos_PoolQueryMask(t *testing.T) { testMask: genTestMask(func(pqm *PoolQueryMask) { *pqm = DefaultPoolQueryMask }), - expString: genOptsStr(PoolQueryOptionRebuild, PoolQueryOptionSpace), + expString: genOptsStr(PoolQueryOptionDisabledEngines, PoolQueryOptionRebuild, PoolQueryOptionSpace), }, "health-only query mask": { testMask: genTestMask(func(pqm *PoolQueryMask) { diff --git a/src/tests/ftest/control/dmg_pool_query_ranks.py b/src/tests/ftest/control/dmg_pool_query_ranks.py index 2522e3fad3af..c84d2dc3c80b 100644 --- a/src/tests/ftest/control/dmg_pool_query_ranks.py +++ b/src/tests/ftest/control/dmg_pool_query_ranks.py @@ -47,37 +47,6 @@ def test_pool_query_ranks_basic(self): data["response"].get("enabled_ranks") ), ) - self.assertIsNone( - data["response"].get("disabled_ranks"), - "Invalid disabled_ranks field: want=None, got={}".format( - 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"].get("enabled_ranks"), - [0, 1, 2], - "Invalid enabled_ranks field: want=[0, 1, 2], got={}".format( - data["response"].get("enabled_ranks") - ), - ) - self.assertIsNone( - data["response"].get("disabled_ranks"), - "Invalid disabled_ranks field: want=None, got={}".format( - 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"].get("enabled_ranks"), - "Invalid enabled_ranks field: want=None, got={}".format( - data["response"].get("enabled_ranks") - ), - ) self.assertListEqual( data["response"].get("disabled_ranks"), [], @@ -86,10 +55,8 @@ def test_pool_query_ranks_basic(self): ), ) - 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.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], @@ -133,9 +100,7 @@ def test_pool_query_ranks_mgmt(self): disabled_ranks = sorted(disabled_ranks + [rank]) self.log.debug("Checking enabled ranks state information") - data = self.dmg.pool_query( - self.pool.identifier, show_enabled=True, show_disabled=True - ) + data = self.dmg.pool_query(self.pool.identifier, show_enabled=True) self.assertListEqual( data["response"].get("enabled_ranks"), enabled_ranks, @@ -175,9 +140,7 @@ def test_pool_query_ranks_mgmt(self): disabled_ranks.remove(rank) self.log.debug("Checking enabled ranks state information") - data = self.dmg.pool_query( - self.pool.identifier, show_enabled=True, show_disabled=True - ) + data = self.dmg.pool_query(self.pool.identifier, show_enabled=True) self.assertListEqual( data["response"].get("enabled_ranks"), enabled_ranks, diff --git a/src/tests/ftest/control/dmg_pool_query_test.py b/src/tests/ftest/control/dmg_pool_query_test.py index ec8c56c62608..b7c83b59b553 100644 --- a/src/tests/ftest/control/dmg_pool_query_test.py +++ b/src/tests/ftest/control/dmg_pool_query_test.py @@ -62,6 +62,7 @@ def test_pool_query_basic(self): "uuid": self.pool.uuid.lower(), "total_targets": self.params.get("total_targets", path="/run/exp_vals/*"), "active_targets": self.params.get("active_targets", path="/run/exp_vals/*"), + "disabled_ranks": self.params.get("disabled_ranks", path="/run/exp_vals/*"), "total_engines": self.params.get("total_engines", path="/run/exp_vals/*"), "disabled_targets": self.params.get("disabled_targets", path="/run/exp_vals/*"), "version": self.params.get("version", path="/run/exp_vals/*"), diff --git a/src/tests/ftest/control/dmg_pool_query_test.yaml b/src/tests/ftest/control/dmg_pool_query_test.yaml index 8c92ae36945b..9b105da40ff4 100644 --- a/src/tests/ftest/control/dmg_pool_query_test.yaml +++ b/src/tests/ftest/control/dmg_pool_query_test.yaml @@ -28,12 +28,13 @@ exp_vals: pool_state: "Ready" total_targets: 4 active_targets: 4 + disabled_ranks: [] total_engines: 1 disabled_targets: 0 version: 1 leader: 0 replicas: [0] - query_mask: "rebuild,space" + query_mask: "disabled_engines,rebuild,space" scm: total: 16000008192 nvme: diff --git a/src/tests/ftest/deployment/network_failure.py b/src/tests/ftest/deployment/network_failure.py index ec67962c3d58..84c7b2593707 100644 --- a/src/tests/ftest/deployment/network_failure.py +++ b/src/tests/ftest/deployment/network_failure.py @@ -227,7 +227,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) + output = dmg_cmd.pool_query(pool=self.pool.identifier) disabled_ranks = output["response"].get("disabled_ranks") self.log.info("Disabled ranks = %s", disabled_ranks) diff --git a/src/tests/ftest/deployment/server_rank_failure.py b/src/tests/ftest/deployment/server_rank_failure.py index d2156d8fc8db..d1637fe54eb5 100644 --- a/src/tests/ftest/deployment/server_rank_failure.py +++ b/src/tests/ftest/deployment/server_rank_failure.py @@ -183,7 +183,7 @@ def verify_rank_failure(self, ior_namespace): errors.append("Server rank {} state isn't joined!".format(member["rank"])) # 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) + output = self.get_dmg_command().pool_query(pool=self.pool.identifier) disabled_ranks = output["response"].get("disabled_ranks") self.log.info("Disabled ranks = %s", disabled_ranks) diff --git a/src/tests/ftest/util/dmg_utils.py b/src/tests/ftest/util/dmg_utils.py index effc3172bac9..a48b45e59ddd 100644 --- a/src/tests/ftest/util/dmg_utils.py +++ b/src/tests/ftest/util/dmg_utils.py @@ -625,13 +625,12 @@ def pool_create(self, scm_size, uid=None, gid=None, nvme_size=None, return data - def pool_query(self, pool, show_enabled=False, show_disabled=False): + def pool_query(self, pool, show_enabled=False): """Query a pool with the dmg command. Args: pool (str): Pool UUID or label to query. show_enabled (bool, optional): Display enabled ranks. - show_disabled (bool, optional): Display disabled ranks. Raises: CommandFailure: if the dmg pool query command fails. @@ -677,8 +676,7 @@ def pool_query(self, pool, show_enabled=False, show_disabled=False): # "error": null, # "status": 0 # } - return self._get_json_result(("pool", "query"), pool=pool, - show_enabled=show_enabled, show_disabled=show_disabled) + return self._get_json_result(("pool", "query"), pool=pool, show_enabled=show_enabled) def pool_query_targets(self, pool, rank=None, target_idx=None): """Call dmg pool query-targets. diff --git a/src/tests/ftest/util/dmg_utils_base.py b/src/tests/ftest/util/dmg_utils_base.py index 39109320af11..951694a2251f 100644 --- a/src/tests/ftest/util/dmg_utils_base.py +++ b/src/tests/ftest/util/dmg_utils_base.py @@ -533,7 +533,6 @@ def __init__(self): super().__init__("/run/dmg/pool/query/*", "query") self.pool = BasicParameter(None, position=1) self.show_enabled = FormattedParameter("--show-enabled", False) - self.show_disabled = FormattedParameter("--show-disabled", False) class QueryTargetsSubCommand(CommandWithParameters): """Defines an object for the dmg pool query-targets command.""" diff --git a/src/tests/ftest/util/test_utils_pool.py b/src/tests/ftest/util/test_utils_pool.py index fbb6484e2925..7c6b5758f87b 100644 --- a/src/tests/ftest/util/test_utils_pool.py +++ b/src/tests/ftest/util/test_utils_pool.py @@ -700,12 +700,11 @@ def overwrite_acl(self): self.log.error("self.acl_file isn't defined!") @fail_on(CommandFailure) - def query(self, show_enabled=False, show_disabled=False): + def query(self, show_enabled=False): """Execute dmg pool query. Args: show_enabled (bool, optional): Display enabled ranks. - show_disabled (bool, optional): Display disabled ranks. Returns: dict: the dmg json command output converted to a python dictionary @@ -720,7 +719,7 @@ def query(self, show_enabled=False, show_disabled=False): while True: try: - return self.dmg.pool_query(self.identifier, show_enabled, show_disabled) + return self.dmg.pool_query(self.identifier, show_enabled) except CommandFailure as error: if end_time is None: @@ -1119,19 +1118,18 @@ def pool_percentage_used(self): } return pool_percent - def set_query_data(self, show_enabled=False, show_disabled=False): + def set_query_data(self, show_enabled=False): """Execute dmg pool query and store the results. Args: show_enabled (bool, optional): Display enabled ranks. - show_disabled (bool, optional): Display disabled ranks. Raises: TestFail: if the dmg pool query command failed """ self.query_data = {} - self.query_data = self.query(show_enabled, show_disabled) + self.query_data = self.query(show_enabled) def _get_query_data_keys(self, *keys, refresh=False): """Get the pool version from the dmg pool query output.