Skip to content

feat: backport orchestrator failswitch workflow tests to release-1.9 - #4246

Merged
gustavolira merged 3 commits into
release-1.9from
backport/orchestrator-failswitch-tests-release-1.9
Feb 13, 2026
Merged

feat: backport orchestrator failswitch workflow tests to release-1.9#4246
gustavolira merged 3 commits into
release-1.9from
backport/orchestrator-failswitch-tests-release-1.9

Conversation

@gustavolira

Copy link
Copy Markdown
Member

Summary

Backport of #3738 and #3996 from main to release-1.9, adapted to the existing CI structure (keeping utils.sh instead of the refactored lib/orchestrator.sh module from main).

CI Scripts (.ibm/pipelines/utils.sh)

  • Migrated orchestrator from user-onboarding to failswitch workflow
  • Changed workflow repo from rhdh-orchestrator-test to rhdhorchestrator
  • Updated manifest paths to fail-switch and greeting workflows (applied via oc apply directly)
  • Updated deploy_orchestrator_workflows() and deploy_orchestrator_workflows_operator() to use failswitch instead of user-onboarding
  • Removed user-onboarding specific secret configuration logic from operator deployment
  • Fixed wait_for_deployment to use sonataflow-platform-data-index-service (was truncated)
  • Updated rbac_deployment() sfp patch with new env vars (QUARKUS_DATASOURCE_REACTIVE_POSTGRESQL_SSL_MODE)

E2E Tests

  • Added failswitch-workflow.spec.ts with comprehensive UI tests (execution, abort, status validations, rerun from failure, cross-workflow linking)
  • Updated orchestrator.ts page object with failswitch methods and updated abort dialog handling
  • Re-enabled greeting-workflow.spec.ts (removed .skip)
  • Added executeCommandWithRetries to log-utils.ts
  • Added orchestrator test skip logic in playwright.config.ts for OSD-GCP and PR OCP helm (non-nightly) jobs

Test plan

  • Verify orchestrator failswitch workflow E2E tests pass on nightly OCP helm jobs
  • Verify greeting workflow E2E tests pass
  • Verify orchestrator tests are properly skipped on OSD-GCP and PR OCP helm jobs
  • Verify deploy_orchestrator_workflows correctly deploys failswitch and greeting workflows
  • Verify deploy_orchestrator_workflows_operator correctly deploys in operator mode

🤖 Generated with Claude Code

Backport of #3738 and #3996 from main, adapted to the release-1.9 CI
structure (keeping utils.sh instead of lib/orchestrator.sh).

Changes:
- Migrate orchestrator from user-onboarding to failswitch workflow
- Update workflow repo from rhdh-orchestrator-test to rhdhorchestrator
- Add failswitch-workflow.spec.ts with comprehensive UI tests
- Update orchestrator page object with new failswitch methods
- Re-enable greeting workflow tests
- Add executeCommandWithRetries to log-utils
- Add orchestrator test skip logic for OSD-GCP and PR OCP helm jobs
- Update sfp patch env vars in rbac_deployment

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@rhdh-qodo-merge

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🔒 Security concerns

Insecure TLS configuration:
.ibm/pipelines/utils.sh sets QUARKUS_DATASOURCE_REACTIVE_TRUST_ALL=true and uses QUARKUS_DATASOURCE_REACTIVE_POSTGRESQL_SSL_MODE=allow, which weakens TLS verification and could enable MITM in non-test environments. Please confirm this is strictly limited to ephemeral CI/test namespaces and cannot leak into production-like deployments.

⚡ Recommended focus areas for review

Retry Logic

The retry loop appears to attempt one more time than intended due to the loop condition, and the logging of attempts/max retries may be misleading. Also consider adding a small backoff between retries to reduce flakiness when the failure is transient (e.g., API server not ready yet).

