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.

Sign up for GitHub

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

Jump to bottom

DAOS-15684 test: add test case for custom server name #14225

Merged
merged 3 commits into from
Apr 29, 2024
Merged

DAOS-15684 test: add test case for custom server name #14225

merged 3 commits into from
Apr 29, 2024

Conversation

daltonbohning
Copy link
Contributor

Test-tag: pr,vm DaosSystemQuery
Skip-unit-tests: true
Skip-fault-injection-test: true

Add a case for changing the default name of "daos_server" Fix bug when setting config name

Required-githooks: true

Before requesting gatekeeper:

  • Two review approvals and any prior change requests have been resolved.
  • Testing is complete and all tests passed or there is a reason documented in the PR why it should be force landed and forced-landing tag is set.
  • Features: (or Test-tag*) commit pragma was used or there is a reason documented that there are no appropriate tags for this PR.
  • Commit messages follows the guidelines outlined here.
  • Any tests skipped by the ticket being addressed have been run and passed in the PR.

Gatekeeper:

  • You are the appropriate gatekeeper to be landing the patch.
  • The PR has 2 reviews by people familiar with the code, including appropriate owners.
  • Githooks were used. If not, request that user install them and check copyright dates.
  • Checkpatch issues are resolved. Pay particular attention to ones that will show up on future PRs.
  • All builds have passed. Check non-required builds for any new compiler warnings.
  • Sufficient testing is done. Check feature pragmas and test tags and that tests skipped for the ticket are run and now pass with the changes.
  • If applicable, the PR has addressed any potential version compatibility issues.
  • Check the target branch. If it is master branch, should the PR go to a feature branch? If it is a release branch, does it have merge approval in the JIRA ticket.
  • Extra checks if forced landing is requested
    • Review comments are sufficiently resolved, particularly by prior reviewers that requested changes.
    • No new NLT or valgrind warnings. Check the classic view.
    • Quick-build or Quick-functional is not used.
  • Fix the commit message upon landing. Check the standard here. Edit it to create a single commit. If necessary, ask submitter for a new summary.

@daltonbohning
DAOS-15684 test: add test case for custom server name
d4f3fad 
Test-tag: pr,vm DaosSystemQuery
Skip-unit-tests: true
Skip-fault-injection-test: true

Add a case for changing the default name of "daos_server"
Fix bug when setting config name

Required-githooks: true

Signed-off-by: Dalton Bohning <[email protected]>
@daltonbohning daltonbohning self-assigned this Apr 23, 2024
@github-actions GitHub Actions
Copy link

Ticket title is 'ftest should allow changing the system name to something other than "daos_server"'
Status is 'In Progress'
https://daosio.atlassian.net/browse/DAOS-15684

@daltonbohning daltonbohning mentioned this pull request Apr 23, 2024
Closed
18 tasks
mchaarawi
mchaarawi reviewed Apr 23, 2024
View reviewed changes
src/tests/ftest/control/daos_system_query.yaml Outdated
timeout: 80
server_config:
name: daos_server
name: custom_daos_server # Use a non-default name
Copy link
Contributor

Choose a reason for hiding this comment

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

i am not sure this test is a good candidate. i do not see it exercising the agent and client side stack to prove that this works.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From a weekly run
https://build.hpdd.intel.com/job/daos-stack/job/daos/job/weekly-testing/379/artifact/Functional%20on%20EL%208/control/daos_system_query.py/job.log

The test runs

  • dmg system query
  • dmg storage format
  • daos system query

Do we need some real IO/app here for the agent+client? Or is daos system query enough?

Copy link
Contributor

@mchaarawi mchaarawi Apr 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

ah ok, daos system query should be good. i just did not see any agent_config in this yaml 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.

The infrastructure will setup an agent config automatically :)

Copy link
Contributor

Choose a reason for hiding this comment

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

this PR has been running for a long time. is it running all VM tests adn do they take that long?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's running Test-tag: pr,vm DaosSystemQuery which shouldn't take very long. It looks to me like the VM stage hasn't started? Or maybe Jenkins isn't reporting correctly

Copy link
Contributor

Choose a reason for hiding this comment

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

vm test does not usually take a very long time to get scheduled. so i doubt it's just waiting in the queue.
but i could be wrong

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 agree. Looks like PRs pushed after this one have gone through VM already. Let me try restarting

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Failed due to some CI issue. triggering again

@daosbuild1
Copy link
Collaborator

Test stage Functional on EL 8.8 completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-14225/1/display/redirect

@daosbuild1
Copy link
Collaborator

Test stage Functional on EL 8.8 completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-14225/2/display/redirect

@daosbuild1
Copy link
Collaborator

Test stage Functional on EL 8.8 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-14225/3/execution/node/359/log

@daltonbohning
Copy link
Contributor Author

Interesting...

https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-14225/3/artifact/Functional%20on%20EL%208.8/control/daos_system_query.py/job.log

  wolf-239vm2:
    -- Logs begin at Wed 2024-04-24 16:22:49 UTC, end at Wed 2024-04-24 17:31:30 UTC. --
    Apr 24 17:30:50 wolf-239vm2.wolf.hpdd.intel.com systemd[1]: Started DAOS Server.
    Apr 24 17:30:51 wolf-239vm2.wolf.hpdd.intel.com daos_server[117369]: ERROR: failed to load config from /etc/daos/daos_server.yml: invalid system name: "custom_daos_server"

The length limit is 15:

daos/src/include/daos_types.h

Line 31 in ddaf6ce

