Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Showtech sonic mgmt framework: Add Management Framework functionality for "show tech-support" #7816

Merged

Conversation

kerry-meyer
Copy link
Contributor

@kerry-meyer kerry-meyer commented Jun 8, 2021

Provide the changes required for supporting the "show-techsupport" command via the SONiC Management Framework front end mechanisms (CLI, REST, and gNOI). The Management Framework functionality implemented by this PR improves on the the capabilities currently provided by the SONiC Click CLI interface via the "show techsupport" command by providing the following additional features:

  • User-friendly "help" information describing command syntax details for CLI invocation.
  • Ability to invoke the command via REST and gNOI mechanisms.

Unit test results are attached to this PR.

unit_test_log_0607.txt

Why I did it

The proposed changes improve on the the capabilities currently provided by the SONiC Click CLI interface via the "show techsupport" command by providing the following additional features:

  • User-friendly "help" information describing command syntax details for CLI invocation.
  • Ability to invoke the command via REST and gNOI mechanisms.

How I did it

  • Provide a Python script as a host D-Bus servelet to trigger invocation of the "generate_dump" script and report the results. The servelet is executed as a result of a corresponding D-Bus request sent by the SONiC Management Framework. (src/sonic-host-services/host_modules/showtech.py)
  • Enable default startup of the sonic-hostservice facility during initialization. (rules/config b/rules/config)

Corequisite PRs

sonic-net/sonic-mgmt-framework#86
sonic-net/sonic-mgmt-common#49

How to verify it

Execute the Unit Test procedure described in the HLD for this feature:

  • Display relevant 'help' information
  • Basic Execution (no optional parameters)
  • Execution with a valid value for the "since" option
  • Execution with an invalid value for the "since" option
  • Execution with no value for the "since" option.

unit_test_log_0607.txt

Which release branch to backport (provide reason below if selected)

N/A

  • 201811
  • 201911
  • 202006
  • 202012

Description for the changelog

Add Management Framework functionality for "show tech-support".

A picture of a cute animal (not mandatory but encouraged)

kerry-meyer and others added 3 commits May 14, 2021 23:45
…_framework

Merging updates from the Azure sonic-buildimage repo on 05/26/2021.
Provide changes needed for the Dell Enterprise version of this feature
to make it compatible with the SONiC Community code base.
rules/config Outdated Show resolved Hide resolved
Provide changes to address code review comments:

  - Invoke the generate_dump script using subprocess.run instead of
    subprocess.check_output.

  - Use output encoding in the subprocess.run call to eliminate
    post-execution decoding of text.

  - Delete unnecessary pattern matching of output for the "success"
    case.
@kerry-meyer
Copy link
Contributor Author

I have made changes to the "showtech.py" file in response to code review comments and have re-tested the resulting image build to verify correct behavior for exception and successful completion cases.

  - Address a remaining PR code review comment by directly using
    the "returncode" value provided by "subprocess" execution.
  - Add filtering of output to extract only the output filename.
    (This eliminates extraneous output that could otherwise be
    seen in some non-error cases when invoking "show-techsupport"
    via the REST API or gNOI.)
@kerry-meyer
Copy link
Contributor Author

I have pushed additional changes to address reviewer comments recommending direct usage of the "returncode" value provided by "subprocess.run" execution and to fix a problem that could otherwise occur for "show tech-support" invocation via REST API and gNOI invocation.

A corresponding change has also been made for the sonic-management-framework PR (sonic-net/sonic-mgmt-common#49) to provide better output information for REST server invocation errors and to fix an lgtm alert.

Merge "latest" Azure master branch changes to the showtech branch.
The following submodules were inadvertently added to this PR and need to be
removed from the PR:

- src/sonic-mgmt-common and src/sonic-mgmt-framework: These submodules
were added prematurely. A separate commit (or new PR) will need to
be done for them after the pending PRs for both submodules have been
merged to the Azure "master" branch.

- src/sonic-frr/frr: Added accidentally; no relevance for this PR.
@joyas-joseph
Copy link
Contributor

/azpw run

1 similar comment
@kerry-meyer
Copy link
Contributor Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

jeff-yin
jeff-yin previously approved these changes Sep 21, 2021
Copy link
Collaborator

@jeff-yin jeff-yin left a comment

Choose a reason for hiding this comment

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

Reminder that this still needs approval/merge

@leeprecy
Copy link
Contributor

reviewed

leeprecy
leeprecy previously approved these changes Oct 21, 2021
Restore the setting of the "ENABLE_HOST_SERVICE_ON_START" flag
to "n" to disable startup of the sonic-hostservices service
during initialization.

This change is being made in order to resolve the only remaining
issue for the associated set of PRs.
@kerry-meyer kerry-meyer dismissed stale reviews from leeprecy and jeff-yin via f9bf1bf November 18, 2021 21:17
@kerry-meyer
Copy link
Contributor Author

I have pushed a change to resolve the only remaining issue for this PR. The change reverts the setting of the "ENABLE_HOST_SERVICE_ON_START" flag
to "n" to disable startup of the sonic-hostservices service
during initialization.

@kerry-meyer
Copy link
Contributor Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

You have several pipelines (over 10) configured to build pull requests in this repository. Specify which pipelines you would like to run by using /azp run [pipelines] command. You can specify multiple pipelines using a comma separated list.

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 7816 in repo Azure/sonic-buildimage

@joyas-joseph
Copy link
Contributor

/azpw run Azure.sonic-buildimage

@kerry-meyer
Copy link
Contributor Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

You have several pipelines (over 10) configured to build pull requests in this repository. Specify which pipelines you would like to run by using /azp run [pipelines] command. You can specify multiple pipelines using a comma separated list.

@kerry-meyer
Copy link
Contributor Author

/azpw run Azure.sonic-buildimage

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@kerry-meyer
Copy link
Contributor Author

/azpw run Azure.sonic-buildimage

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@qiluo-msft qiluo-msft merged commit 9b795db into sonic-net:master Jan 10, 2022
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.

7 participants