ci: define basic e2e tests by andyatmiami · Pull Request #1118 · kubeflow/notebooks

andyatmiami · 2026-05-22T03:11:56Z

Add a self-contained Cypress e2e test suite under testing/e2e/ that validates core user workflows against a real Kubernetes cluster. The suite runs two specs: a user creating a Workspace through the wizard, and an admin creating a WorkspaceKind via YAML upload.

Kubernetes environment setup (namespace, RBAC, seed data) is handled programmatically via @kubernetes/client-node in Cypress plugin tasks rather than shell scripts and static YAML manifests. A global before() hook calls setupE2e() before any spec runs, making the test project self-contained with all dependencies co-located. The JupyterLab WorkspaceKind seed data references the controller's sample YAML directly to avoid maintaining duplicate fixtures.

Authentication is simulated by injecting the kubeflow-userid header via cy.intercept(), with separate loginAsAdmin and loginAsUser helpers that map to distinct RBAC policies (cluster-scoped admin vs namespace-scoped user).

Custom Cypress commands (k8sGet, k8sDelete, k8sWaitForResource) provide a thin wrapper over the Kubernetes API for in-test assertions and cleanup. Each spec deletes resources it creates in afterEach() hooks.

Page objects encapsulate UI interactions for the workspaces list, workspace creation wizard, workspace kinds list, and workspace kind creation form.

The CI workflow deploys all three components (controller, backend, frontend) to a Kind cluster with Istio and cert-manager, runs a sanity check to verify endpoints respond, then executes Cypress with the gateway port-forwarded. The frontend is built in standalone deployment mode via .env.production.local so it serves without the central dashboard. Test artifacts (videos, screenshots) are uploaded on failure.

The Makefile provides an e2e convenience target that chains setup-cluster, deploy-all, sanity-check, and local-e2e for local development.

github-actions · 2026-07-22T00:42:18Z

This pull request has been automatically marked as stale because it has not had recent activity.
It will be closed if no further activity occurs.
Thank you for your contributions.

Members may comment /lifecycle frozen to prevent this pull request from being marked as stale.

github-actions · 2026-08-13T00:31:12Z

This pull request has been automatically closed because it has not had recent activity.
You can reopen the PR if you want.

christian-heusel · 2026-08-13T07:56:12Z

/reopen

google-oss-prow · 2026-08-13T07:56:18Z

@christian-heusel: Reopened this PR.

Details

In response to this:

/reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

google-oss-prow · 2026-08-13T07:56:23Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from andyatmiami. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

christian-heusel

Hey @andyatmiami thanks a lot for driving this forward, I think E2E tests will be more and more important as we move forward! 🚧

I have left you a few review comments, feel free to implement or challenge as you deem sensible!

Also could you rebase your changes? A lot has changed since you have initially posted this 😅

christian-heusel · 2026-08-25T18:15:33Z

+      - name: Upload test artifacts
+        uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
+        if: failure()
+        with:
+          name: e2e-test-results
+          path: |
+            testing/e2e/cypress/videos/
+            testing/e2e/cypress/screenshots/
+          retention-days: 7


Do we need to upload all videos and screenshot in the case of a test failure? 🤔

christian-heusel · 2026-08-25T18:16:28Z

+    env:
+      TERM: xterm-256color


What is this used for any why do we need it? 🤔 Please add a comment that explains why it is there in case that it is important 😅

christian-heusel · 2026-08-25T18:19:30Z

This file should most likely also be added to the dependabot config in order to ensure that it stays up-to-date:

notebooks/.github/dependabot.yml

Line 46 in 9bd0400

directory: "/workspaces/frontend"

christian-heusel · 2026-08-25T18:21:33Z

 clean: ## Remove downloaded tool binaries.
 	rm -rf $(LOCALBIN)
 	@echo "INFO: '$(LOCALBIN)' successfully cleaned."
+	rm -rf $(FRONTEND_ENV_LOCAL)


Since this is not a directory we can also drop the -r argument (recursive rm is scary 👻 ):

Suggested change

rm -rf $(FRONTEND_ENV_LOCAL)

rm -f $(FRONTEND_ENV_LOCAL)

christian-heusel · 2026-08-25T18:26:09Z

+for i in $(seq 1 10); do
+  if curl -sk "https://localhost:${GATEWAY_PORT}/workspaces/api/v1/healthcheck" >/dev/null 2>&1; then
+    echo "✓ Gateway port-forward ready"
+    break
+  fi
+  if [[ $i -eq 10 ]]; then
+    echo "✗ ERROR: Gateway port-forward failed to become ready"
+    exit 1
+  fi
+  sleep 2
+done


Same review comment as on the other PR, we can do this part purely in curl:

