Split shell improvements - #912
Merged
PawelPlesniak merged 26 commits intoMay 14, 2026
Merged
Conversation
Member
Author
|
buzz off copilot |
emmuhamm
force-pushed
the
emmuhamm/split-shell-improvements
branch
from
May 6, 2026 16:19
6452a4d to
e03689b
Compare
This was
linked to
issues
May 8, 2026
Member
Author
|
Keep this targetting Pawel's branch so we have a 'split shell / multi user' branch to test on |
Member
Author
|
@PawelPlesniak I think this is ready for your look and testing!! |
emmuhamm
marked this pull request as ready for review
May 14, 2026 11:02
Collaborator
|
This is great, thank you Emir! |
PawelPlesniak
merged commit May 14, 2026
66c8707
into
PawelPlesniakEmuhammad/SplitShellFixing
2 of 4 checks passed
This was referenced May 14, 2026
emmuhamm
added a commit
that referenced
this pull request
Jun 22, 2026
emmuhamm
added a commit
that referenced
this pull request
Jun 23, 2026
emmuhamm
added a commit
that referenced
this pull request
Jun 23, 2026
22 tasks
17 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #709
Fixes #910
Fixes #326
Fixes #904
This PR improves the split shell usage in drunc, see the above issues for details.
Main changes
1. Shell ID Support in Contexts
shell_idacross the key shell context classes (ControllerContext,ProcessManagerContext,UnifiedShellContext). This allows the codebase and logs to always distinguish which shell user interaction is occurring through, improving auditability and clarity in message passing between shells and the process manager.2. Logging and Operator Feedback
New Logging Features & Operator Visibility:
Introduced the new
log_pm_cmdutility: Any explicit shell commands a user runs (plus explicit/overridden arguments) are communicated to the process manager, tagged with user/session/shell type. This allows messages to be sent and saved by the process managers.this logs the command executed, arguments if any, sessions if any, and the shell context.
The logging mechanism is designed for rich, structured feedback: it logs the command executed, its arguments (but only those the user explicitly set), the session (if available), and the shell context.
See example log messages here, which should be self explanatory
emmuhamm sent ps via process_manager_shell; from np04-srv-028.cern.ch:52606emmuhamm sent logs with arguments {'name': "('root-controller',)", 'session': "'emir-test-2'"} via process_manager_shell; from np04-srv-028.cern.ch:52606emmuhamm sent logs with arguments {'name': "('mlt',)"} for session emir-test-1 via unified_shell; from np04-srv-028.cern.ch:41896Both the process manager shell and unified shell now send explicit connect/disconnect messages to the process manager via the new gRPC
send_msgendpoint.emmuhamm connected from unified shell; from np04-srv-028.cern.ch:41896Added
resolve_context_peerutility. This function parses gRPC peer strings, extracts and tries to converts raw IP addresses into hostnames. This means a networked command or interaction will now show something likenp04-srv-028.cern.ch:41750instead of a less informativeipv4:10.73.136.70:41750.How it Works—gRPC Enhancements
send_msgendpoint, fully implemented for both SSH- and Kubernetes-backed managers. All connect, disconnect, and command logs are routed through this, ensuring comprehensive auditability. Failures in this endpoint do not block user actions but are logged for troubleshooting.3. Improved Session and Database Path Logic
More Reliable File Resolution:
oks_parser.py, when multiple possible configuration file matches are found in theDUNEDAQ_DB_PATH, the code now:Connection/Config Defaults:
conf_file), the code now assumes and prependsoksconflibs:, ensuring that new users and scripts do not fail silently due to missing protocol specifiers.4. Query Logic Corrections
_match_processes_against_query, the logic for filtering processes against user-specified queries in the process manager has been corrected:5. CLI/UX Improvements and Refactors
kill,flush,logs, andrestarthave been unified to ensure consistent parameter usage and reduce duplicated boilerplate.waitcommand for the process manager shell.session_injectorautomatically ensures the session context is injected into any relevant query, making script writing and interactive usage much simpler and less error-prone.6. Backend and Test/Mock Improvements
pm_typeattribute and fully implement thesend_msginterface for gRPC messaging.send_msglogic, improving test reliability.Type of change
List of required branches from other repositories
DUNE-DAQ/druncschema#85
Suggested manual testing checklist
Requires three shells. Currently tested on
NFD_DEV_260504_A9onnp04-srv-028.logging related testing
Open a new shell with dunedaq env and run the process manager terminal
drunc-process-manager-shell grpc://localhost:50000Open a new shell with dunedaq env and run the process manager shell terminal
drunc-process-manager-shell grpc://localhost:50000Open a new shell with dunedaq env and run the unified shell with session name of your choice (this command uses its own example)
drunc-unified-shell grpc://np04-srv-028:50000 config/daqsystemtest/example-configs.data.xml local-1x1-config emir-test-1np04-srv-028withlocalhost. see it worksoksconflibs:is not required when passing the config in the unified shell, showing it works. Also test by adding inoksconflibs:in the above config, egoksconflibs:config/daqsystemtest/example-configs.data.xmlBoot sessions:
bootboot oksconflibs:config/daqsystemtest/example-configs.data.xml local-1x1-config emir-test-3Try the different commands that exist in both shells
pmshould work as expected in both. Note that it will only show the relevant session names in the unified shelllogs --name root-controllerin both shells. it should only work in the unified shell since there are multiple root-controllers in the PM shellTESTING NOTES
Note so far has only been tested on ssh-local. Have not tried on k8s or similar. I dont think I've broken the main way we do it which is via unified shell but lets see..
Also pytests will pass, but there is one that fails. This is clearly documented in the PR changes though, and will have to pass before it can go in.
Developer checklist
Prior to marking this as "Ready for Review"
Tests ran on: WHAT HOSTNAME from release RELEASE_NAME
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_bundlerequires 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 .pytest --marker) passeddaqsystemtest_integtest_bundle.sh -k minimal_system_quick_test.pydaqsystemtest_integtest_bundle.sh./scripts/drunc_integtest_bundle.sh)Final checklist prior to marking this as "Ready for Review"
Reviewer checklist
druncare in the log filesdruncfailure appears:Once the features are validated and both the unit and integration tests pass, the PRs is ready to be merged.
Choose one of the following an complete all substepsPrior to merging
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
For co-ordinated merge