Skip to content

Support collection fixes and additions - #1325

Merged
troglobit merged 3 commits into
mainfrom
support-extra
Dec 18, 2025
Merged

troglobit merged 3 commits into
mainfrom
support-extra

Conversation

@troglobit

@troglobit troglobit commented Dec 18, 2025 •

Copy link
Copy Markdown
Contributor

Description

  • Extend support script to handle case where /var/lib/support exists and is owned by root
  • Extract all use of sudo inside script in favor of letting the user decide
  • Fix misplaced iitod.conf override for Styx DCP-SC-28p

Checklist

Tick relevant boxes, this PR is-a or has-a:

  • Bugfix
    • Regression tests
    • ChangeLog updates (for next release)
  • Feature
    • YANG model change => revision updated?
    • Regression tests added?
    • ChangeLog updates (for next release)
    • Documentation added?
  • Test changes
    • Checked in changed Readme.adoc (make test-spec)
    • Added new test to group Readme.adoc and yaml file
  • Code style update (formatting, renaming)
  • Refactoring (please detail in commit messages)
  • Build related changes
  • Documentation content changes
    • ChangeLog updated (for major changes)
  • Other (please describe): support script plugin

@troglobit
troglobit requested a review from wkz December 18, 2025 07:47

@wkz wkz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What do you think about having cmd_collect() do one initial check for sudo availability - if it is available, we re-exec the support script with sudo?

Then we don't have to think about it in any of the following commands. I.e., everything that follows can assume that the privileges it has is the maximum available.

@troglobit

Copy link
Copy Markdown
Contributor Author

What do you think about having cmd_collect() do one initial check for sudo availability - if it is available, we re-exec the support script with sudo?

Then we don't have to think about it in any of the following commands. I.e., everything that follows can assume that the privileges it has is the maximum available.

It's sure easier for future extensions and additions, but I'd rather like to see it possible to run the script as a less privileged user without sudo access. (Also, giving a script full root access instead of per-command feels a bit unsafe to me.)

So I propose this instead:

  SUDO=""                                                                                                                                                 
  if [ "$(id -u)" -ne 0 ]; then                                                                                                                           
      if command -v sudo >/dev/null 2>&1 && sudo -n true 2>/dev/null; then                                                                                
          SUDO="sudo"                                                                                                                                     
      fi                                                                                                                                                  
  fi                                                                                                                                                      
  export SUDO 

That way our plugins can always rely on $SUDO being available to them, and users could get a subset of the information available to them. This also works for future users that may have to input sudo password.

@troglobit

This comment was marked as outdated.

Actually disable iitod on Styx DCP-SC-28p to work around #670, this
prohibits software control of LEDs, leaving the default HW control,
which has proven more stable on this platform.

Signed-off-by: Joachim Wiberg <[email protected]>
Attempt to fix permissions with sudo before falling back to $HOME,
and verify directory is actually writable before proceeding.

Signed-off-by: Joachim Wiberg <[email protected]>
@troglobit

Copy link
Copy Markdown
Contributor Author

AFK we agreed that extracting sudo everywhere from within the script and instead letting the user decide the level of logs retrieved, calling with/without sudo.

@wkz wkz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Excellent! ✨

@troglobit
troglobit merged commit 230574b into main Dec 18, 2025
@troglobit
troglobit deleted the support-extra branch December 18, 2025 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants