Skip to content

Commit 92f7b3b

Browse files
committed
fix duplicate verifications based on feedback
1 parent 034c616 commit 92f7b3b

File tree

1 file changed

+28
-46
lines changed

1 file changed

+28
-46
lines changed

tests/workbenches/notebook-controller/test_custom_images.py

Lines changed: 28 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66

77
import pytest
88

9-
from kubernetes.dynamic.exceptions import ResourceNotFoundError
109
from ocp_resources.pod import Pod
1110
from ocp_resources.pod import ExecOnPodError
1211
from ocp_resources.namespace import Namespace
@@ -20,8 +19,6 @@
2019

2120
# Error messages
2221
_ERR_EMPTY_PACKAGES = "packages list cannot be empty"
23-
_ERR_POD_NOT_EXISTS = "Pod {pod_name} does not exist"
24-
_ERR_POD_NOT_RUNNING = "Pod {pod_name} is not in Running state (current: {phase})"
2522
_ERR_CONTAINER_NOT_FOUND = "Container '{container_name}' not found in pod. Available containers: {containers}"
2623

2724

@@ -64,20 +61,12 @@ def verify_package_import(
6461
6562
Raises:
6663
ValueError: If packages list is empty
67-
RuntimeError: If pod is not in Running state or container doesn't exist
64+
RuntimeError: If container doesn't exist
6865
"""
6966
# Input validation
7067
if not packages:
7168
raise ValueError(_ERR_EMPTY_PACKAGES)
7269

73-
# Check pod exists and is running
74-
if not pod.exists:
75-
raise ResourceNotFoundError(_ERR_POD_NOT_EXISTS.format(pod_name=pod.name))
76-
77-
pod_status = pod.instance.status
78-
if pod_status.phase != "Running":
79-
raise RuntimeError(_ERR_POD_NOT_RUNNING.format(pod_name=pod.name, phase=pod_status.phase))
80-
8170
# Verify container exists
8271
try:
8372
# Use any() with a generator for a faster check
@@ -103,47 +92,48 @@ def verify_package_import(
10392
LOGGER.debug(f"Executing: {command}")
10493

10594
start_time = time()
95+
output = ""
96+
error_message = None
97+
import_successful = False
98+
pod_logs = None
99+
stderr_output = ""
100+
106101
try:
107102
# Execute command in container with timeout
108103
output = pod.execute(container=container_name, command=command_list, timeout=timeout)
109-
execution_time = time() - start_time
110-
111-
# Success case
112-
results[package_name] = PackageVerificationResult(
113-
package_name=package_name,
114-
import_successful=True,
115-
command_executed=command,
116-
execution_time_seconds=execution_time,
117-
stdout=output if output else "",
118-
)
119-
LOGGER.info(f"Package {package_name}: ✓ (import successful in {execution_time:.2f}s)")
120-
104+
import_successful = True
105+
121106
except ExecOnPodError as e:
122-
execution_time = time() - start_time
123-
124107
# Failure case - extract error message
125108
error_message = str(e)
126109
stderr_output = error_message
127110

128111
# Collect pod logs if requested
129-
pod_logs = None
130112
if collect_diagnostics:
131113
try:
132114
pod_logs = pod.log(container=container_name, tail_lines=100)
133115
except (RuntimeError, AttributeError, KeyError) as log_error:
134116
LOGGER.warning(f"Failed to collect pod logs: {log_error}")
135117
pod_logs = "Could not retrieve pod logs"
136118

137-
results[package_name] = PackageVerificationResult(
138-
package_name=package_name,
139-
import_successful=False,
140-
command_executed=command,
141-
execution_time_seconds=execution_time,
142-
error_message=error_message,
143-
pod_logs=pod_logs,
144-
stderr=stderr_output,
145-
)
146-
LOGGER.warning(f"Package {package_name}: ✗ (import failed: {error_message})")
119+
execution_time = time() - start_time
120+
output = output if output else ""
121+
122+
if import_successful:
123+
LOGGER.info(f"Package {package_name}: ✓ (import successful in {execution_time:.2f}s)")
124+
else:
125+
LOGGER.warning(f"Package {package_name}: ✗ (import failed: {error_message})")
126+
127+
results[package_name] = PackageVerificationResult(
128+
package_name=package_name,
129+
import_successful=import_successful,
130+
command_executed=command,
131+
execution_time_seconds=execution_time,
132+
stdout=output,
133+
error_message=error_message,
134+
pod_logs=pod_logs,
135+
stderr=stderr_output,
136+
)
147137

148138
return results
149139

@@ -171,20 +161,12 @@ def install_packages_in_pod(
171161
172162
Raises:
173163
ValueError: If packages list is empty
174-
RuntimeError: If pod is not in Running state or container doesn't exist
164+
RuntimeError: If container doesn't exist
175165
"""
176166
# Input validation
177167
if not packages:
178168
raise ValueError(_ERR_EMPTY_PACKAGES)
179169

180-
# Check pod exists and is running
181-
if not pod.exists:
182-
raise RuntimeError(_ERR_POD_NOT_EXISTS.format(pod_name=pod.name))
183-
184-
pod_status = pod.instance.status
185-
if pod_status.phase != "Running":
186-
raise RuntimeError(_ERR_POD_NOT_RUNNING.format(pod_name=pod.name, phase=pod_status.phase))
187-
188170
# Verify container exists
189171
try:
190172
# Use any() with a fast, short-circuiting check

0 commit comments

Comments
 (0)