Fix minor split shell issues by emmuhamm · Pull Request #935 · DUNE-DAQ/drunc

emmuhamm · 2026-06-01T10:38:36Z

Description

Fixes #926

Blocks boot behaviour in unified shell if another session (with the same name) is already booted
Asks for confirmation when booting in the process manager shell if user tries to spawn a session with the same name as an existing session
Multi host support works by mapping localhost instances to an actual host

Type of change

Optimization
Bug fix

List of required branches from other repositories

N/A

Suggested manual testing checklist

In three different shells

drunc-process-manager ssh-standalone 50000
drunc-process-manager-shell grpc://np04-srv-028:50000
drunc-unified-shell grpc://np04-srv-028:50000 config/daqsystemtest/example-configs.data.xml ehn1-local-1x1-config emir-test-1-tester

Try the following combination

same user same host
same user different host
different user same host
different user different host

it should all work for whatever the usual commands are you run in the drunc session if you use the ehn1 configuration.

there seems to be a problem when running with the local connectivity server, so any combination of different users will cause a failure for this. This is being tracked in #937

Developer checklist

Prior to marking this as "Ready for Review"

Tests ran on: NP04 - 028 from release 03 June 2026 nightly

Unit tests - some tests can't be ran on the CI. This is documented. If this PR checks a feature that can't be tested with CI, this has been marked appropriately.

Integration tests - the daqsystemtest_integtest_bundle requires a lot of resources, and connections to the EHN1 infrastructure. Check the cross referenced list if you can't run these. The developer needs to run at least the .

Unit tests (pytest --marker) passed
- With relevant marker
- Without marker
Integration tests passed
- Only daqsystemtest_integtest_bundle.sh -k minimal_system_quick_test.py
- Full daqsystemtest_integtest_bundle.sh
Testing skipped as there are no core code changes in this PR, this only relates to documentation/CI workflows
Drunc integration tests pass (./scripts/drunc_integtest_bundle.sh)

Final checklist prior to marking this as "Ready for Review"

Code is clearly commented.
New unit tests have been added, or is documented in # ISSUE NUMBER
A suitable reviewer has been chosen from this list.

Reviewer checklist

This branch has been rebased with develop prior to testing.
Suggested manual tests show changes.
CI workflows fails documented (if present)
Integration tests passed (on either np0x or IC HEP clusters)
- Use the following guidelines to determine which of the integration tests you need to run
  - You do not need to run any integration tests if
    - Code changes are not associated with src/
    - PR changes only affect docstrings
    - In this case, be sure to validate any suggested manual testing.
  - Run only the minimum integration test as daqsystemtest_integtest_bundle.sh -k minimal_system_quick_test.py if
    - PR changes only affect a few log entries
    - PR changes are small, and do not have a large impact on the workflow (use carefully)
  - Otherwise run the full integration test bundle as daqsystemtest_integtest_bundle.sh
- What to do if the integration tests fail?
  - Only concern yourself if failures related to drunc are in the log files
  - If non-drunc failure appears:
    - Validate failure in fresh working area
    - Contact Pawel if unsure
If you have ran the full integration test bundle, leave a comment on the PR stating
- Which host the integration tests have ran on
- [Optional] A copy of the test summary
Drunc integration tests pass (scripts/drunc_integtest_bundle.sh)

Once the above boxes are checked, the PR(s) can be merged following the steps below.

Prior to merging

Choose one of the following an complete all substeps

Changes only affect the Run Control, are in a single repository, and do not affect the end user.
- Changes are documented in docstrings and code comments
- Wiki has been updated if architectural or endpoint changes
Otherwise
- Workflow changes demonstrated in the Change Log (if necessary)
- Wiki has been updated (if necessary)
- #dunedaq-integration Slack channel notified (see below)

Once completed, the reviewer can merge the PR.

Notification message for a Slack channel

Note - this should be to #dunedaq-integration for general workflow that isn't during a release candidate period, and to #daq-release-prep otherwise.

For an single merge that changes the user workflow

The CCM WG has an isolated PR ready to merge that affects user workflows. The PR is:

_URL_

I will leave time for any comments, otherwise will merge these at the end of the work day _Insert your time zone_.

For co-ordinated merge

The CCM WG has a set of co-ordinated merges ready to merge. The PRs are:

_URL_

_URL_


I will leave time for any comments, otherwise will merge these at the end of the day.

emmuhamm

Hello hello @PawelPlesniak this is ready for review now. Ended up being a shorter PR than I thought it would be.

I've left a comment for you to look at and for us to decide on. Can you let me know what you think?

emmuhamm · 2026-06-08T10:44:30Z

    if len(processes.values) > 0:
        click.confirm(
-            f"You already have {len(processes.values)} processes running, are you sure you want to boot a session?",
+            f"You already have {len(processes.values)} processes running for {session_name}, are you sure you want to boot a session?",
            abort=True,
        )


In the process manager shell, I've modified this to basically check if someone wants to boot a session with the same name as another session, theres a click.confirm here.

Do we want to keep this as a confirm or do we want to outright disallow this behaviour?

emmuhamm · 2026-06-17T13:01:02Z

Superseded by #941

emmuhamm self-assigned this Jun 1, 2026

emmuhamm added the Process Manager label Jun 1, 2026

emmuhamm added this to the fddaq-v5.7.0 milestone Jun 1, 2026

emmuhamm linked an issue Jun 1, 2026 that may be closed by this pull request

Minor issues from PMaaS testing (20260515) #926

Open

emmuhamm force-pushed the PawelPlesniakEmuhammad/SplitShellFixing branch from 0114aa4 to a4d174f Compare June 3, 2026 14:33

emmuhamm added 8 commits June 3, 2026 16:35

[FIXUP] hardcode to 29 to test if boot works on different hosts

0e99a6a

minimum fix needed to fix the LCS issue

29f0172

Remove the confirmation?

77280e1

update controller advertising output

446f774

[HACK] remoce shared context to enable controller shell to work

053c4db

add some notes and logging

5ce8e03

new, add minimal working for it to run anywhere?

33e0ae6

finally the multi host stuff works

0107676

emmuhamm force-pushed the emmuhamm/fix-minor-split-shell-issues branch from 12b3c17 to 0107676 Compare June 3, 2026 14:37

emmuhamm added 6 commits June 4, 2026 11:39

pawel push

5994467

clean up comments

df9c2a0

Clean up even more

926814e

block boot on existing sessions

b54a79c

Cleanup

a26a9b2

Fix pytest

e26bc6d

emmuhamm force-pushed the emmuhamm/fix-minor-split-shell-issues branch from cfc2991 to e26bc6d Compare June 8, 2026 13:06

emmuhamm commented Jun 8, 2026

View reviewed changes

emmuhamm marked this pull request as ready for review June 8, 2026 13:08

emmuhamm requested a review from PawelPlesniak June 8, 2026 13:13

emmuhamm closed this Jun 17, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix minor split shell issues#935

Fix minor split shell issues#935
emmuhamm wants to merge 14 commits into
PawelPlesniakEmuhammad/SplitShellFixingfrom
emmuhamm/fix-minor-split-shell-issues

emmuhamm commented Jun 1, 2026 •

edited

Loading

Uh oh!

emmuhamm left a comment

Uh oh!

emmuhamm Jun 8, 2026

Uh oh!

emmuhamm commented Jun 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

emmuhamm commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

List of required branches from other repositories

Suggested manual testing checklist

Developer checklist

Prior to marking this as "Ready for Review"

Final checklist prior to marking this as "Ready for Review"

Reviewer checklist

Prior to merging

Notification message for a Slack channel

For an single merge that changes the user workflow

For co-ordinated merge

Uh oh!

emmuhamm left a comment

Choose a reason for hiding this comment

Uh oh!

emmuhamm Jun 8, 2026

Choose a reason for hiding this comment

Uh oh!

emmuhamm commented Jun 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

emmuhamm commented Jun 1, 2026 •

edited

Loading