static async executeCommandWithRetries(
  command: string,
  args: string[] = [],
  maxRetries: number = 3,
): Promise<string> {
  let attempt = 0;
  while (attempt <= maxRetries) {
    try {
      console.log(
        `Attempt ${attempt + 1}/${maxRetries}: Executing command: ${command} ${args.join(" ")}`,
      );
      const output = await LogUtils.executeCommand(command, args);
      console.log(`Command executed successfully on attempt ${attempt + 1}`);
      return output;
    } catch (error) {
      console.error(
        `Error executing command on attempt ${attempt + 1}:`,
        error,
      );
      attempt++;
    }
  }

  throw new Error(
    `Failed to execute command "${command} ${args.join(" ")}" after ${maxRetries} attempts.`,
  );
Flaky Waits

The rollout/pod readiness waiting uses a very small timeout per attempt and relies on repeated oc wait calls; in slower clusters this can cause intermittent failures. Consider increasing the per-attempt timeout and/or using rollout status for the deployment/statefulset, and ensure the overall wait budget matches the expected restart time.

async function restartAndWait(ns: string): Promise<void> {
  console.log("restarting deployment failswitch");
  const restartArgs = [
    "-n",
    ns,
    "rollout",
    "restart",
    "deployment",
    "failswitch",
  ];
  await LogUtils.executeCommand("oc", restartArgs);

  console.log("waiting for pods to be ready");
  const waitArgs = [
    "-n",
    ns,
    "wait",
    "--for=condition=ready",
    "pod",
    "-l",
    "app.kubernetes.io/name=failswitch",
    "--timeout=5s",
  ];
  await LogUtils.executeCommandWithRetries("oc", waitArgs, 5);
}
Brittle Selectors

Several UI assertions rely on exact visible strings and positional selectors (e.g., matching specific dialog title text, -- Aborted, and nth()/text-filtered locators). These are likely to be fragile across minor UI copy/layout changes; prefer stable test ids/roles with scoped locators and avoid strict timestamp regexes unless necessary.

  async abortWorkflow() {
    await expect(
      this.page.getByRole("button", { name: "Abort" }),
    ).toBeEnabled();
    await this.page.getByRole("button", { name: "Abort" }).click();
    await this.page
      .getByRole("dialog", { name: /Abort workflow run\?/i })
      .getByRole("button", { name: "Abort" })
      .click();
    await expect(this.page.getByText("Run has aborted")).toBeVisible();
    await expect(this.page.getByText("-- Aborted")).toBeVisible();
  }

  async validateErrorPopup() {
    await expect(
      this.page.getByRole("button", { name: "Error: Request failed with" }),
    ).toBeVisible();
    await this.page
      .getByRole("button", { name: "Error: Request failed with" })
      .click();
    // Here we can add an error validation check, when we have error messages that can
    // be validated, right now it is the same error for every issue
  }

  async validateErrorPopupDoesNotExist() {
    await expect(
      this.page.getByRole("button", { name: "Error: Request failed with" }),
    ).toHaveCount(0);
  }

  async resetWorkflow() {
    await this.page.getByRole("button", { name: "Reset" }).click();
  }

  async selectFailSwitchWorkflowItem() {
    const workflowHeader = this.page.getByRole("heading", {
      name: "Workflows",
    });
    await expect(workflowHeader).toBeVisible();
    await expect(workflowHeader).toHaveText("Workflows");
    await expect(Workflows.workflowsTable(this.page)).toBeVisible();
    await this.page.getByRole("link", { name: "FailSwitch workflow" }).click();
  }

  async runFailSwitchWorkflow(input = "OK") {
    const runButton = this.page.getByRole("button", { name: "Run" });
    await expect(runButton).toBeVisible();
    await runButton.click();
    await this.page.getByLabel(/switch/i).click();
    await this.page.getByRole("option", { name: input }).click();
    await this.page.getByRole("button", { name: "Next" }).click();
    await this.page.getByRole("button", { name: "Run" }).click();

    switch (input) {
      case "OK":
        await this.validateCurrentWorkflowStatus("Completed");
        break;
      case "KO":
        await this.validateCurrentWorkflowStatus("Failed");
        break;
      case "Wait":
        await this.validateCurrentWorkflowStatus("Running");
        break;
    }
  }

  async validateWorkflowStatusDetails(status = "Completed") {
    const details = this.page
      .getByRole("article")
      .filter({ has: this.page.getByRole("heading", { name: "Workflow" }) });

    if (status === "Running") {
      // Verify Run status heading and spinner in details area
      await expect(
        details.getByRole("heading", { name: /Run\s*status/i }),
      ).toBeVisible();
      await expect(
        this.page
          .locator("b")
          .filter({ hasText: "Running" })
          .getByRole("progressbar"),
      ).toBeVisible();
      // Verify a button shows 'Running' text and has a spinner
      const workflowButtons = this.page
        .locator("div")
        .filter({ hasText: "Abort Running..." })
        .nth(4);
      await expect(workflowButtons).toHaveText(/Running/i);
      await expect(workflowButtons.getByRole("progressbar")).toBeVisible();
      // Results section verifications
      await expect(
        this.page.getByTestId("info-card-subheader").getByRole("img"),
      ).toBeVisible();
      // Verify workflow is running message is visible with timestamp
      // Note: Following line is blocked in main branch due to bug RHDHBUGS-2220. TODO: Uncomment this once the bug is fixed.
      // await expect(this.page.getByText(/workflow is running\.?\s*Started at\s+\d{1,2}\/\d{1,2}\/\d{4},\s+\d{1,2}:\d{2}:\d{2}\s+(AM|PM)/i)).toBeVisible();
    }
    if (status === "Failed") {
      await expect(
        details.getByTestId("ErrorOutlineOutlinedIcon"),
      ).toBeVisible();
      await expect(
        this.page.getByText(
          /Run has failed at\s+\d{1,2}\/\d{1,2}\/\d{4},\s+\d{1,2}:\d{2}:\d{2}\s+(AM|PM)/,
        ),
      ).toBeVisible();
      await expect(
        this.page.getByTestId("ErrorOutlineOutlinedIcon"),
      ).toBeVisible();
    }
    if (status === "Completed") {
      await expect(
        this.page
          .locator("b")
          .filter({ hasText: "Completed" })
          .getByTestId("CheckCircleOutlinedIcon"),
      ).toBeVisible();
      await expect(
        this.page.getByText(
          /Run completed at\s+\d{1,2}\/\d{1,2}\/\d{4},\s+\d{1,2}:\d{2}:\d{2}\s+(AM|PM)/,
        ),
      ).toBeVisible();
      await expect(this.page.getByTestId("SuccessOutlinedIcon")).toBeVisible();
    }
  }

  async validateCurrentWorkflowStatus(status = "Completed", timeout = 120000) {
    await expect(this.page.getByText(`${status}`, { exact: true })).toBeVisible(
      {
        timeout,
      },
    );
  }

  async reRunFailSwitchWorkflow(input = "OK") {
    await expect(this.page.getByText("Run again")).toBeVisible();
    await this.page.getByText("Run again").click();
    await this.page.getByLabel("switch").click();
    await this.page.getByRole("option", { name: input }).click();
    await this.page.getByRole("button", { name: "Next" }).click();
    await this.page.getByRole("button", { name: "Run" }).click();
  }

  async reRunOnFailure(input = "Entire workflow") {
    await expect(this.page.getByText("Run again")).toBeVisible();
    await this.page.getByText("Run again").click();
    await this.page.getByRole("menuitem", { name: input }).click();
  }
}
📄 References
  1. No matching references available

