Skip to content

Commit a14968e

Browse files
hdwhdwmssonicbld
authored andcommitted
Fix gnoi system time test to compare against device time (#21303)
* Fix gnoi system time test to compare against device time The test was comparing gNOI System Time response with local machine time, which can fail due to NTP errors or clock drift. Now compares against the device's own time using shell command. Use device time from shell command instead of local machine time for more accurate gNOI System Time API validation. Replace grpc_channel fixture with create_grpc_channel() function to eliminate SSL state sharing between forked processes. The fixture maintained persistent SSL connections that caused segmentation faults when duthost.shell() operations forked processes that inherited SSL file descriptors. Changes: - Add create_grpc_channel() function in grpc_utils.py - Remove grpc_channel fixture dependency from test - Add explicit channel cleanup in try/finally block - Maintain original test functionality while fixing stability This resolves segfaults in test_gnoi_system_time when combining gRPC operations with Ansible shell commands due to SSL state corruption in multiprocessing.
1 parent 95349d3 commit a14968e

File tree

2 files changed

+89
-21
lines changed

2 files changed

+89
-21
lines changed

tests/gnmi/grpc_utils.py

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
import sys
22
import os
3+
import grpc
4+
import logging
5+
6+
from tests.common.helpers.gnmi_utils import GNMIEnvironment
37

48

59
def get_gnoi_system_stubs():
@@ -16,3 +20,52 @@ def get_gnoi_system_stubs():
1620
sys.path.append(os.path.abspath(PROTO_ROOT))
1721
from gnoi.system import system_pb2_grpc, system_pb2
1822
return system_pb2_grpc, system_pb2
23+
24+
25+
def create_grpc_channel(duthost):
26+
"""
27+
Create a gRPC channel with secure credentials.
28+
29+
This function replaces the grpc_channel fixture to avoid SSL state sharing
30+
issues between forked processes during shell operations.
31+
32+
Args:
33+
duthost: DUT host object
34+
35+
Returns:
36+
grpc.Channel: Configured gRPC channel
37+
"""
38+
# Get DUT gRPC server address and port
39+
ip = duthost.mgmt_ip
40+
env = GNMIEnvironment(duthost, GNMIEnvironment.GNMI_MODE)
41+
port = env.gnmi_port
42+
target = f"{ip}:{port}"
43+
44+
# Load the TLS certificates
45+
with open("gnmiCA.pem", "rb") as f:
46+
root_certificates = f.read()
47+
with open("gnmiclient.crt", "rb") as f:
48+
client_certificate = f.read()
49+
with open("gnmiclient.key", "rb") as f:
50+
client_key = f.read()
51+
52+
# Create SSL credentials
53+
credentials = grpc.ssl_channel_credentials(
54+
root_certificates=root_certificates,
55+
private_key=client_key,
56+
certificate_chain=client_certificate,
57+
)
58+
59+
# Create gRPC channel
60+
logging.info("Creating gRPC secure channel to %s", target)
61+
channel = grpc.secure_channel(target, credentials)
62+
63+
try:
64+
grpc.channel_ready_future(channel).result(timeout=10)
65+
logging.info("gRPC channel is ready")
66+
except grpc.FutureTimeoutError as e:
67+
logging.error("Error: gRPC channel not ready: %s", e)
68+
channel.close()
69+
raise Exception("Failed to connect to gRPC server") from e
70+
71+
return channel
Lines changed: 36 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
import pytest
22
import logging
3-
import time
43

5-
from tests.gnmi.grpc_utils import get_gnoi_system_stubs
4+
from tests.gnmi.grpc_utils import get_gnoi_system_stubs, create_grpc_channel
65

76
pytestmark = [
87
pytest.mark.topology("any"),
@@ -18,24 +17,40 @@
1817
system_pb2_grpc, system_pb2 = get_gnoi_system_stubs()
1918

2019

21-
def test_gnoi_system_time(grpc_channel):
20+
def test_gnoi_system_time(duthosts, rand_one_dut_hostname):
2221
"""
23-
Verify the gNOI System Time API returns the current system time in valid JSON format.
22+
Verify the gNOI System Time API returns the current system time.
2423
"""
25-
# Use the shared gRPC channel
26-
stub = system_pb2_grpc.SystemStub(grpc_channel)
27-
28-
# Create and send request
29-
request = system_pb2.TimeRequest()
30-
response = stub.Time(request)
31-
32-
# Log the response
33-
logging.info("Received response: %s", response)
34-
35-
# Assert the time falls into a reasonable interval
36-
current_time_ns = int(time.time() * 1e9)
37-
reasonable_interval_ns = 60 * 1e9 # 60 seconds in nanoseconds
38-
39-
assert (
40-
abs(response.time - current_time_ns) < reasonable_interval_ns
41-
), f"System time {response.time} is not within the reasonable interval of current time {current_time_ns}"
24+
duthost = duthosts[rand_one_dut_hostname]
25+
26+
# Get device time in seconds first (before gRPC operations)
27+
device_time_result = duthost.shell("date +%s", module_ignore_errors=True)
28+
device_time_s = int(device_time_result["stdout"].strip())
29+
device_time_ns = device_time_s * int(1e9)
30+
31+
# Create gRPC channel (no longer a fixture to avoid SSL state sharing)
32+
channel = create_grpc_channel(duthost)
33+
34+
try:
35+
# Create gRPC stub
36+
stub = system_pb2_grpc.SystemStub(channel)
37+
38+
# Create and send request
39+
request = system_pb2.TimeRequest()
40+
response = stub.Time(request)
41+
42+
# Log the response
43+
logging.info("Received response: %s", response)
44+
logging.info("Device time from shell: %d", device_time_ns)
45+
46+
# Assert the gNOI time is close to device shell time
47+
reasonable_interval_ns = 60 * 1e9 # 60 seconds in nanoseconds
48+
49+
time_diff = abs(response.time - device_time_ns)
50+
assert time_diff < reasonable_interval_ns, (
51+
f"gNOI time {response.time} differs from device time "
52+
f"{device_time_ns} by {time_diff}ns (max: {reasonable_interval_ns}ns)"
53+
)
54+
finally:
55+
# Always close the channel to avoid resource leaks
56+
channel.close()

0 commit comments

Comments
 (0)