[voq] refine eos fanout function to check fanout link oper status #9128

wenyiz2021 · 2023-07-26T19:10:21Z

Description of PR

This is to fix hidden issue in #8468
It could cause failure for test_voq_nbr.py and test_lag_2.py.

eos.py: current function check_intf_link_state is trying to find 'Up' from show int <interface> output, but it always returns false because output looks like Ethernet2/1 is up, line protocol is up (connected), probably differs between eos image versions. A more reliable way is to check show int <interface> | json and find connected in output. Detail is described in code comment.

Summary:
Fixes # (issue)

Type of change

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

Back port request

201911
202012
202205

Approach

What is the motivation for this PR?

How did you do it?

How did you verify/test it?

Any platform specific information?

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

Documentation

mssonicbld · 2023-07-26T19:11:24Z

The pre-commit check detected issues in the files touched by this pull request.
The pre-commit check is a mandatory check, please fix detected issues.

Detailed pre-commit check results:
trim trailing whitespace.................................................Failed - hook id: trailing-whitespace - exit code: 1 - files were modified by this hook Fixing tests/common/devices/eos.py fix end of files.........................................................Passed check yaml...........................................(no files to check)Skipped check for added large files..............................................Passed check python ast.........................................................Passed flake8...................................................................Passed check conditional mark sort..........................(no files to check)Skipped

To run the pre-commit checks locally, you can follow below steps:

Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
docker container.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

Go to repository root folder
Install the pre-commit hooks:

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

pre-commit run --from-ref <commit_id> --to-ref <commit_id>

mssonicbld · 2023-07-26T20:24:57Z

The pre-commit check detected issues in the files touched by this pull request.
The pre-commit check is a mandatory check, please fix detected issues.

Detailed pre-commit check results:
trim trailing whitespace.................................................Failed - hook id: trailing-whitespace - exit code: 1 - files were modified by this hook Fixing tests/common/devices/eos.py fix end of files.........................................................Passed check yaml...........................................(no files to check)Skipped check for added large files..............................................Passed check python ast.........................................................Passed flake8...................................................................Passed check conditional mark sort..........................(no files to check)Skipped

To run the pre-commit checks locally, you can follow below steps:

Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
docker container.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

Go to repository root folder
Install the pre-commit hooks:

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

pre-commit run --from-ref <commit_id> --to-ref <commit_id>

mssonicbld · 2023-07-26T20:55:28Z

The pre-commit check detected issues in the files touched by this pull request.
The pre-commit check is a mandatory check, please fix detected issues.

Detailed pre-commit check results:
trim trailing whitespace.................................................Failed - hook id: trailing-whitespace - exit code: 1 - files were modified by this hook Fixing tests/common/devices/eos.py fix end of files.........................................................Passed check yaml...........................................(no files to check)Skipped check for added large files..............................................Passed check python ast.........................................................Passed flake8...................................................................Passed check conditional mark sort..........................(no files to check)Skipped

To run the pre-commit checks locally, you can follow below steps:

Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
docker container.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

Go to repository root folder
Install the pre-commit hooks:

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

pre-commit run --from-ref <commit_id> --to-ref <commit_id>

slianganet · 2023-07-26T21:41:26Z

tests/common/devices/eos.py

+        """
+        This function returns link oper as well as admin status
+            e.g. cable not connected:
+                     Ethernet1/1 is down, line protocol is notpresent (notconnect)
+                 link is admin down(cable not present):
+                     Ethernet1/1 is administratively down, line protocol is notpresent (disabled)
+                 link is admin down(cable present):
+                     Ethernet2/1 is administratively down, line protocol is down (disabled)
+                 link is admin up&oper up:
+                     Ethernet2/1 is up, line protocol is up (connected)
+                 link is admin up&oper down:
+                     Ethernet2/1 is down, line protocol is down (notconnect)
+        In conclusion, if 'up' found in output line, link is oper up&admin up.
+        Link could not be admin down&oper up.
+        This function could not tell if it's admin up&oper down
+        """
        show_int_result = self.eos_command(
            commands=['show interface %s' % interface_name])