@rhdh-qodo-merge

Copy link
Copy Markdown

PR Type

Enhancement, Tests


Description

  • Backport orchestrator failswitch workflow tests from main to release-1.9

  • Migrate orchestrator workflows from user-onboarding to failswitch workflow

  • Add comprehensive failswitch workflow E2E test suite with execution, abort, and rerun scenarios

  • Update CI scripts to deploy failswitch and greeting workflows with corrected manifests and PostgreSQL configuration

  • Re-enable greeting workflow tests and add orchestrator test skip logic for specific job types


File Walkthrough

Relevant files
Configuration changes
playwright.config.ts
Add orchestrator test skip logic for specific jobs             

e2e-tests/playwright.config.ts

  • Add logic to detect PR OCP helm and OSD-GCP jobs
  • Skip orchestrator tests for non-nightly PR OCP helm jobs and OSD-GCP
    environments
  • Conditionally exclude orchestrator test patterns from test execution
+12/-0   
utils.sh
Migrate orchestrator workflows to failswitch                         

.ibm/pipelines/utils.sh

  • Update workflow repository from rhdh-orchestrator-test to
    rhdhorchestrator
  • Change workflow manifests from user-onboarding to fail-switch and
    greeting workflows
  • Replace helm installation with direct oc apply for workflow manifests
  • Update deploy_orchestrator_workflows to deploy failswitch and greeting
    workflows
  • Update deploy_orchestrator_workflows_operator with corrected
    deployment name and removed user-onboarding specific secret patching
  • Fix wait_for_deployment to use sonataflow-platform-data-index-service
    instead of truncated name
  • Update SonataFlowPlatform patch with corrected PostgreSQL SSL
    environment variables
  • Simplify operator deployment by removing complex user-onboarding
    secret configuration logic
+24/-65 
Enhancement
log-utils.ts
Add command execution with retry logic                                     

e2e-tests/playwright/e2e/audit-log/log-utils.ts

  • Add executeCommandWithRetries method with configurable retry attempts
  • Implement retry logic with console logging for command execution
    attempts
  • Support up to 3 retries by default with error handling
+36/-0   
orchestrator.ts
Add failswitch workflow page object methods                           

e2e-tests/playwright/support/pages/orchestrator.ts

  • Add selectFailSwitchWorkflowItem method to navigate to failswitch
    workflow
  • Add runFailSwitchWorkflow method to execute workflow with input
    parameter selection
  • Add validateWorkflowStatusDetails method to verify Running, Failed,
    and Completed statuses
  • Add validateCurrentWorkflowStatus and
    validateWorkflowAllRunsStatusIcons methods
  • Add reRunFailSwitchWorkflow and reRunOnFailure methods for workflow
    reruns
  • Update abortWorkflow method to use improved dialog selector and status
    validation
  • Update validateWorkflowAllRuns method to improve status filter
    selection logic
+156/-10
Tests
failswitch-workflow.spec.ts
Add failswitch workflow E2E test suite                                     

e2e-tests/playwright/e2e/plugins/orchestrator/failswitch-workflow.spec.ts

  • Add comprehensive failswitch workflow E2E test suite with 6 test cases
  • Test workflow execution with different input parameters (OK, KO, Wait)
  • Test abort functionality, status validations, and rerun from failure
    point
  • Test cross-workflow linking from failswitch to greeting workflow
  • Include helper functions for patching HTTPBIN environment variable and
    pod restart management
+207/-0 
greeting-workflow.spec.ts
Re-enable greeting workflow tests                                               

e2e-tests/playwright/e2e/plugins/orchestrator/greeting-workflow.spec.ts

  • Remove .skip from test suite description to re-enable greeting
    workflow tests
+1/-1     

@rhdh-qodo-merge

rhdh-qodo-merge Bot commented Feb 12, 2026

Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix race condition in restart logic

To prevent a race condition in the restartAndWait function, add a command to
wait for the deployment rollout to complete before waiting for the new pods to
be ready.

e2e-tests/playwright/e2e/plugins/orchestrator/failswitch-workflow.spec.ts [172-196]

 async function restartAndWait(ns: string): Promise<void> {
   console.log("restarting deployment failswitch");
   const restartArgs = [
     "-n",
     ns,
     "rollout",
     "restart",
     "deployment",
     "failswitch",
   ];
   await LogUtils.executeCommand("oc", restartArgs);
 
+  console.log("waiting for rollout to complete");
+  const rolloutStatusArgs = [
+    "-n",
+    ns,
+    "rollout",
+    "status",
+    "deployment/failswitch",
+    "--timeout=120s",
+  ];
+  await LogUtils.executeCommand("oc", rolloutStatusArgs);
+
   console.log("waiting for pods to be ready");
   const waitArgs = [
     "-n",
     ns,
     "wait",
     "--for=condition=ready",
     "pod",
     "-l",
     "app.kubernetes.io/name=failswitch",
-    "--timeout=5s",
+    "--timeout=60s",
   ];
   await LogUtils.executeCommandWithRetries("oc", waitArgs, 5);
 }
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a race condition in the e2e test logic that could lead to flaky tests, and the proposed fix using oc rollout status makes the test significantly more robust.

Medium
General
Simplify abort workflow steps

Refactor the abortWorkflow function to use more robust, case-insensitive
selectors for buttons and status text, and simplify the overall logic.

e2e-tests/playwright/support/pages/orchestrator.ts [219-230]

 async abortWorkflow() {
-  await expect(
-    this.page.getByRole("button", { name: "Abort" }),
-  ).toBeEnabled();
-  await this.page.getByRole("button", { name: "Abort" }).click();
-  await this.page
-    .getByRole("dialog", { name: /Abort workflow run\?/i })
-    .getByRole("button", { name: "Abort" })
-    .click();
-  await expect(this.page.getByText("Run has aborted")).toBeVisible();
-  await expect(this.page.getByText("-- Aborted")).toBeVisible();
+  const abortButton = await this.page.getByRole("button", { name: /Abort/i });
+  await expect(abortButton).toBeEnabled();
+  await abortButton.click();
+  const dialog = this.page.getByRole("dialog");
+  await dialog.getByRole("button", { name: /Abort/i }).click();
+  await expect(this.page.getByText(/aborted/i)).toBeVisible();
 }

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 6

