Skip to content

Commit

Permalink
[202012] Add missing imports exposed after ansible upgrade (#10674)
Browse files Browse the repository at this point in the history
After ansible version of sonic-mgmt docker image is upgraded to 6.7, some scripts failed with errors like:
global name 'os' is not defined

I don't know the reason why these issues are exposed after ansible upgrade. Anyway, these scripts are using modules before import them and need fix.

This change added missing imports for the modules. What's more, this change also fixed some other undefined name issue.

This change is only for 202012 branch. The master and 202305 branch are protected by pre-commit and do not have this issue. For 202205 branch, there was another PR.

Signed-off-by: Xin Wang <xiwang5@microsoft.com>
  • Loading branch information
wangxin committed Nov 9, 2023
1 parent 4473f6d commit 19b74cb
Show file tree
Hide file tree
Showing 33 changed files with 75 additions and 47 deletions.
12 changes: 6 additions & 6 deletions ansible/roles/test/files/ptftests/dhcp_relay_test.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import ast
import struct
import ipaddress
import os

# Packet Test Framework imports
import ptf
Expand Down Expand Up @@ -107,7 +108,7 @@ def setUp(self):

self.hostname = self.test_params['hostname']
self.verified_option82 = False

if self.test_params.has_key('other_client_port'):
self.other_client_port = ast.literal_eval(self.test_params['other_client_port'])

Expand Down Expand Up @@ -171,7 +172,7 @@ def setUp(self):
self.option82 += link_selection

# We'll assign our client the IP address 1 greater than our relay interface (i.e., gateway) IP
self.client_ip = incrementIpAddress(self.relay_iface_ip, 1)
self.client_ip = incrementIpAddress(self.relay_iface_ip, 1)
self.client_subnet = self.test_params['relay_iface_netmask']

self.dest_mac_address = self.test_params['dest_mac_address']
Expand All @@ -184,7 +185,7 @@ def tearDown(self):

"""
Packet generation functions/wrappers
"""

def create_dhcp_discover_packet(self, dst_mac=BROADCAST_MAC, src_port=DHCP_CLIENT_PORT):
Expand Down Expand Up @@ -492,8 +493,8 @@ def client_send_discover(self, dst_mac=BROADCAST_MAC, src_port=DHCP_CLIENT_PORT)
dhcp_discover = self.create_dhcp_discover_packet(dst_mac, src_port)
testutils.send_packet(self, self.client_port_index, dhcp_discover)

#Verify the relayed packet has option82 info or not. Sniffing for the relayed packet on leaves and
#once the packet is recieved checking for the destination and looking into options and verifying
#Verify the relayed packet has option82 info or not. Sniffing for the relayed packet on leaves and
#once the packet is recieved checking for the destination and looking into options and verifying
#the option82 info

def pkt_callback(self, pkt):
Expand Down Expand Up @@ -762,4 +763,3 @@ def runTest(self):
if self.test_params.has_key('other_client_port'):
self.verify_dhcp_relay_pkt_on_other_client_port_with_no_padding(self.dest_mac_address, self.client_udp_src_port)
self.verify_dhcp_relay_pkt_on_server_port_with_no_padding(self.dest_mac_address, self.client_udp_src_port)

8 changes: 5 additions & 3 deletions ansible/roles/test/files/ptftests/dhcpv6_relay_test.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import ast
import os
import socket
import struct
import subprocess

# Packet Test Framework imports
Expand Down Expand Up @@ -96,7 +98,7 @@ def __init__(self):
self.test_params = testutils.test_params_get()
self.client_port_index = int(self.test_params['client_port_index'])
self.client_link_local = self.generate_client_interace_ipv6_link_local_address(self.client_port_index)

DataplaneBaseTest.__init__(self)

def setUp(self):
Expand Down Expand Up @@ -375,10 +377,10 @@ def verify_relayed_request_relay_forward(self):
masked_packet.set_do_not_care_scapy(packet.UDP, "len")
masked_packet.set_do_not_care_scapy(DHCP6OptClientLinkLayerAddr, "clladdr")
masked_packet.set_do_not_care_scapy(scapy.layers.dhcp6.DHCP6_RelayForward, "linkaddr")

# verify packets received on the ports connected to our leaves
testutils.verify_packet_any_port(self, masked_packet, self.server_port_indices)

# Simulate a DHCP server sending a DHCPv6 RELAY-REPLY encapsulating REPLY packet message to client.
def server_send_reply_relay_reply(self):
# Form and send DHCPv6 RELAY-REPLY encapsulating REPLY packet
Expand Down
8 changes: 4 additions & 4 deletions ansible/roles/test/files/ptftests/fib_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -305,14 +305,14 @@ def check_ipv4_route(self, src_port, dst_ip_addr, dst_port_list):
logging.info('Recieved packet at port {} and packet is {} bytes'.format(rcvd_port,len_rcvd_pkt))
logging.info('Recieved packet with length of {}'.format(len_rcvd_pkt))
exp_src_mac = self.router_macs[self.ptf_test_port_map[str(dst_port_list[rcvd_port])]['target_dut']]
actual_src_mac = Ether(rcvd_pkt).src
actual_src_mac = scapy.Ether(rcvd_pkt).src
if exp_src_mac != actual_src_mac:
raise Exception("Pkt sent from {} to {} on port {} was rcvd pkt on {} which is one of the expected ports, "
"but the src mac doesn't match, expected {}, got {}".
format(ip_src, ip_dst, src_port, dst_port_list[rcvd_port], exp_src_mac, actual_src_mac))
return (rcvd_port, rcvd_pkt)
elif self.pkt_action == self.ACTION_DROP:
verify_no_packet_any(self, masked_exp_pkt, dst_ports)
verify_no_packet_any(self, masked_exp_pkt, dst_port_list)
return (None, None)
#---------------------------------------------------------------------

Expand Down Expand Up @@ -384,14 +384,14 @@ def check_ipv6_route(self, src_port, dst_ip_addr, dst_port_list):
logging.info('Recieved packet at port {} and packet is {} bytes'.format(rcvd_port,len_rcvd_pkt))
logging.info('Recieved packet with length of {}'.format(len_rcvd_pkt))
exp_src_mac = self.router_macs[self.ptf_test_port_map[str(dst_port_list[rcvd_port])]['target_dut']]
actual_src_mac = Ether(rcvd_pkt).src
actual_src_mac = scapy.Ether(rcvd_pkt).src
if actual_src_mac != exp_src_mac:
raise Exception("Pkt sent from {} to {} on port {} was rcvd pkt on {} which is one of the expected ports, "
"but the src mac doesn't match, expected {}, got {}".
format(ip_src, ip_dst, src_port, dst_port_list[rcvd_port], exp_src_mac, actual_src_mac))
return (rcvd_port, rcvd_pkt)
elif self.pkt_action == self.ACTION_DROP:
verify_no_packet_any(self, masked_exp_pkt, dst_ports)
verify_no_packet_any(self, masked_exp_pkt, dst_port_list)
return (None, None)

def check_within_expected_range(self, actual, expected):
Expand Down
1 change: 1 addition & 0 deletions ansible/roles/test/files/ptftests/populate_fdb.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import ipaddress
import json
import logging
import os
import ptf

# Packet Test Framework imports
Expand Down
1 change: 1 addition & 0 deletions ansible/roles/test/files/ptftests/radv_ipv6_ra_test.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Packet Test Framework imports
import logging
import os

import ptf
import ptf.testutils as testutils
Expand Down
6 changes: 3 additions & 3 deletions tests/acl/test_acl.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@
"ipv6": "acltb_v6_test_rules.j2"
}
ACL_RULES_PART_TEMPLATES = {
"ipv4": tuple("acltb_test_rules_part_{}.j2".format(i) for i in xrange(1, 3)),
"ipv6": tuple("acltb_v6_test_rules_part_{}.j2".format(i) for i in xrange(1, 3))
"ipv4": tuple("acltb_test_rules_part_{}.j2".format(i) for i in range(1, 3)),
"ipv6": tuple("acltb_v6_test_rules_part_{}.j2".format(i) for i in range(1, 3))
}