-        return 'Up' in show_int_result['stdout_lines'][0]
+        return 'up' in show_int_result['stdout_lines'][0].lower()
+
+    def check_intf_link_oper_state(self, interface_name):
+        """
+        This function returns oper state for eos fanout
+        e.g. Et1/1 is admin up, oper down
+             Et2/1 is admin down
+             Et3/1 is oper up
+        Et1/1      str2-7804-lc7-1-Ethernet0            notconnect   1102     full   100G   100GBASE-CR4
+        Et2/1      str2-7804-lc7-1-Ethernet4            disabled     1103     full   100G   100GBASE-CR4
+        Et3/1      str2-7804-lc7-1-Ethernet8            connected    1104     full   100G   100GBASE-CR4
+        """
+        show_int_result = self.eos_command(commands=['show interface status'])
+        for output_line in show_int_result['stdout_lines'][0]:
+            fields = output_line.split(' ')
+            output_port = fields[0].replace('Et', 'Ethernet')
+            if interface_name == output_port:
+                # if link is not oper up, we consider it as oper down, it could be admin down as well
+                if 'connected' not in fields:
+                    logging.info("Interface {} is oper down on {}".format(output_port, self.hostname))
+                    return False
+        return True
+
+    def check_intf_link_admin_state(self, interface_name):
+        """
+        This function returns admin state for eos fanout
+        """
+        show_int_result = self.eos_command(commands=['show interface status'])
+        for output_line in show_int_result['stdout_lines'][0]:
+            fields = output_line.split(' ')
+            output_port = fields[0].replace('Et', 'Ethernet')
+            if interface_name == output_port:
+                # if link is not admin down, we consider it as admin up, however, it could be oper down(notconnect)
+                if 'disabled' in fields:
+                    logging.info("Interface {} is admin down on {}".format(output_port, self.hostname))
+                    return False
+        return True


I would caution against adding new code that tries to interpret or directly parse text output from eos commands. The approach in #9127 would be much preferred as the JSON output gives a well-defined model.

I agree with you in the json way.
but I think it's necessary to avoid the confusion in the function name as well. if we check for 'connected' then it's checking link oper status only, this is another thing I would want to clarify in this PR

Sounds good, I'll let others comment on the preferred naming here. Just wanted to give my 2c on using JSON output.

mssonicbld · 2023-07-27T17:11:26Z

The pre-commit check detected issues in the files touched by this pull request.
The pre-commit check is a mandatory check, please fix detected issues.

Detailed pre-commit check results:
trim trailing whitespace.................................................Passed fix end of files.........................................................Passed check yaml...........................................(no files to check)Skipped check for added large files..............................................Passed check python ast.........................................................Failed - hook id: check-ast - exit code: 1 tests/common/devices/eos.py: failed parsing with CPython 3.8.10: Traceback (most recent call last): File "/home/AzDevOps/.cache/pre-commit/repoqxfenael/py_env-python3/lib/python3.8/site-packages/pre_commit_hooks/check_ast.py", line 21, in main ast.parse(f.read(), filename=filename) File "/usr/lib/python3.8/ast.py", line 47, in parse return compile(source, filename, mode, flags, File "tests/common/devices/eos.py", line 136 commands=['show interface | json' 0nterface_name]) ^ SyntaxError: unmatched ')' ... [truncated extra lines, please run pre-commit locally to view full check results]

To run the pre-commit checks locally, you can follow below steps:

Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
docker container.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

Go to repository root folder
Install the pre-commit hooks:

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

pre-commit run --from-ref <commit_id> --to-ref <commit_id>

mssonicbld · 2023-07-27T17:12:26Z

The pre-commit check detected issues in the files touched by this pull request.
The pre-commit check is a mandatory check, please fix detected issues.

Detailed pre-commit check results:
trim trailing whitespace.................................................Passed fix end of files.........................................................Passed check yaml...........................................(no files to check)Skipped check for added large files..............................................Passed check python ast.........................................................Failed - hook id: check-ast - exit code: 1 tests/common/devices/eos.py: failed parsing with CPython 3.8.10: Traceback (most recent call last): File "/home/AzDevOps/.cache/pre-commit/repo6_ymyom4/py_env-python3/lib/python3.8/site-packages/pre_commit_hooks/check_ast.py", line 21, in main ast.parse(f.read(), filename=filename) File "/usr/lib/python3.8/ast.py", line 47, in parse return compile(source, filename, mode, flags, File "tests/common/devices/eos.py", line 136 commands=['show interface | json' 0nterface_name]) ^ SyntaxError: unmatched ')' ... [truncated extra lines, please run pre-commit locally to view full check results]

To run the pre-commit checks locally, you can follow below steps:

Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
docker container.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

Go to repository root folder
Install the pre-commit hooks:

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

pre-commit run --from-ref <commit_id> --to-ref <commit_id>

slianganet · 2023-07-27T20:08:29Z

Thanks for incorporating the feedback and adding comments! I'll let someone with write access give you approval, but otherwise LGTM.

wenyiz2021 · 2023-07-27T21:53:10Z

thanks @slianganet

Thanks for incorporating the feedback and adding comments! I'll let someone with write access give you approval, but otherwise LGTM.

thanks @slianganet and @gechiang for the corporation!

…nic-net#9128)

mssonicbld · 2023-07-28T00:18:43Z

Cherry-pick PR to 202205: #9166

)

…nic-net#9128)

wenyiz2021 added 6 commits July 26, 2023 11:54

Update eos.py

686c994

Update fanout.py

68a377e

Update sonic.py

4de36ae

Update test_voq_nbr.py

f09ecc0

Update test_lag_2.py

b30d476

Merge branch 'sonic-net:master' into fanout_oper_status

f9fc441

Update eos.py

9b219df

wenyiz2021 added 2 commits July 26, 2023 13:26