if ! curl --silent --output /dev/null --fail \ --retry 10 --retry-delay 2 --retry-all-errors \ "http://localhost:${local_port}${probe_path}"; then echo "✗ ERROR: ${label} endpoint not reachable after 10 attempts" return 1 fi

#763 (comment)

christian-heusel · 2026-08-25T19:00:48Z

+async function k8sGet(params: K8sResourceParams): Promise<object> {
+  const api = getClient();
+  if (params.namespace) {
+    const resp = await api.getNamespacedCustomObject({
+      group: params.group,
+      version: params.version,
+      namespace: params.namespace,
+      plural: params.plural,
+      name: params.name,
+    });
+    return resp;
+  }
+  const resp = await api.getClusterCustomObject({
+    group: params.group,
+    version: params.version,
+    plural: params.plural,
+    name: params.name,
+  });
+  return resp;
+}


I think this can be simplified by using variadic parameter expansion & destructuring (applies to a few other places aswell):

Suggested change

async function k8sGet(params: K8sResourceParams): Promise<object> {

const api = getClient();

if (params.namespace) {

const resp = await api.getNamespacedCustomObject({

group: params.group,

version: params.version,

namespace: params.namespace,

plural: params.plural,

name: params.name,

});

return resp;

}

const resp = await api.getClusterCustomObject({

group: params.group,

version: params.version,

plural: params.plural,

name: params.name,

});

return resp;

}

async function k8sGet(params: K8sResourceParams): Promise<object> {

const api = getClient();

const { namespace } = params;

if (namespace) {

return api.getNamespacedCustomObject({ ...params, namespace });

}

return api.getClusterCustomObject(params);

}

christian-heusel · 2026-08-25T19:02:48Z

+function sleep(ms: number): Promise<void> {
+  return new Promise((resolve) => setTimeout(resolve, ms));
+}


Sleep functions are genererally a bit suspicious, why do we need it here and can we replace it with a "native" timeout? 🤔

However the 🤖 says its a pragmatic way of approaching the issue, so maybe we can just leave it as-is:

Is there an alternative way of implementing k8sWaitForResource() that relies on upstream primitives and timeouts?

Yes — two upstream primitives worth considering instead of the hand-rolled Date.now() polling loop:

k8s.Watch (from @kubernetes/client-node) opens a watch connection and streams ADDED/MODIFIED events, so you resolve as soon as the resource appears instead of polling every 2s. It's event-driven and typically faster/lighter, but adds real complexity here: you still need an initial GET (the resource may already exist before the watch starts), you have to handle the watch's own reconnect/error semantics, and cluster-vs-namespaced-scoped watching means building a different path string per case — essentially duplicating the k8sGet branching one level down.

AbortSignal.timeout(ms), passed as abortSignal into the API call options, would let you drop your manual deadline/Date.now() bookkeeping and instead just catch the abort error — but that only replaces the timeout arithmetic, not the poll loop itself, since you'd still need to retry the GET on 404/abort until success.

My take: the current poll-and-isNotFoundError loop is the pragmatic choice for a test helper — it's ~15 lines, easy to reason about, and "wait up to N seconds for a CRD to exist" isn't performance-sensitive in e2e tests. I'd only reach for Watch if polling interval latency were actually causing flaky/slow tests. Want me to prototype the Watch-based version so you can compare?

christian-heusel · 2026-08-25T19:08:46Z

+const CONTROLLER_SAMPLES = path.resolve(
+  __dirname,
+  '../../../../../workspaces/controller/manifests/kustomize/samples',
+);


Maybe it's simpler to somehow figure out the repository root to spare the ../..? 🤔

Not sure if I like this though:

Suggested change

const CONTROLLER_SAMPLES = path.resolve(

__dirname,

'../../../../../workspaces/controller/manifests/kustomize/samples',

);

function findRepoRoot(startDir: string): string {

let dir = startDir;

while (!fs.existsSync(path.join(dir, '.git'))) {

const parent = path.dirname(dir);

if (parent === dir) {

throw new Error(`Could not find repository root above ${startDir}`);

}

dir = parent;

}

return dir;

}

const CONTROLLER_SAMPLES = path.join(

findRepoRoot(__dirname),

'workspaces/controller/manifests/kustomize/samples',

);

christian-heusel · 2026-08-25T19:10:13Z

+    core: kc.makeApiClient(k8s.CoreV1Api),
+    rbac: kc.makeApiClient(k8s.RbacAuthorizationV1Api),
+    custom: kc.makeApiClient(k8s.CustomObjectsApi),


Do we perhaps also need https://kubernetes-client.github.io/javascript/api-reference/workloads/AppsV1Api?_highlight=state#stateful-set-1 here? 🤔

christian-heusel · 2026-08-25T19:11:46Z

