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

Support share image for gnmi and telemetry #10348

Merged
merged 13 commits into from
Nov 1, 2023

Conversation

ganglyu
Copy link
Contributor

@ganglyu ganglyu commented Oct 16, 2023

Description of PR

Summary:
Support container update for telemetry and gnmi.
Fixes # (issue)

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • Test case(new/improvement)

Back port request

  • 201911
  • 202012
  • 202205
  • 202305

Approach

What is the motivation for this PR?

We will share docker image to support gnmi and telemetry.
So we need to update minigraph, gnmi and telemetry test to support this change.

How did you do it?

For telemetry test, check telemetry container at first, if there's no telemetry container, use gnmi container.
For gnmi test, check gnmi container at first, if there's no gnmi container, use telemetry container.
For minigraph test, check feature status to choose service.

How did you verify/test it?

Run minigraph, gnmi and telemetry end to end test.

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

@ganglyu ganglyu changed the title Support single gnmi image Support share image for gnmi and telemetry Oct 20, 2023
test_service = "telemetry"
logging.info("Bringing down telemetry service")
service_status = duthost.critical_process_status("telemetry")
containers_states, succeeded = duthost.get_feature_status()
Copy link
Contributor

Choose a reason for hiding this comment

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

get_feature_status

Instead of using running status of feature, is it better to use config of feature?

For example, if telemetry feature is configured as enabled, but crashed, fail this testcase instead of fallback to gnmi feature.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Feature config is not correct, for example, telemetry is enabled, but there's no telemetry service.
I will use docker image to check feature status, if there's telemetry docker image, we should telemetry service.

class GNMIEnvironment(object):
def __init__(self, duthost):
self.duthost = duthost
self.use_gnmi_container = duthost.shell("docker ps | grep -w gnmi", module_ignore_errors=True)['rc'] == 0
Copy link
Contributor

Choose a reason for hiding this comment

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

use_gnmi_container

Similar concern, do you intent to check running status of config?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will user docker image instead.

@sonic-net sonic-net deleted a comment from mssonicbld Oct 28, 2023
tests/telemetry/conftest.py Show resolved Hide resolved
tests/gnmi/helper.py Outdated Show resolved Hide resolved
tests/telemetry/conftest.py Show resolved Hide resolved
tests/gnmi/helper.py Outdated Show resolved Hide resolved
@zbud-msft zbud-msft self-requested a review October 31, 2023 00:18
Copy link
Contributor

@zbud-msft zbud-msft left a comment

Choose a reason for hiding this comment

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

Added one comment

@sonic-net sonic-net deleted a comment from mssonicbld Oct 31, 2023
@sonic-net sonic-net deleted a comment from mssonicbld Oct 31, 2023
@sonic-net sonic-net deleted a comment from mssonicbld Oct 31, 2023
@ganglyu ganglyu merged commit 6f9acd3 into sonic-net:master Nov 1, 2023
13 checks passed
ganglyu added a commit to ganglyu/sonic-mgmt that referenced this pull request Nov 9, 2023
What is the motivation for this PR?
We will share docker image to support gnmi and telemetry.
So we need to update minigraph, gnmi and telemetry test to support this change.

How did you do it?
For telemetry test, check telemetry container at first, if there's no telemetry container, use gnmi container.
For gnmi test, check gnmi container at first, if there's no gnmi container, use telemetry container.
For minigraph test, check feature status to choose service.

How did you verify/test it?
Run end to end test.
StormLiangMS pushed a commit that referenced this pull request Nov 10, 2023
* Support share image for gnmi and telemetry (#10348)

What is the motivation for this PR?
We will share docker image to support gnmi and telemetry.
So we need to update minigraph, gnmi and telemetry test to support this change.

How did you do it?
For telemetry test, check telemetry container at first, if there's no telemetry container, use gnmi container.
For gnmi test, check gnmi container at first, if there's no gnmi container, use telemetry container.
For minigraph test, check feature status to choose service.

How did you verify/test it?
Run end to end test.

* Ignore telemetry container in loganalyzer check (#10637)

What is the motivation for this PR?
We will remove telemetry container in public repo, and FEATURE config for telemetry is still enabled.
Therefore, we need ignore syslog error.

How did you do it?
Update loganalyzer_common_ignore.txt.

How did you verify/test it?
Run end to end test.
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.

3 participants