__

Why: The suggestion improves the test's robustness by using more flexible, case-insensitive selectors and simplifies the logic for aborting a workflow, making it less prone to breaking from minor UI text changes.

Low
Correct the retry logic and logging

Refactor the executeCommandWithRetries function to correct the retry loop logic,
ensuring the number of attempts and log messages are accurate.

e2e-tests/playwright/e2e/audit-log/log-utils.ts [42-68]

 static async executeCommandWithRetries(
   command: string,
   args: string[] = [],
   maxRetries: number = 3,
 ): Promise<string> {
-  let attempt = 0;
-  while (attempt <= maxRetries) {
+  const totalAttempts = maxRetries + 1;
+  for (let attempt = 1; attempt <= totalAttempts; attempt++) {
     try {
       console.log(
-        `Attempt ${attempt + 1}/${maxRetries}: Executing command: ${command} ${args.join(" ")}`,
+        `Attempt ${attempt}/${totalAttempts}: Executing command: ${command} ${args.join(" ")}`,
       );
       const output = await LogUtils.executeCommand(command, args);
-      console.log(`Command executed successfully on attempt ${attempt + 1}`);
+      console.log(`Command executed successfully on attempt ${attempt}`);
       return output;
     } catch (error) {
       console.error(
-        `Error executing command on attempt ${attempt + 1}:`,
+        `Error executing command on attempt ${attempt}:`,
         error,
       );
-      attempt++;
+      if (attempt === totalAttempts) {
+        throw new Error(
+          `Failed to execute command "${command} ${args.join(" ")}" after ${totalAttempts} attempts.`,
+        );
+      }
     }
   }
-
+  // This part is now unreachable but kept for type safety.
   throw new Error(
-    `Failed to execute command "${command} ${args.join(" ")}" after ${maxRetries} attempts.`,
+    `Failed to execute command "${command} ${args.join(" ")}" after ${totalAttempts} attempts.`,
   );
 }
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why: The suggestion correctly points out confusing logging and an off-by-one error in the retry logic, improving code clarity and correctness for debugging purposes.

Low
  • Update

@gustavolira

Copy link
Copy Markdown
Member Author

/test ?

@gustavolira

Copy link
Copy Markdown
Member Author

/test e2e-ocp-helm-nightly

@github-actions

Copy link
Copy Markdown
Contributor

After patching a SonataFlow CR, the operator needs time to reconcile and
create the deployment. Without waiting, `oc rollout status` fails with
"deployments.apps not found".

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

🚫 Image Push Skipped.

The container image push was skipped because the build was skipped (either due to [skip-build] tag or no relevant changes with existing image)

Fixes Sonar code smell violations for local variable naming convention.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

🚫 Image Push Skipped.

The container image push was skipped because the build was skipped (either due to [skip-build] tag or no relevant changes with existing image)

@sonarqubecloud

Copy link
Copy Markdown

@gustavolira

Copy link
Copy Markdown
Member Author

/test e2e-ocp-helm-nightly

@gustavolira

Copy link
Copy Markdown
Member Author

/test e2e-ocp-helm

@y-first

y-first commented Feb 13, 2026

Copy link
Copy Markdown
Member

LGTM

@y-first

y-first commented Feb 13, 2026

Copy link
Copy Markdown
Member

/approve
/lgtm

@openshift-ci

openshift-ci Bot commented Feb 13, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: y-first
Once this PR has been reviewed and has the lgtm label, please assign josephca for approval. For more information see the 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

@gustavolira
gustavolira enabled auto-merge (squash) February 13, 2026 15:47

@PatAKnight PatAKnight left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@gustavolira
gustavolira merged commit a0c14bb into release-1.9 Feb 13, 2026
16 of 17 checks passed
@gustavolira
gustavolira deleted the backport/orchestrator-failswitch-tests-release-1.9 branch February 13, 2026 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants