[code sync] Merge code from sonic-net/sonic-utilities:202605 to 202608 - #444
Merged
Conversation
mssonicbld
commented
Aug 27, 2026
Collaborator
Signed-off-by: Nazarii Hnydyn <nazariig@nvidia.com> <!-- Please make sure you've read and understood our contributing guidelines: https://github.com/Azure/SONiC/blob/gh-pages/CONTRIBUTING.md CODE_OF_CONDUCT.md LICENSE README.md SECURITY.md SUPPORT.md azure-pipelines failure_prs.log scripts skip_prs.log Make sure all your commits include a signature generated with `git commit -s` ** If this is a bug fix, make sure your description includes "closes #xxxx", "fixes #xxxx" or "resolves #xxxx" so that GitHub automatically closes the related issue when the PR is merged. If you are adding/modifying/removing any command or utility script, please also make sure to add/modify/remove any unit tests from the tests directory as appropriate. If you are modifying or removing an existing 'show', 'config' or 'sonic-clear' subcommand, or you are adding a new subcommand, please make sure you also update the Command Line Reference Guide (doc/Command-Reference.md) to reflect your changes. Please provide the following information: --> ## The bug The `config` command uses a custom Click group, `AbbreviationGroup`, that supports abbreviated subcommand names via prefix matching: `sonic-buildimage/src/sonic-utilities/utilities_common/cli.py` ```python def get_command(self, ctx, cmd_name): # Try to get builtin commands as normal rv = click.Group.get_command(self, ctx, cmd_name) if rv is not None: return rv ... matches = [] shortest = None for x in self.list_commands(ctx): if x.lower().startswith(cmd_name.lower()): matches.append(x) ... if not matches: return None elif len(matches) == 1: return click.Group.get_command(self, ctx, matches[0]) else: for x in matches: if not x.startswith(shortest): break else: return click.Group.get_command(self, ctx, shortest) ctx.fail('Too many matches: %s' % ', '.join(sorted(matches))) ``` `switch` is an ambiguous prefix — it matches `switch-fast-linkup`, `switch-hash`, `switch-trimming`, and `switchport`. The shortest match is `switchport`, but not all matches start with `switchport`, so the `for/else` falls through to `ctx.fail(...)`, which raises `click.exceptions.UsageError`. ## Why the trailing space matters The two Tab presses take completely different code paths inside Click: - **`config switch` + Tab** (cursor right after the word): Click treats `switch` as the **incomplete** token. It just filters the list of subcommand names by that prefix and prints the candidates. `get_command("switch")` is never called, so nothing fails — that's why we see the nice list of 4 completions. - **`config switch ` + Tab** (extra space): now `switch` is a **complete** argument and the incomplete token is `""`. To figure out which subcommand's arguments to complete next, Click's `_resolve_context` walks the arg list and calls `command.resolve_command(ctx, ["switch"])` → `get_command(ctx, "switch")`. That triggers the ambiguous-prefix path and hits `ctx.fail()`. The real defect is that `ctx.fail()` raises an exception **during shell completion**. Click's completion resolver doesn't catch exceptions from `get_command`, so instead of silently returning "no completions," the whole traceback gets dumped into the terminal. ## Why it only appears now During completion, Click sets `ctx.resilient_parsing = True` precisely so that resolution logic degrades gracefully instead of erroring. `AbbreviationGroup.get_command` ignores that flag. (We are also on a newer Click / Python 3.13, where completion resolution is stricter — older versions were more forgiving, which is likely why we hadn't seen this before.) ## The fix Guard the failure so it returns `None` during completion instead of raising: ```python else: for x in matches: if not x.startswith(shortest): break else: return click.Group.get_command(self, ctx, shortest) if ctx.resilient_parsing: return None ctx.fail('Too many matches: %s' % ', '.join(sorted(matches))) ``` With that, tab-completion on an ambiguous prefix just yields no completion (or the caller can list children) instead of crashing, while actually *running* `config switch` still gives the proper "Too many matches" usage error. #### What I did * Fixed traceback dumping into the terminal on command autocompletion #### How I did it * Added a guard for `AbbreviationGroup` autocompletion #### How to verify it 1. Run UTs #### Previous command output (if the output of a command-line utility has changed) ``` root@sonic:/home/admin# config switch Traceback (most recent call last): File "/usr/local/bin/config", line 8, in <module> sys.exit(config()) ~~~~~~^^ File "/usr/lib/python3/dist-packages/click/core.py", line 1161, in __call__ return self.main(*args, **kwargs) ~~~~~~~~~^^^^^^^^^^^^^^^^^ File "/usr/lib/python3/dist-packages/click/core.py", line 1077, in main self._main_shell_completion(extra, prog_name, complete_var) ~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib/python3/dist-packages/click/core.py", line 1156, in _main_shell_completion rv = shell_complete(self, ctx_args, prog_name, complete_var, instruction) File "/usr/lib/python3/dist-packages/click/shell_completion.py", line 49, in shell_complete echo(comp.complete()) ~~~~~~~~~~~~~^^ File "/usr/lib/python3/dist-packages/click/shell_completion.py", line 293, in complete completions = self.get_completions(args, incomplete) File "/usr/lib/python3/dist-packages/click/shell_completion.py", line 273, in get_completions ctx = _resolve_context(self.cli, self.ctx_args, self.prog_name, args) File "/usr/lib/python3/dist-packages/click/shell_completion.py", line 525, in _resolve_context name, cmd, args = command.resolve_command(ctx, args) ~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^ File "/usr/lib/python3/dist-packages/click/core.py", line 1738, in resolve_command cmd = self.get_command(ctx, cmd_name) File "/usr/local/lib/python3.13/dist-packages/utilities_common/cli.py", line 59, in get_command ctx.fail('Too many matches: %s' % ', '.join(sorted(matches))) ~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib/python3/dist-packages/click/core.py", line 691, in fail raise UsageError(message, self) click.exceptions.UsageError: Too many matches: switch-fast-linkup, switch-hash, switch-trimming, switchport Usage: config [OPTIONS] COMMAND [ARGS]... Try 'config -h' for help. Error: Too many matches: switch-fast-linkup, switch-hash, switch-trimming, switchport ``` #### New command output (if the output of a command-line utility has changed) ``` root@sonic:/home/admin# config switch switch-fast-linkup switch-hash switch-trimming switchport ... ``` #### A picture of a cute animal (not mandatory but encouraged) ``` .---. .----------- / \ __ / ------ / / \( )/ ----- ////// ' \/ ` --- //// / // : : --- // / / /` '-- // //..\\ ====UU====UU==== '//||\\` ''`` ``` Signed-off-by: Sonic Build Admin <sonicbld@microsoft.com>
…nks (#4790)
#### What I did
Fixed `sonic-installer install` failing in `migrate_sonic_packages()` with `Temporary failure in name resolution` when `sonic-package-manager migrate` has to fetch a package manifest from a container registry inside the new image's chroot.
#4365 made the DNS injection symlink-aware, but its path math only handles absolute symlink targets: `lstrip("/")` is a no-op on a relative target such as `../run/resolvconf/resolv.conf` (the form the Debian resolvconf package ships in the image), so `os.path.join()` produced a path whose `..` escaped the chroot mount. The host's DNS configuration was written to the host filesystem (`/tmp/run/...`) instead of into the chroot overlay, leaving the chroot with whatever stale `resolv.conf` the image's squashfs happened to carry from its build server — so package migration worked or failed depending on which build machine produced the image.
#### How I did it
- Resolve a relative symlink target against the symlink's own directory (`etc/`), per POSIX symlink semantics.
- `os.path.normpath()` on the anchored absolute path collapses `..` and clamps it at the chroot root, mirroring how the kernel resolves the link inside the chroot — the computed path can never escape the image mount.
- `cp -L --remove-destination`: if the computed target already exists as a symlink, replace it instead of writing through it.
- Absolute targets pass through unchanged (`normpath` is the identity on them), so the case #4365 fixed behaves exactly as before; the regular-file branch is untouched.
#### How to verify it
- `tests/test_sonic_installer.py`: `test_install` is parametrized over the new image's `/etc/resolv.conf` layouts — absolute symlink, relative symlink, a target with excess `..`, and a regular file. The relative-symlink case fails against the previous code; the expected command list is asserted with strict order/argv equality.
- Verified end to end on a Mellanox MSN4700: installing an image whose squashfs ships the relative symlink and stale, unreachable build-time nameservers previously failed at the `sonic-package-manager migrate` step with the DNS error above (and left a stray copy of the host's DNS configuration under `/tmp/run/` on the host). With this fix the same install completes: the host DNS lands inside the chroot overlay (`rw/run/resolvconf/resolv.conf`) and no stray host file is created.
Signed-off-by: Sonic Build Admin <sonicbld@microsoft.com>
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
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.