Janunez/daos 16627 by jamesanunez · Pull Request #18955 · daos-stack/daos

jamesanunez · 2026-08-27T18:42:04Z

Steps for the author:

Commit message follows the guidelines.
Appropriate Features or Test-tag pragmas were used.
Appropriate Functional Test Stages were run.
At least two positive code reviews including at least one code owner from each category referenced in the PR.
Testing is complete. If necessary, forced-landing label added and a reason added in a comment.

After all prior steps are complete:

Gatekeeper requested (daos-gatekeeper added as a reviewer).

daltonbohning

Sorry, I started reviewing this before noticing it was switched to draft :)
I'll post my comments but feel free to ignore since you're still working on this

daltonbohning · 2026-08-27T18:45:27Z

+  system_ram_reserved: 2
+pool:
+  size: 90%
+  svcn: 1


Recommend not setting this unless specifically needed. The default pool svcn will be >1 when there are enough servers

daltonbohning · 2026-08-27T18:45:37Z

+  svcn: 1
+container:
+  type: POSIX
+  control_method: daos


Don't need this anymore

Suggested change

control_method: daos

daltonbohning · 2026-08-27T18:45:58Z

+  client_processes:
+    np_16:
+      np: 16


Could simplify

Suggested change

client_processes:

np_16:

np: 16

client_processes:

np: 16

daltonbohning · 2026-08-27T18:46:13Z

+    np_16:
+      np: 16
+  test_file: testFile
+  repetitions: 1


Not needed because this is the default

Suggested change

repetitions: 1

daltonbohning · 2026-08-27T18:46:33Z

+  iorflags:
+    flags: "-v -k -e -w -r -R -G 27"


Probably can simplify this?

Suggested change

iorflags:

flags: "-v -k -e -w -r -R -G 27"

flags: "-v -k -e -w -r -R -G 27"

daltonbohning · 2026-08-27T18:46:45Z

+  objectclass:
+    oclass_SX:
+      dfs_oclass: "SX"


Suggested change

objectclass:

oclass_SX:

dfs_oclass: "SX"

dfs_oclass: "SX"

daltonbohning · 2026-08-27T18:48:03Z

+        self.log_step("Scanning DAOS control logs for URI lines")
+
+        line_count = 0
+        txt = f"URI to {provider}".replace('+', '\+')


You should escape the escape character since you want a literal \ in the string

Suggested change

txt = f"URI to {provider}".replace('+', '\+')

txt = f"URI to {provider}".replace('+', '\\+')

I'll integrate your suggestion.

github-actions · 2026-08-27T18:52:26Z

Errors are component not formatted correctly,Ticket number prefix incorrect,PR title is malformatted. See https://daosio.atlassian.net/wiki/spaces/DC/pages/11133911069/Commit+Comments,Unable to load ticket data
https://daosio.atlassian.net/browse/Janunez/daos

Test the changing fabric provider without reformatting storage feature. The test writes with IOR, changes the fabric provider and read the data written for correctness. The second test does the same and then reverts the system provider back to the original provider. Skip-unit-test: true Skip-fault-injection-test: true Test-tag: test_change_fabric_provider Test-tag: test_change_fabric_provider_and_revert Signed-off-by: James Nunez <james.nunez@hpe.com>

jamesanunez · 2026-08-27T20:51:55Z

+  svcn: 1
+container:
+  type: POSIX
+  control_method: daos


As Dalton suggested: I need to remove the 'control_method' line.

jamesanunez · 2026-08-27T20:52:24Z

+  repetitions: 1
+  read_flags: "-v -C -k -e -r -R -G 27"
+  iorflags:
+    flags: "-v -k -e -w -r -R -G 27"


Simplify as Dalton suggested: simplify into single 'flags: "-v -k -e -w -r -R -G 27"' line

jamesanunez

Need to update YAML file with Dalton's suggestions.

jamesanunez · 2026-08-27T20:54:47Z

+  system_ram_reserved: 2
+pool:
+  size: 90%
+  svcn: 1


As Dalton suggested: Remove 'svcn' line.

jamesanunez · 2026-08-27T20:56:05Z

+  api: POSIX
+  client_processes:
+    np_16:
+      np: 16


As Dalton suggested: simplify lines with single line with 'np: 16'

jamesanunez · 2026-08-27T20:56:49Z

+    np_16:
+      np: 16
+  test_file: testFile
+  repetitions: 1


As Dalton suggested: remove repetitions line

daltonbohning · 2026-08-27T20:56:10Z

+                for line in data.stdout:
+                    self.log.info("%s", line)
+                    line_count += 1


I think this might already be logged by default? We could view a test log to double check

daltonbohning · 2026-08-27T21:07:36Z

+                client_ok = provider in line
+            elif re.match(r"^\d+\s+\S+://", line) and provider not in line:
+                ranks_ok = False
+        return client_ok and ranks_ok


It would be helpful to give an example of what r"^\d+\s+\S+://" matches on.
Also, there might be a gap here. What if NO lines match that expression, and so therefore ranks_ok = True by default?

daltonbohning · 2026-08-27T21:08:13Z

+        common_providers = get_common_provider(self.log,
+                                               self.host_info.all_hosts, self.test_env.interface)


It's a preference, but we tend to do this instead

Suggested change

common_providers = get_common_provider(self.log,

self.host_info.all_hosts, self.test_env.interface)

common_providers = get_common_provider(

self.log, self.host_info.all_hosts, self.test_env.interface)

daltonbohning · 2026-08-27T21:19:40Z

+        # Find a different provider than the input provider
+        new_provider = None
+        for provider in common_providers:
+            if current_provider not in provider and provider in SUPPORTED_PROVIDERS:
+                new_provider = provider
+                break
+
+        if new_provider is None:
+            self.fail(f'No alternative provider found; available: {common_providers}, '
+                      f'current: {current_provider}')
+
+        return new_provider


This is partly preference too, but if you are familiar with set logic you can do this like

alternate_providers = ( set(SUPPORTED_PROVIDERS) .intersection(common_providers) .difference(set([current_provider])) ) if not alternate_providers: # error # Return a random provider return self.random.choice(alternate_providers)

daltonbohning · 2026-08-27T21:23:26Z

+        self.log.info(f'Initial IOR write with provider '
+                      f'{original_provider} completed successfully')
+
+        # Work around for pool.connect() issue


Maybe mention something like "the pool handle is not valid after server restart"

daltonbohning · 2026-08-27T21:27:05Z

+        # Restart the DAOS servers and agents with the new config.
+        self._restart_servers_and_agents(new_provider)
+
+        # Verify the provider was changed
+        current_provider = self.server_managers[0].manager.job.yaml.get_value('provider')
+        if current_provider != new_provider:
+            self.fail(f'Provider change failed: expected {new_provider}, '
+                      f'actual {current_provider}')


self.server_managers[0].manager.job.yaml.get_value is what the framework uses to generate the config. So just because that is correct does not necessarily mean the server DID come up with that provider.
So I would recommend either

Remove this and just rely on the dump_attachinfo check you already have

Put this check before the _restart_servers_and_agents so it's clear that it's just verifying our own code

rpadma2

Simple suggestions:

Add a title for the PR rather than just branch information. (Example : Basic test for changing fabric provider).
Add the test tags as part of git commit message... You don't need to run all the test stages since this is focused only on the functional test stage... Example.

Skip-func-hw-test-medium: false
Skip-func-hw-test-medium-md-on-ssd: false
Skip-fault-injection-test: true
Skip-unit-test: true
Skip-nlt: true
Test-tag: test_change_fabric_provider

daltonbohning · 2026-08-28T15:40:26Z

Simple suggestions:

Add a title for the PR rather than just branch information. (Example : Basic test for changing fabric provider).

Add the test tags as part of git commit message... You don't need to run all the test stages since this is focused only on the functional test stage... Example.

Skip-func-hw-test-medium: false Skip-func-hw-test-medium-md-on-ssd: false Skip-fault-injection-test: true Skip-unit-test: true Skip-nlt: true Test-tag: test_change_fabric_provider

The Test-tag and skips are determined automatically so this PR does not need to specify any of those pragmas.
If you look here
https://jenkins-3.daos.hpc.amslabs.hpecorp.net/blue/rest/organizations/jenkins/pipelines/daos-stack/pipelines/daos/branches/PR-18955/runs/2/nodes/39/log/?start=0
CI set these by default

[2026-08-27T20:46:59.091Z] Test-tag: ChangingFabricProvider
[2026-08-27T20:46:59.091Z] Skip-unit-tests: True
[2026-08-27T20:46:59.091Z] Skip-fault-injection-test: True

So @jamesanunez at most you can just use Quick-functional: true since you are only modifying exactly this one test.

daosbuild3 · 2026-08-29T10:26:45Z

Test stage Functional Hardware Medium MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-18955/2/execution/node/1061/log

jamesanunez requested review from a team as code owners August 27, 2026 18:42

jamesanunez marked this pull request as draft August 27, 2026 18:44

daltonbohning reviewed Aug 27, 2026

View reviewed changes

jamesanunez force-pushed the janunez/DAOS-16627 branch from 1c2a07b to 005ffee Compare August 27, 2026 20:45

jamesanunez marked this pull request as ready for review August 27, 2026 20:47

jamesanunez commented Aug 27, 2026

View reviewed changes

daltonbohning reviewed Aug 27, 2026

View reviewed changes

rpadma2 reviewed Aug 28, 2026

View reviewed changes

	iorflags:
	flags: "-v -k -e -w -r -R -G 27"
	flags: "-v -k -e -w -r -R -G 27"

	txt = f"URI to {provider}".replace('+', '\+')
	txt = f"URI to {provider}".replace('+', '\\+')

		common_providers = get_common_provider(self.log,
		self.host_info.all_hosts, self.test_env.interface)

Uh oh!

Conversation

jamesanunez commented Aug 27, 2026

Steps for the author:

After all prior steps are complete:

Uh oh!

daltonbohning left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Aug 27, 2026

Uh oh!

jamesanunez Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jamesanunez Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jamesanunez left a comment

Choose a reason for hiding this comment

Uh oh!

jamesanunez Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jamesanunez Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rpadma2 left a comment

Choose a reason for hiding this comment

Uh oh!

daltonbohning commented Aug 28, 2026

Uh oh!

daosbuild3 commented Aug 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

jamesanunez Aug 27, 2026 •

edited

Loading

jamesanunez Aug 27, 2026 •

edited

Loading

jamesanunez Aug 27, 2026 •

edited

Loading

jamesanunez Aug 27, 2026 •

edited

Loading