+function isConflictError(err: unknown): boolean {
+  if (err instanceof Error && err.message.includes('HTTP-Code: 409')) {
+    return true;
+  }
+  const httpErr = err as { statusCode?: number };
+  return httpErr.statusCode === 409;
+}
+
+function isNotFoundError(err: unknown): boolean {
+  if (err instanceof Error && err.message.includes('HTTP-Code: 404')) {
+    return true;
+  }
+  const httpErr = err as { statusCode?: number };
+  return httpErr.statusCode === 404;
+}


Same comment as above (https://github.com/kubeflow/notebooks/pull/1118/changes#r3856211135) we could potentially use the same solution here if we like it 😊

Add a self-contained Cypress e2e test suite under testing/e2e/ that validates core user workflows against a real Kubernetes cluster. The suite runs two specs: a user creating a Workspace through the wizard, and an admin creating a WorkspaceKind via YAML upload. Kubernetes environment setup (namespace, RBAC, seed data) is handled programmatically via @kubernetes/client-node in Cypress plugin tasks rather than shell scripts and static YAML manifests. A global before() hook calls setupE2e() before any spec runs, making the test project self-contained with all dependencies co-located. The JupyterLab WorkspaceKind seed data references the controller's sample YAML directly to avoid maintaining duplicate fixtures. Authentication is simulated by injecting the kubeflow-userid header via cy.intercept(), with separate loginAsAdmin and loginAsUser helpers that map to distinct RBAC policies (cluster-scoped admin vs namespace-scoped user). Custom Cypress commands (k8sGet, k8sDelete, k8sWaitForResource) provide a thin wrapper over the Kubernetes API for in-test assertions and cleanup. Each spec deletes resources it creates in afterEach() hooks. Page objects encapsulate UI interactions for the workspaces list, workspace creation wizard, workspace kinds list, and workspace kind creation form. The CI workflow deploys all three components (controller, backend, frontend) to a Kind cluster with Istio and cert-manager, runs a sanity check to verify endpoints respond, then executes Cypress with the gateway port-forwarded. The frontend is built in standalone deployment mode via .env.production.local so it serves without the central dashboard. Test artifacts (videos, screenshots) are uploaded on failure. The Makefile provides an `e2e` convenience target that chains setup-cluster, deploy-all, sanity-check, and local-e2e for local development. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Andy Stoneberg <astonebe@redhat.com>

github-project-automation Bot added this to Kubeflow Notebooks May 22, 2026

google-oss-prow Bot added the do-not-merge/work-in-progress label May 22, 2026

github-project-automation Bot moved this to Needs Triage in Kubeflow Notebooks May 22, 2026

google-oss-prow Bot added the area/ci area - related to ci label May 22, 2026

google-oss-prow Bot requested review from caponetto and thaorell May 22, 2026 03:12

google-oss-prow Bot added area/frontend area - related to frontend components area/v2 area - version - kubeflow notebooks v2 size/XXL labels May 22, 2026

andyatmiami force-pushed the test/e2e-cypress-tests branch 4 times, most recently from 624922e to 48df9ca Compare May 22, 2026 03:56

github-actions Bot added the lifecycle/stale label Jul 22, 2026

github-actions Bot closed this Aug 13, 2026

github-project-automation Bot moved this from Needs Triage to Done in Kubeflow Notebooks Aug 13, 2026

google-oss-prow Bot reopened this Aug 13, 2026

github-project-automation Bot moved this from Done to Needs Triage in Kubeflow Notebooks Aug 13, 2026

github-actions Bot removed the lifecycle/stale label Aug 14, 2026

christian-heusel suggested changes Aug 25, 2026

View reviewed changes

google-oss-prow Bot assigned christian-heusel Aug 25, 2026

andyatmiami force-pushed the test/e2e-cypress-tests branch from 48df9ca to eb57635 Compare August 30, 2026 18:41

-const CONTROLLER_SAMPLES = path.resolve(
-  __dirname,
-  '../../../../../workspaces/controller/manifests/kustomize/samples',
-);
+function findRepoRoot(startDir: string): string {
+  let dir = startDir;
+  while (!fs.existsSync(path.join(dir, '.git'))) {
+    const parent = path.dirname(dir);
+    if (parent === dir) {
+      throw new Error(`Could not find repository root above ${startDir}`);
+    }
+    dir = parent;
+  }
+  return dir;
+}
+const CONTROLLER_SAMPLES = path.join(
+  findRepoRoot(__dirname),
+  'workspaces/controller/manifests/kustomize/samples',
+);

Uh oh!

Conversation

andyatmiami commented May 22, 2026

Uh oh!

github-actions Bot commented Jul 22, 2026

Uh oh!

github-actions Bot commented Aug 13, 2026

Uh oh!

christian-heusel commented Aug 13, 2026

Uh oh!

google-oss-prow Bot commented Aug 13, 2026

Uh oh!

google-oss-prow Bot commented Aug 13, 2026

Uh oh!

christian-heusel left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants