Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DAOS-10830 test: Add recovery/ddb.py to test ddb #9875

Merged
merged 24 commits into from
Sep 9, 2022

Conversation

shimizukko
Copy link
Contributor

Test the following ddb subcommands:
ls: test_ls()
rm: test_rm()
load: test_load()
dump_value: test_dump_value()

See code or the ticket for the test steps.

Add container_list_objects() in daos_utils.py

Skip-unit-tests: true
Skip-fault-injection-test: true
Test-tag: ddb
Signed-off-by: Makito Kano [email protected]

Test the following ddb subcommands:
ls: test_ls()
rm: test_rm()
load: test_load()
dump_value: test_dump_value()

See code or the ticket for the test steps.

Add container_list_objects() in daos_utils.py

Skip-unit-tests: true
Skip-fault-injection-test: true
Test-tag: ddb
Signed-off-by: Makito Kano <[email protected]>
Skip-unit-tests: true
Skip-fault-injection-test: true
Test-tag: ddb
Signed-off-by: Makito Kano <[email protected]>
@github-actions
Copy link

github-actions bot commented Aug 1, 2022

Bug-tracker data:
Ticket title is 'Test ddb - DAOS Debug Tool'
Status is 'In Review'
https://daosio.atlassian.net/browse/DAOS-10830

Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

src/tests/ftest/cat_recovery/ddb.py Outdated Show resolved Hide resolved
src/tests/ftest/cat_recovery/ddb.py Outdated Show resolved Hide resolved
@daosbuild1
Copy link
Collaborator

Skip-unit-tests: true
Skip-fault-injection-test: true
Test-tag: ddb
Signed-off-by: Makito Kano <[email protected]>
@daosbuild1
Copy link
Collaborator

Test stage Build RPM on EL 8 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-9875/1/execution/node/318/log

@daosbuild1 daosbuild1 dismissed their stale review August 1, 2022 17:30

Updated patch

Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. No errors found by checkpatch.

@daosbuild1
Copy link
Collaborator

Test stage Build RPM on EL 8 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-9875/2/execution/node/322/log

@daosbuild1
Copy link
Collaborator

Test stage Build DEB on Ubuntu 20.04 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-9875/2/execution/node/319/log

@daosbuild1
Copy link
Collaborator

Test stage Build RPM on Leap 15 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-9875/2/execution/node/316/log

@daosbuild1
Copy link
Collaborator

Test stage Build on Leap 15 with Intel-C and TARGET_PREFIX completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-9875/2/execution/node/344/log

@daosbuild1
Copy link
Collaborator

Test stage Build on CentOS 7 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-9875/2/execution/node/358/log

@daosbuild1
Copy link
Collaborator

Test stage Build RPM on EL 8 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-9875/3/execution/node/168/log

@daosbuild1
Copy link
Collaborator

Test stage Build on Leap 15 with Intel-C and TARGET_PREFIX completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-9875/3/execution/node/171/log

@daosbuild1
Copy link
Collaborator

Test stage Build RPM on Leap 15 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-9875/3/execution/node/235/log

@daosbuild1
Copy link
Collaborator

Test stage Build DEB on Ubuntu 20.04 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-9875/3/execution/node/238/log

Skip-unit-tests: true
Skip-fault-injection-test: true
Test-tag: ddb
Signed-off-by: Makito Kano <[email protected]>
Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. No errors found by checkpatch.

@daosbuild1
Copy link
Collaborator

Test stage Build RPM on EL 8 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-9875/4/execution/node/352/log

@daosbuild1
Copy link
Collaborator

Test stage Build RPM on Leap 15 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-9875/4/execution/node/346/log

@daosbuild1
Copy link
Collaborator

Test stage Build on Leap 15 with Intel-C and TARGET_PREFIX completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-9875/4/execution/node/356/log

@daosbuild1
Copy link
Collaborator

Test stage Build DEB on Ubuntu 20.04 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-9875/4/execution/node/321/log

@daosbuild1
Copy link
Collaborator

Test stage Build on CentOS 7 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-9875/4/execution/node/358/log

Skip-unit-tests: true
Skip-fault-injection-test: true
Test-tag: ddb
Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. No errors found by checkpatch.

Skip-unit-tests: true
Skip-fault-injection-test: true
Test-tag: ddb_load
Signed-off-by: Makito Kano <[email protected]>
Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

src/tests/ftest/cat_recovery/ddb.py Outdated Show resolved Hide resolved
@daosbuild1
Copy link
Collaborator

Skip-unit-tests: true
Skip-fault-injection-test: true
Test-tag: test_pool_info_query ddb
Required-githooks: true
Signed-off-by: Makito Kano <[email protected]>
Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. No errors found by checkpatch.

@shimizukko shimizukko marked this pull request as ready for review August 24, 2022 11:44
@shimizukko shimizukko requested a review from a team as a code owner August 24, 2022 11:44
Copy link
Contributor

@daltonbohning daltonbohning left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Recommend fixing the spelling errors if you need to repush

@@ -10,7 +10,7 @@ def scons():

env.Install(ftest_install_dir, Glob('*.*'))

dirs = ['aggregation', 'fault_injection', 'checksum',
dirs = ['aggregation', 'fault_injection', 'cat_recovery', 'checksum',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you feel about simplifying the new directory name to recovery?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Comment on lines 611 to 612
pool (str): Pool UUID.
cont (str): Container UUID.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion - commands accept UUID or labels

Suggested change
pool (str): Pool UUID.
cont (str): Container UUID.
pool (str): Pool UUID or label
cont (str): Container UUID or label

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Comment on lines 19 to 21
SAMPLE_DKEY = "Sample dkey"
SAMPLE_AKEY = "Sample akey"
SAMPLE_DATA = "Sample data"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we put these in the config? Or maybe generate random strings?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the code to generate random string for these.

def insert_objects(self, object_count, dkey_count, akey_count):
"""Insert objects, dkeys, akeys, and data into the container.

Inserted objects: self.ioreqs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should fix this spelling error. Maybe by adding an entry to words.dict

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

self.akeys = []
self.data_list = []

def insert_objects(self, object_count, dkey_count, akey_count):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this function be genericized and moved to a util file?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Comment on lines 155 to 158
:avocado: tags=all,weekly_regression
:avocado: tags=vm
:avocado: tags=cat_rec
:avocado: tags=ddb,ddb_ls
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recommend cat_recovery or just recovery as the feature tag. And adding the unique test name tag. Similar for others in this file

Suggested change
:avocado: tags=all,weekly_regression
:avocado: tags=vm
:avocado: tags=cat_rec
:avocado: tags=ddb,ddb_ls
:avocado: tags=all,weekly_regression
:avocado: tags=vm
:avocado: tags=recovery
:avocado: tags=ddb,test_recovery_ddb_ls

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Comment on lines 186 to 189
# stdout is a list which contains each line as separate element. Concatenate them
# to single string so that we can apply regex.
ls_out = "\n".join(cmd_result[0]["stdout"])
match = re.findall(r"(\[\d+\])\s+([a-f0-9\-]+)", ls_out)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should be able to just do this

Suggested change
# stdout is a list which contains each line as separate element. Concatenate them
# to single string so that we can apply regex.
ls_out = "\n".join(cmd_result[0]["stdout"])
match = re.findall(r"(\[\d+\])\s+([a-f0-9\-]+)", ls_out)
match = re.findall(r"(\[\d+\])\s+([a-f0-9\-]+)", cmd_result[0]["stdout"], re.MULTILINE)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cmd_result[0]["stdout"] is a list that contains each line as separate element, so this approach didn't work.

match = re.findall(r"(\[\d+\])\s+([a-f0-9\-]+)", ls_out)
self.log.info("List container match = %s", match)

actual_uuid = match[0][1].lower()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you use re.search, you'll only get the first match, which is what you're using here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Comment on lines 208 to 210
ls_out = "\n".join(cmd_result[0]["stdout"])
match = re.findall(r"(\[\d+\])\s+\'(\d+\.\d+)\'", ls_out)
self.log.info("List objects match = %s", match)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ls_out = "\n".join(cmd_result[0]["stdout"])
match = re.findall(r"(\[\d+\])\s+\'(\d+\.\d+)\'", ls_out)
self.log.info("List objects match = %s", match)
match = re.findall(r"(\[\d+\])\s+\'(\d+\.\d+)\'", cmd_result[0]["stdout"], re.MULTILINE)
self.log.info("List objects match = %s", match)

Comment on lines 561 to 562
data_size = len(new_data)
data_size += 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
data_size = len(new_data)
data_size += 1
data_size = len(new_data) + 1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

…ral_utils.py

Rename file, test methods, and test tags.
Generate random base string for dkey, akey, and data.
Fix pylint.

Skip-unit-tests: true
Skip-fault-injection-test: true
Test-tag: test_pool_info_query ddb
Required-githooks: true

Signed-off-by: Makito Kano <[email protected]>
Skip-unit-tests: true
Skip-fault-injection-test: true
Test-tag: test_pool_info_query ddb
Required-githooks: true
Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. No errors found by checkpatch.

FYI: Errors found in lines not modified in the patch:

src/tests/ftest/util/general_utils.py:162:
(pylint-consider-using-dict-items) Consider iterating with .items()

src/tests/ftest/util/general_utils.py:267:
(pylint-unexpected-keyword-arg) Unexpected keyword argument 'allow_output_check' in function call

src/tests/ftest/util/general_utils.py:204:
(pylint-inconsistent-return-statements) Either all return statements in a function should return an expression, or none of them should.

src/tests/ftest/util/general_utils.py:481:
(pylint-consider-using-dict-items) Consider iterating with .items()

src/tests/ftest/util/general_utils.py:491:
(pylint-consider-using-dict-items) Consider iterating with .items()

Skip-unit-tests: true
Skip-fault-injection-test: true
Test-tag: test_pool_info_query ddb
Required-githooks: true
Signed-off-by: Makito Kano <[email protected]>
Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. No errors found by checkpatch.

FYI: Errors found in lines not modified in the patch:

src/tests/ftest/util/general_utils.py:162:
(pylint-consider-using-dict-items) Consider iterating with .items()

src/tests/ftest/util/general_utils.py:267:
(pylint-unexpected-keyword-arg) Unexpected keyword argument 'allow_output_check' in function call

src/tests/ftest/util/general_utils.py:204:
(pylint-inconsistent-return-statements) Either all return statements in a function should return an expression, or none of them should.

src/tests/ftest/util/general_utils.py:481:
(pylint-consider-using-dict-items) Consider iterating with .items()

src/tests/ftest/util/general_utils.py:491:
(pylint-consider-using-dict-items) Consider iterating with .items()

Skip-unit-tests: true
Skip-fault-injection-test: true
Test-tag: test_pool_info_query ddb
Required-githooks: true
Signed-off-by: Makito Kano <[email protected]>
Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. No errors found by checkpatch.

FYI: Errors found in lines not modified in the patch:

src/tests/ftest/util/general_utils.py:162:
(pylint-consider-using-dict-items) Consider iterating with .items()

src/tests/ftest/util/general_utils.py:267:
(pylint-unexpected-keyword-arg) Unexpected keyword argument 'allow_output_check' in function call

src/tests/ftest/util/general_utils.py:204:
(pylint-inconsistent-return-statements) Either all return statements in a function should return an expression, or none of them should.

src/tests/ftest/util/general_utils.py:481:
(pylint-consider-using-dict-items) Consider iterating with .items()

src/tests/ftest/util/general_utils.py:491:
(pylint-consider-using-dict-items) Consider iterating with .items()

Copy link
Contributor

@daltonbohning daltonbohning left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, except some minor comments

Comment on lines +1605 to +1607
except DaosTestError as error:
print("ERROR: Copying {} from {}: {}".format(remote_file_path, remote, error))
raise error
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if we should be using print here.
@phender Might have a suggestion.

Comment on lines 87 to 92
object_count = 5
dkey_count = 2
akey_count = 1
insert_objects(
context=self.context, container=self.container, object_count=5,
dkey_count=2, akey_count=1, base_dkey=self.random_dkey,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not pass the variables?

Suggested change
object_count = 5
dkey_count = 2
akey_count = 1
insert_objects(
context=self.context, container=self.container, object_count=5,
dkey_count=2, akey_count=1, base_dkey=self.random_dkey,
object_count = 5
dkey_count = 2
akey_count = 1
insert_objects(
context=self.context, container=self.container, object_count=object_count ,
dkey_count=dkey_count, akey_count=akey_count , base_dkey=self.random_dkey,

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

Skip-unit-tests: true
Skip-fault-injection-test: true
Test-tag: test_pool_info_query ddb
Required-githooks: true
Signed-off-by: Makito Kano <[email protected]>
Skip-unit-tests: true
Skip-fault-injection-test: true
Test-tag: test_pool_info_query ddb
Required-githooks: true
Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. No errors found by checkpatch.

FYI: Errors found in lines not modified in the patch:

src/tests/ftest/util/general_utils.py:162:
(pylint-consider-using-dict-items) Consider iterating with .items()

src/tests/ftest/util/general_utils.py:267:
(pylint-unexpected-keyword-arg) Unexpected keyword argument 'allow_output_check' in function call

src/tests/ftest/util/general_utils.py:204:
(pylint-inconsistent-return-statements) Either all return statements in a function should return an expression, or none of them should.

src/tests/ftest/util/general_utils.py:481:
(pylint-consider-using-dict-items) Consider iterating with .items()

src/tests/ftest/util/general_utils.py:491:
(pylint-consider-using-dict-items) Consider iterating with .items()

Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. No errors found by checkpatch.

FYI: Errors found in lines not modified in the patch:

src/tests/ftest/util/general_utils.py:162:
(pylint-consider-using-dict-items) Consider iterating with .items()

src/tests/ftest/util/general_utils.py:267:
(pylint-unexpected-keyword-arg) Unexpected keyword argument 'allow_output_check' in function call

src/tests/ftest/util/general_utils.py:204:
(pylint-inconsistent-return-statements) Either all return statements in a function should return an expression, or none of them should.

src/tests/ftest/util/general_utils.py:481:
(pylint-consider-using-dict-items) Consider iterating with .items()

src/tests/ftest/util/general_utils.py:491:
(pylint-consider-using-dict-items) Consider iterating with .items()

@daosbuild1
Copy link
Collaborator

Test stage Functional on EL 8 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-9875/21/execution/node/639/log

@shimizukko shimizukko changed the title DAOS-10830 test: Add cat_recovery/ddb.py to test ddb DAOS-10830 test: Add recovery/ddb.py to test ddb Aug 31, 2022
Skip-unit-tests: true
Skip-fault-injection-test: true
Test-tag: test_pool_info_query ddb
Required-githooks: true
Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. No errors found by checkpatch.

FYI: Errors found in lines not modified in the patch:

src/tests/ftest/util/general_utils.py:162:
(pylint-consider-using-dict-items) Consider iterating with .items()

src/tests/ftest/util/general_utils.py:267:
(pylint-unexpected-keyword-arg) Unexpected keyword argument 'allow_output_check' in function call

src/tests/ftest/util/general_utils.py:204:
(pylint-inconsistent-return-statements) Either all return statements in a function should return an expression, or none of them should.

src/tests/ftest/util/general_utils.py:481:
(pylint-consider-using-dict-items) Consider iterating with .items()

src/tests/ftest/util/general_utils.py:491:
(pylint-consider-using-dict-items) Consider iterating with .items()

return None # to appease pylint

def test_recovery_ddb_ls(self):
"""Test ddb ls.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might want to add JIRA DAOS-10830 to all the def test_.. description

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll update in the subsequent PR. Thanks.

@shimizukko shimizukko requested a review from a team September 2, 2022 15:54
@jolivier23 jolivier23 added the CR Catastrophic Recovery Feature label Sep 2, 2022
@mjmac mjmac merged commit f9b6133 into feature/cat_recovery Sep 9, 2022
@mjmac mjmac deleted the makito/DAOS-10830 branch September 9, 2022 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CR Catastrophic Recovery Feature
Development

Successfully merging this pull request may close these issues.

6 participants