DEFAULT_SRC_IP = {
Expand Down Expand Up @@ -129,7 +129,7 @@
def remove_dataacl_table(duthosts):
"""
Remove DATAACL to free TCAM resources.
The change is written to configdb as we don't want DATAACL recovered after reboot
The change is written to configdb as we don't want DATAACL recovered after reboot
"""
TABLE_NAME = "DATAACL"
for duthost in duthosts:
Expand Down
1 change: 1 addition & 0 deletions tests/arp/arp_utils.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import logging
import re
from tests.common.helpers.assertions import pytest_assert
from tests.common.utilities import wait_until

Expand Down
2 changes: 1 addition & 1 deletion tests/bgp/test_bgpmon.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ def bgpmon_peer_connected(duthost, bgpmon_peer):
ptf_interface = "eth" + str(peer_ports[rcvd_port_index])
res = ptfhost.shell('cat /sys/class/net/{}/address'.format(ptf_interface))
original_mac = res['stdout']
ptfhost.shell("ifconfig %s hw ether %s" % (ptf_interface, Ether(rcvd_pkt).dst))
ptfhost.shell("ifconfig %s hw ether %s" % (ptf_interface, scapy.Ether(rcvd_pkt).dst))
ptfhost.shell("ip add add %s dev %s" % (peer_addr + "/24", ptf_interface))
ptfhost.exabgp(name=BGP_MONITOR_NAME,
state="started",
Expand Down
7 changes: 4 additions & 3 deletions tests/common/reboot.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import time
import re
import logging
import sys
from multiprocessing.pool import ThreadPool, TimeoutError
from collections import deque

Expand Down Expand Up @@ -182,7 +183,7 @@ def execute_reboot_helper():
raise Exception('DUT {} did not startup'.format(hostname))

logger.info('ssh has started up on {}'.format(hostname))

logger.info('waiting for switch {} to initialize'.format(hostname))

if safe_reboot:
Expand All @@ -196,14 +197,14 @@ def execute_reboot_helper():

else:
time.sleep(wait)

# Wait warmboot-finalizer service
if reboot_type == REBOOT_TYPE_WARM and wait_warmboot_finalizer:
logger.info('waiting for warmboot-finalizer service to finish on {}'.format(hostname))
ret = wait_until(warmboot_finalizer_timeout, 5, 0, check_warmboot_finalizer_inactive, duthost)
if not ret:
raise Exception('warmboot-finalizer service timeout on DUT {}'.format(hostname))

DUT_ACTIVE.set()
logger.info('{} reboot finished on {}'.format(reboot_type, hostname))
pool.terminate()
Expand Down
1 change: 1 addition & 0 deletions tests/console/test_console_loopback.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import pytest
import pexpect
import random
import string


Expand Down
1 change: 1 addition & 0 deletions tests/container_checker/test_container_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
Test the feature of container_checker
"""
import logging
import time

import pytest
import time
Expand Down
2 changes: 1 addition & 1 deletion tests/drop_packets/test_configurable_drop_counters.py
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ def _generate_vlan_servers(vlan_network, vlan_ports):
# - IP addresses are randomly selected from the given VLAN network
# - "Hosts" (IP/MAC pairs) are distributed evenly amongst the ports in the VLAN
addr_list = list(IPNetwork(vlan_network))
for counter, i in enumerate(xrange(2, VLAN_HOSTS + 2)):
for counter, i in enumerate(range(2, VLAN_HOSTS + 2)):
mac = VLAN_BASE_MAC_PATTERN.format(counter)
port = vlan_ports[i % len(vlan_ports)]
addr = random.choice(addr_list)
Expand Down
13 changes: 7 additions & 6 deletions tests/ecmp/test_fgnhg.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import logging
import ipaddress
import json
from collections import defaultdict
from tests.ptf_runner import ptf_runner
from tests.common import config_reload
from tests.common.helpers.assertions import pytest_assert
Expand Down Expand Up @@ -298,7 +299,7 @@ def fg_ecmp(ptfhost, duthost, router_mac, net_ports, port_list, ip_to_port, bank
flows_per_nh = NUM_FLOWS/len(port_list)
for port in port_list:
exp_flow_count[port] = flows_per_nh

flows_to_redist = exp_flow_count[shutdown_link]
for port in bank_0_port:
if port != shutdown_link:
Expand Down Expand Up @@ -402,7 +403,7 @@ def fg_ecmp(ptfhost, duthost, router_mac, net_ports, port_list, ip_to_port, bank
if port != withdraw_nh_port:
exp_flow_count[port] = flows_for_withdrawn_nh_bank

partial_ptf_runner(ptfhost, 'add_nh', dst_ip, exp_flow_count, add_nh_port=shutdown_link)
partial_ptf_runner(ptfhost, 'add_nh', dst_ip, exp_flow_count, add_nh_port=shutdown_link)


### Send the same flows again, but enable the next-hop which was down previously
Expand All @@ -422,7 +423,7 @@ def fg_ecmp(ptfhost, duthost, router_mac, net_ports, port_list, ip_to_port, bank
for port in port_list:
exp_flow_count[port] = flows_per_nh

partial_ptf_runner(ptfhost, 'add_nh', dst_ip, exp_flow_count, add_nh_port=withdraw_nh_port)
partial_ptf_runner(ptfhost, 'add_nh', dst_ip, exp_flow_count, add_nh_port=withdraw_nh_port)


### Simulate route and link flap conditions by toggling the route
Expand Down Expand Up @@ -467,10 +468,10 @@ def fg_ecmp(ptfhost, duthost, router_mac, net_ports, port_list, ip_to_port, bank
for port in bank_1_port:
exp_flow_count[port] = flows_per_nh

partial_ptf_runner(ptfhost, 'withdraw_bank', dst_ip, exp_flow_count, withdraw_nh_bank=withdraw_nh_bank)
partial_ptf_runner(ptfhost, 'withdraw_bank', dst_ip, exp_flow_count, withdraw_nh_bank=withdraw_nh_bank)


### Send the same flows again, but enable 1 next-hop in a previously down bank to check
### Send the same flows again, but enable 1 next-hop in a previously down bank to check
### if flows redistribute back to previously down bank
logger.info("Send the same flows again, but enable 1 next-hop in a previously down bank to check "
"if flows redistribute back to previously down bank")
Expand Down Expand Up @@ -597,7 +598,7 @@ def common_setup_teardown(tbinfo, duthosts, rand_one_dut_hostname, ptfhost):
if USE_INNER_HASHING is True:
configure_switch_vxlan_cfg(duthost)

yield duthost, cfg_facts, router_mac, net_ports
yield duthost, cfg_facts, router_mac, net_ports

finally:
cleanup(duthost, ptfhost)
Expand Down
1 change: 1 addition & 0 deletions tests/everflow/test_everflow_ipv6.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
"""Test cases to support the Everflow IPv6 Mirroring feature in SONiC."""
import time
import pytest
import ptf.testutils as testutils

Expand Down
1 change: 1 addition & 0 deletions tests/everflow/test_everflow_per_interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import logging
import time
import pytest
import os

import ptf.testutils as testutils
import everflow_test_utilities as everflow_utils
Expand Down
1 change: 1 addition & 0 deletions tests/fdb/test_fdb.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import pprint
import re
import random
from collections import defaultdict

from tests.common.helpers.assertions import pytest_assert
from tests.common.fixtures.ptfhost_utils import change_mac_addresses # lgtm[py/unused-import]
Expand Down
1 change: 1 addition & 0 deletions tests/fdb/test_fdb_mac_move.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import logging
import time
import math
from collections import defaultdict

from tests.common.utilities import wait_until
from tests.common.helpers.assertions import pytest_assert
Expand Down
1 change: 1 addition & 0 deletions tests/memory_checker/test_memory_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
help of new Monit syntax although Monit failed to reset its internal counter.
"""
import logging
import time
from multiprocessing.pool import ThreadPool

import pytest
Expand Down
3 changes: 2 additions & 1 deletion tests/nat/nat_helpers.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import copy
import re
import os
import time
Expand Down Expand Up @@ -1111,7 +1112,7 @@ def get_redis_val(duthost, db, key):
try:
output = exec_command(duthost, ["redis-dump -d {} --pretty -k *{}*".format(db, key)])
if output["rc"]:
raise Exception('Return code is {} not 0'.format(output_cli["rc"]))
raise Exception('Return code is {} not 0'.format(output["rc"]))
redis_dict = json.loads(output['stdout'])
for table in redis_dict:
if 'expireat' in redis_dict[table]:
Expand Down
1 change: 1 addition & 0 deletions tests/nat/test_static_nat.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import copy
import time
import json
import re

import pytest

Expand Down
1 change: 1 addition & 0 deletions tests/pfcwd/test_pfcwd_all_port_storm.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import logging
import os
import time
import pytest

from tests.common.fixtures.conn_graph_facts import fanout_graph_facts
Expand Down
2 changes: 1 addition & 1 deletion tests/pfcwd/test_pfcwd_timer_accuracy.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ def test_pfcwd_timer_accuracy(self, duthosts, rand_one_dut_hostname, pfcwd_timer
self.all_detect_time = list()
self.all_restore_time = list()
try:
for i in xrange(1, 20):
for i in range(1, 20):
logger.info("--- Pfcwd Timer Test iteration #{}".format(i))
self.run_test()
self.verify_pfcwd_timers()
Expand Down
6 changes: 3 additions & 3 deletions tests/platform_tests/thermal_control_test_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ def start_thermal_control_daemon(dut):
dut.start_pmon_daemon(daemon_name)
wait_until(10, 2, 0, check_expected_daemon_status, dut, expected_running_status)
running_daemon_status, _ = dut.get_pmon_daemon_status(daemon_name)
assert running_daemon_status == expected_running_status, "Run command '{}' failed after starting of thermalctld on {}".format(start_pmon_daemon, dut.hostname)
assert running_daemon_status == expected_running_status, "Run command '{}' failed after starting of thermalctld on {}".format("start_pmon_daemon", dut.hostname)
logging.info("thermalctld processes started successfully on {}".format(dut.hostname))

def stop_thermal_control_daemon(dut):
Expand All @@ -314,9 +314,9 @@ def stop_thermal_control_daemon(dut):
dut.stop_pmon_daemon(daemon_name)
wait_until(10, 2, 0, check_expected_daemon_status, dut, expected_stopped_status)
stopped_daemon_status, _ = dut.get_pmon_daemon_status(daemon_name)
assert stopped_daemon_status == expected_stopped_status, "Run command '{}' failed after stopping of thermalctld on {}".format(stop_pmon_daemon, dut.hostname)
assert stopped_daemon_status == expected_stopped_status, "Run command '{}' failed after stopping of thermalctld on {}".format("stop_pmon_daemon", dut.hostname)
logging.info("thermalctld processes stopped successfully on {}".format(dut.hostname))

class ThermalPolicyFileContext:
"""
Context class to help replace thermal control policy file and restore it automatically.
Expand Down
1 change: 1 addition & 0 deletions tests/platform_tests/verify_dut_health.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import pytest
import logging
import time
import traceback
from tests.common.helpers.assertions import pytest_assert
from tests.common.platform.transceiver_utils import parse_transceiver_info
from tests.common.reboot import reboot, REBOOT_TYPE_COLD
Expand Down
3 changes: 3 additions & 0 deletions tests/qos/qos_sai_base.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import ipaddress
import json
import logging
import os
import pytest
import random
import re
import sys
import yaml

from tests.common.fixtures.ptfhost_utils import ptf_portmap_file # lgtm[py/unused-import]
Expand Down
Loading

0 comments on commit 19b74cb

Please sign in to comment.