#define DAOS_SYS_NAME_MAX 15

Although this exists?

daos/src/include/daos_pool.h

Line 213 in ddaf6ce

#define DAOS_SYS_NAME_MAX_LEN 127

And is only used on one place:

daos/src/client/dfs/duns.c

Line 1380 in ddaf6ce

D_STRNDUP(attrp->da_sys, sys, DAOS_SYS_NAME_MAX_LEN);

@mchaarawi
remove duplicate sys length and use the control one
ffdce60 
Test-tag: pr,vm DaosSystemQuery
Skip-unit-tests: true
Skip-fault-injection-test: true

Required-githooks: true

Signed-off-by: Mohamad Chaarawi <[email protected]>
@mchaarawi
Copy link
Contributor

Interesting...

https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-14225/3/artifact/Functional%20on%20EL%208.8/control/daos_system_query.py/job.log

  wolf-239vm2:
    -- Logs begin at Wed 2024-04-24 16:22:49 UTC, end at Wed 2024-04-24 17:31:30 UTC. --
    Apr 24 17:30:50 wolf-239vm2.wolf.hpdd.intel.com systemd[1]: Started DAOS Server.
    Apr 24 17:30:51 wolf-239vm2.wolf.hpdd.intel.com daos_server[117369]: ERROR: failed to load config from /etc/daos/daos_server.yml: invalid system name: "custom_daos_server"

The length limit is 15:

daos/src/include/daos_types.h

Line 31 in ddaf6ce

#define DAOS_SYS_NAME_MAX 15

Although this exists?

daos/src/include/daos_pool.h

Line 213 in ddaf6ce

#define DAOS_SYS_NAME_MAX_LEN 127

And is only used on one place:

daos/src/client/dfs/duns.c

Line 1380 in ddaf6ce

D_STRNDUP(attrp->da_sys, sys, DAOS_SYS_NAME_MAX_LEN);

yes not sure how we got into that situation. anyway made a small update to remove that duplicate def and use the control one.

@daltonbohning
run pr and unit test since core was modified
a0a5dff 
Features: DaosSystemQuery

Required-githooks: true

Signed-off-by: Dalton Bohning <[email protected]>
@daltonbohning
Copy link
Contributor Author

daltonbohning commented Apr 24, 2024 •
edited
Loading

yes not sure how we got into that situation. anyway made a small update to remove that duplicate def and use the control one.

Thanks :) At risk of setting a bad example, I updated the pragmas to run unit tests and pr

@daltonbohning
Copy link
Contributor Author

Clean run:
https://build.hpdd.intel.com/blue/organizations/jenkins/daos-stack%2Fdaos/detail/PR-14225/6/pipeline

And the new test case shows it's working
https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-14225/6/artifact/Functional%20on%20EL%208.8/control/daos_system_query.py/job.log

2024-04-24 23:01:52,607 process          L0604 INFO | Running '/usr/bin/daos -j system query'
2024-04-24 23:01:52,844 process          L0416 DEBUG| [stdout] {
2024-04-24 23:01:52,845 process          L0416 DEBUG| [stdout]   "response": {
2024-04-24 23:01:52,846 process          L0416 DEBUG| [stdout]     "system_name": "other_dserver",
2024-04-24 23:01:52,846 process          L0416 DEBUG| [stdout]     "fabric_provider": "ofi+tcp",
2024-04-24 23:01:52,846 process          L0416 DEBUG| [stdout]     "rank_uris": [
2024-04-24 23:01:52,846 process          L0416 DEBUG| [stdout]       {
2024-04-24 23:01:52,846 process          L0416 DEBUG| [stdout]         "rank": 0,
2024-04-24 23:01:52,846 process          L0416 DEBUG| [stdout]         "uri": "ofi+tcp://10.8.4.23:31416"
2024-04-24 23:01:52,847 process          L0416 DEBUG| [stdout]       }
2024-04-24 23:01:52,847 process          L0416 DEBUG| [stdout]     ]
2024-04-24 23:01:52,847 process          L0416 DEBUG| [stdout]   },
2024-04-24 23:01:52,847 process          L0416 DEBUG| [stdout]   "error": null,
2024-04-24 23:01:52,847 process          L0416 DEBUG| [stdout]   "status": 0
2024-04-24 23:01:52,847 process          L0416 DEBUG| [stdout] }

@daltonbohning daltonbohning marked this pull request as ready for review April 29, 2024 15:39
@daltonbohning daltonbohning requested review from a team as code owners April 29, 2024 15:39
@daltonbohning daltonbohning requested a review from a team April 29, 2024 19:38
@phender phender merged commit 867c8ee into master Apr 29, 2024
52 checks passed
@phender phender deleted the dbohning/daos-15684-new-test branch April 29, 2024 19:39
@mjmac mjmac mentioned this pull request Apr 30, 2024
Closed
mjmac pushed a commit that referenced this pull request Apr 30, 2024
@daltonbohning @mchaarawi @mjmac
DAOS-15684 test: add test case for custom server name (#14225)
48e3b33 
Add a case for changing the default name of "daos_server"
Fix bug when setting config name

Signed-off-by: Dalton Bohning <[email protected]>
Co-authored-by: Mohamad Chaarawi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@phender phender phender approved these changes

@mchaarawi mchaarawi mchaarawi approved these changes

@shimizukko shimizukko Awaiting requested review from shimizukko

Assignees

@daltonbohning daltonbohning

Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@daltonbohning @daosbuild1 @mchaarawi @phender