Skip to content

Commit 3699117

Browse files
committed
RHOAIENG-48973: fix notebook tests
1 parent 3153fe9 commit 3699117

9 files changed

Lines changed: 81 additions & 20 deletions

File tree

.github/workflows/ui_notebooks_test.yaml

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,38 @@ jobs:
104104
poetry run yarn playwright install chromium
105105
working-directory: ui-tests
106106

107+
- name: Verify Kind cluster connectivity
108+
run: |
109+
echo "Checking kubeconfig location..."
110+
echo "HOME: $HOME"
111+
ls -la ~/.kube/ || echo "No .kube directory found"
112+
113+
echo "Checking kubeconfig format..."
114+
kubectl config view --minify
115+
116+
# Check if kubeconfig has embedded certs or file references
117+
if grep -q "certificate-authority:" ~/.kube/config 2>/dev/null; then
118+
echo "WARNING: kubeconfig uses external certificate files"
119+
fi
120+
if grep -q "certificate-authority-data:" ~/.kube/config 2>/dev/null; then
121+
echo "OK: kubeconfig has embedded certificate data"
122+
fi
123+
124+
echo "Testing cluster connectivity..."
125+
kubectl cluster-info
126+
kubectl get nodes
127+
kubectl get pods -A
128+
129+
# Test Python kubernetes client connectivity
130+
echo "Testing Python kubernetes client..."
131+
python3 -c "
132+
from kubernetes import client, config
133+
config.load_kube_config()
134+
v1 = client.CoreV1Api()
135+
nodes = v1.list_node()
136+
print(f'Successfully connected! Found {len(nodes.items)} nodes')
137+
"
138+
107139
- name: Fix 3_widget_example.ipynb notebook for test
108140
run: |
109141
# Remove login/logout cells, as KinD doesn't support authentication using token
@@ -118,6 +150,13 @@ jobs:
118150
run: |
119151
set -euo pipefail
120152
153+
# Verify kubectl can connect to the cluster
154+
kubectl cluster-info
155+
kubectl get nodes
156+
157+
# Debug: Show kubeconfig location
158+
echo "KUBECONFIG: ${KUBECONFIG:-~/.kube/config}"
159+
121160
poetry run yarn test
122161
working-directory: ui-tests
123162

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
pytorch_lightning==1.9.5
1+
pytorch_lightning==2.6.0
22
ray_lightning
3-
torchmetrics==0.9.1
4-
torchvision==0.19.0
3+
torchmetrics==1.8.2
4+
torchvision==0.20.1
55
minio

src/codeflare_sdk/common/kubernetes_cluster/auth.py

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -284,15 +284,27 @@ def config_check() -> str:
284284
return config_path
285285

286286

287-
def _client_with_cert(client: client.ApiClient, ca_cert_path: Optional[str] = None):
288-
client.configuration.verify_ssl = True
287+
def _client_with_cert(api_client: client.ApiClient, ca_cert_path: Optional[str] = None):
288+
"""
289+
Configure SSL certificate verification for a Kubernetes API client.
290+
291+
If a custom CA cert path is provided or configured via environment variable,
292+
it will be used. Otherwise, the existing ssl_ca_cert configuration from the
293+
kubeconfig (which may include embedded certificates) is preserved.
294+
295+
Args:
296+
api_client: The Kubernetes API client to configure.
297+
ca_cert_path: Optional path to a custom CA certificate file.
298+
"""
299+
api_client.configuration.verify_ssl = True
289300
cert_path = _gen_ca_cert_path(ca_cert_path)
290-
if cert_path is None:
291-
client.configuration.ssl_ca_cert = None
292-
elif os.path.isfile(cert_path):
293-
client.configuration.ssl_ca_cert = cert_path
294-
else:
295-
raise FileNotFoundError(f"Certificate file not found at {cert_path}")
301+
if cert_path is not None:
302+
if os.path.isfile(cert_path):
303+
api_client.configuration.ssl_ca_cert = cert_path
304+
else:
305+
raise FileNotFoundError(f"Certificate file not found at {cert_path}")
306+
# If cert_path is None, preserve the existing ssl_ca_cert from kubeconfig
307+
# (which may contain embedded certificate data from certificate-authority-data)
296308

297309

298310
def _gen_ca_cert_path(ca_cert_path: Optional[str]):

src/codeflare_sdk/common/kubernetes_cluster/test_auth_edge_cases.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,17 +77,20 @@ def test_gen_ca_cert_path_defaults_to_none(monkeypatch):
7777

7878

7979
def test_client_with_cert_none(mocker, monkeypatch):
80-
"""Test _client_with_cert when cert_path is None."""
80+
"""Test _client_with_cert when cert_path is None preserves existing ssl_ca_cert."""
8181
# Clear env var to ensure we're testing the None case
8282
monkeypatch.delenv("CF_SDK_CA_CERT_PATH", raising=False)
8383

8484
mock_client = mocker.MagicMock()
8585
mock_client.configuration = mocker.MagicMock()
86+
# Set an existing ssl_ca_cert to verify it's preserved
87+
mock_client.configuration.ssl_ca_cert = "/existing/ca.crt"
8688

8789
_client_with_cert(mock_client, None)
8890

8991
assert mock_client.configuration.verify_ssl is True
90-
assert mock_client.configuration.ssl_ca_cert is None
92+
# ssl_ca_cert should be preserved (not overwritten to None)
93+
assert mock_client.configuration.ssl_ca_cert == "/existing/ca.crt"
9194

9295

9396
def test_client_with_cert_file_not_found(mocker):

src/codeflare_sdk/common/widgets/test_widgets.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,8 @@ def test_cluster_apply_down_buttons(mocker):
7676

7777
# Check if the `apply` and `down` methods were called
7878
mock_wait_ready.assert_called_once()
79-
mock_apply.assert_called_once()
79+
# Widget button uses shorter TLS timeout (60s) to avoid blocking UI
80+
mock_apply.assert_called_once_with(timeout=60)
8081
mock_down.assert_called_once()
8182

8283

src/codeflare_sdk/common/widgets/widgets.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -299,7 +299,9 @@ def cluster_apply_down_buttons(
299299
def on_apply_button_clicked(b): # Handle the apply button click event
300300
with output:
301301
output.clear_output()
302-
cluster.apply()
302+
# Use shorter TLS timeout (60s) for widget button clicks to avoid blocking UI
303+
# Users who need full TLS wait can use wait_ready() checkbox or call apply() directly
304+
cluster.apply(timeout=60)
303305

304306
# If the wait_ready Checkbox is clicked(value == True) trigger the wait_ready function
305307
if wait_ready_check.value:

tests/e2e/mnist.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ def test_dataloader(self):
249249
callbacks=[TQDMProgressBar(refresh_rate=20)],
250250
num_nodes=int(os.environ.get("GROUP_WORLD_SIZE", 1)),
251251
devices=int(os.environ.get("LOCAL_WORLD_SIZE", 1)),
252-
replace_sampler_ddp=False,
252+
use_distributed_sampler=False,
253253
strategy="ddp",
254254
)
255255

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
--extra-index-url https://download.pytorch.org/whl/cu118
22
torch==2.7.1+cu118
33
torchvision==0.22.1+cu118
4-
pytorch_lightning==1.9.5
4+
pytorch_lightning==2.6.0
55
torchmetrics==1.8.2
66
minio

ui-tests/tests/widget_notebook_example.test.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ test.describe("Widget Functionality", () => {
7171
});
7272

7373
// Test Cluster Apply button WITHOUT the wait_ready checkbox checked
74-
// This avoids the 300s TLS timeout + dashboard_check issues in KinD
74+
// This avoids the long TLS timeout + dashboard_check issues in KinD
7575
await interactWithWidget(page, applyDownWidgetCellIndex, 'button:has-text("Cluster Apply")', async (button) => {
7676
await button.click();
7777

@@ -81,6 +81,10 @@ test.describe("Widget Functionality", () => {
8181
expect(successMessage).not.toBeNull();
8282
});
8383

84+
// Wait for apply() to complete (widget uses 60s TLS timeout)
85+
// We wait for either success or failure message from the TLS setup phase
86+
await page.waitForSelector('text=/Cluster .widgettest. (is ready|resources applied but TLS setup incomplete)/', { timeout: 90000 });
87+
8488
// Test view_clusters widget
8589
const viewClustersCellIndex = 4; // 5 on OpenShift
8690
await page.notebook.runCell(cellCount - 2, true);
@@ -104,8 +108,8 @@ test.describe("Widget Functionality", () => {
104108
// Test Delete Cluster button to clean up
105109
await interactWithWidget(page, viewClustersCellIndex, 'button:has-text("Delete Cluster")', async (button) => {
106110
await button.click();
107-
// Wait for deletion confirmation
108-
const successMessage = await page.waitForSelector(`text=Cluster widgettest in the ${namespace} namespace was deleted successfully.`, { timeout: 10000 });
111+
// Wait for deletion confirmation - increase timeout as cluster deletion can take time
112+
const successMessage = await page.waitForSelector(`text=Cluster widgettest in the ${namespace} namespace was deleted successfully.`, { timeout: 30000 });
109113
expect(successMessage).not.toBeNull();
110114
});
111115
});

0 commit comments

Comments
 (0)