fix pre-commit

a8d7657

Update eos.py

4f8e94a

wenyiz2021 added the Request for 202205 branch label Jul 26, 2023

fix pre-commit

c2d6b11

wenyiz2021 marked this pull request as ready for review July 26, 2023 21:01

wenyiz2021 requested review from gechiang, wangxin and arlakshm July 26, 2023 21:01

wenyiz2021 mentioned this pull request Jul 26, 2023

Fix eos implementation of check_intf_link_state() #9127

Closed

6 tasks

wenyiz2021 changed the title ~~refine fanout functions to check fanout link oper status~~ [voq] [pc] refine fanout functions to check fanout link oper status Jul 26, 2023

wenyiz2021 self-assigned this Jul 26, 2023

slianganet reviewed Jul 26, 2023

View reviewed changes

wenyiz2021 added 4 commits July 26, 2023 14:58

Update eos.py

9ad474f

Update test_lag_2.py

6c5fbdf

Update eos.py

bf286b0

Update fanout.py

c23f4cc

wenyiz2021 added 2 commits July 27, 2023 10:14

Update eos.py

322e947

Update eos.py

64ca937

wenyiz2021 added 3 commits July 27, 2023 12:51

Update fanout.py

74bf671

Update sonic.py

837d1ab

Update test_voq_nbr.py

db3ab2d

arlakshm approved these changes Jul 27, 2023

View reviewed changes

wenyiz2021 changed the title ~~[voq] [pc] refine fanout functions to check fanout link oper status~~ [voq] refine fanout functions to check fanout link oper status Jul 27, 2023

wenyiz2021 changed the title ~~[voq] refine fanout functions to check fanout link oper status~~ [voq] refine eos fanout function to check fanout link oper status Jul 27, 2023

wenyiz2021 merged commit 9c1e953 into sonic-net:master Jul 27, 2023
11 checks passed

wenyiz2021 deleted the fanout_oper_status branch July 27, 2023 21:52

wangxin added the Approved for 202205 branch label Jul 28, 2023

mssonicbld pushed a commit to mssonicbld/sonic-mgmt that referenced this pull request Jul 28, 2023

[voq] refine eos fanout function to check fanout link oper status (so…

3e5117b

…nic-net#9128)

mssonicbld added the Created PR to 202205 branch label Jul 28, 2023

mssonicbld mentioned this pull request Jul 28, 2023

[action] [PR:9128] [voq] refine eos fanout function to check fanout link oper status #9166

Merged

6 tasks

mssonicbld pushed a commit that referenced this pull request Jul 28, 2023

[voq] refine eos fanout function to check fanout link oper status (#9128

4b94791

)

mssonicbld added Included in 202205 branch and removed Created PR to 202205 branch labels Jul 28, 2023

ysmanman mentioned this pull request Jul 28, 2023

[voq] link flap test failed because 'fanout port didn't go up as expected' #9169

Open

AharonMalkin pushed a commit to AharonMalkin/sonic-mgmt that referenced this pull request Jan 25, 2024

[voq] refine eos fanout function to check fanout link oper status (so…

7081a4f

…nic-net#9128)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[voq] refine eos fanout function to check fanout link oper status #9128

[voq] refine eos fanout function to check fanout link oper status #9128

wenyiz2021 commented Jul 26, 2023 •

edited

Loading

mssonicbld commented Jul 26, 2023

mssonicbld commented Jul 26, 2023

mssonicbld commented Jul 26, 2023

slianganet Jul 26, 2023 •

edited

Loading

wenyiz2021 Jul 26, 2023

slianganet Jul 26, 2023

mssonicbld commented Jul 27, 2023

mssonicbld commented Jul 27, 2023

slianganet commented Jul 27, 2023

wenyiz2021 commented Jul 27, 2023

mssonicbld commented Jul 28, 2023

[voq] refine eos fanout function to check fanout link oper status #9128

[voq] refine eos fanout function to check fanout link oper status #9128

Conversation

wenyiz2021 commented Jul 26, 2023 • edited Loading

Description of PR

Type of change

Back port request

Approach

What is the motivation for this PR?

How did you do it?

How did you verify/test it?

Any platform specific information?

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

Documentation

mssonicbld commented Jul 26, 2023

mssonicbld commented Jul 26, 2023

mssonicbld commented Jul 26, 2023

slianganet Jul 26, 2023 • edited Loading

Choose a reason for hiding this comment

wenyiz2021 Jul 26, 2023

Choose a reason for hiding this comment

slianganet Jul 26, 2023

Choose a reason for hiding this comment

mssonicbld commented Jul 27, 2023

mssonicbld commented Jul 27, 2023

slianganet commented Jul 27, 2023

wenyiz2021 commented Jul 27, 2023

mssonicbld commented Jul 28, 2023

wenyiz2021 commented Jul 26, 2023 •

edited

Loading

slianganet Jul 26, 2023 •

edited

Loading