Skip to content

feat: Introduce force re-deploy option#87

Open
AndrienkoAleksandr wants to merge 4 commits intoredhat-developer:mainfrom
AndrienkoAleksandr:force-deploy
Open

feat: Introduce force re-deploy option#87
AndrienkoAleksandr wants to merge 4 commits intoredhat-developer:mainfrom
AndrienkoAleksandr:force-deploy

Conversation

@AndrienkoAleksandr
Copy link
Copy Markdown
Contributor

@AndrienkoAleksandr AndrienkoAleksandr commented Apr 20, 2026

What does this pull request do:

Introduce force re-deploy option. This PR introduces a way to redeploy RHDH with modified configuration without full teardown, while reusing existing merge logic.

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
@AndrienkoAleksandr
Copy link
Copy Markdown
Contributor Author

@subhashkhileri, what do you think about this pull request?

@AndrienkoAleksandr
Copy link
Copy Markdown
Contributor Author

@subhash Khileri,

I hope you’re doing well. I’d really appreciate your thoughts on this PR:
#87

Along with that, I wanted to share some use cases I encountered while working on e2e tests.

I’ve recently worked on two e2e-related pull requests:

In both cases, I had separate test.describe sections that required RHDH to be deployed with different application or plugin configurations.

Currently, rhdh.deploy uses the runOnce strategy. Because of that, when I need to redeploy RHDH with a different configuration, I have to rely on either scaleDownAndRestart or teardown. However, both approaches have some drawbacks:

  • scaleDownAndRestart – There’s no ready-to-use logic available for merging application or plugin configurations. This logic is currently outside the scope of rhdh.configure and encapsulated in private methods. As a result, for each new PR, I either need to reimplement this merging logic for plugin tests or ask for those private methods to be exposed.
  • teardown – This completely removes the RHDH namespace. When used together with rhdh.configure and rhdh.deploy, it significantly increases test execution time, since we have to wait for the namespace to be fully deleted and then for a full redeployment.

Given these limitations, do you think the approach proposed in this PR makes sense? Or would you recommend continuing with the existing methods?

Thanks in advance for your feedback!

@subhashkhileri
Copy link
Copy Markdown
Member

subhashkhileri commented Apr 20, 2026

Hey! Thanks for working on this — the bulk-import e2e tests look great. I have some thoughts on the forceUpdate approach that I wanted to share.

Only one describe block needs forceUpdate

Looking at the consumer PR (#1972), only the orchestrator mode describe uses forceUpdate. The other three describes all share the same RHDH configuration (app-config-rhdh.yaml + values.yaml) and already work fine with runOnce deduplication. So the entire new API is driven by a single describe block.

Suggested approach: split into two spec files

The orchestrator mode tests are fully independent — they create their own test data (scoped by Date.now()), clean up in afterAll, and don't share mutable state with the other describes. So we could split into just two files:

// playwright.config.ts
export default defineConfig({
  projects: [
    { name: "bulk-import", testMatch: "bulk-import.spec.ts" },
    { name: "bulk-import-orchestrator", testMatch: "bulk-import-orchestrator.spec.ts" },
  ],
});
  • bulk-import.spec.ts — the three existing describes (same config, one deploy shared via runOnce)
  • bulk-import-orchestrator.spec.ts — orchestrator mode only, its own namespace and deployment

They run in parallel, each with full runOnce protection. No new API needed, and the orchestrator tests don't block the standard tests — so it should be faster overall too.

If they must stay in one file — existing APIs already work

If there's a reason to keep everything in one spec file, the existing public API already supports updating RHDH config mid-test. Just make sure to wrap it in runOnce so it's protected against worker restarts:

await test.runOnce("bulk-import-orchestrator-config", async () => {
  await rhdh.k8sClient.applyConfigMapFromObject(
    "app-config-rhdh",
    newAppConfig,
    rhdh.deploymentConfig.namespace,
  );
  await rhdh.scaleDownAndRestart();
  await rhdh.waitUntilReady();
});

This makes the intent clear to anyone reading the test — the existing deployment is being updated and restarted with a new configuration, rather than a full redeploy happening behind the scenes. And wrapping it in runOnce ensures it won't repeat on worker restarts.

Why I'm cautious about forceUpdate bypassing runOnce

The reason runOnce exists is to protect against Playwright worker restarts. When a test fails, Playwright kills the worker and spins up a new one, which re-runs beforeAll including deploy(). Without runOnce, that triggers a full RHDH redeployment (~10+ min) on every worker restart.

With forceUpdate: true, that protection is gone. If any test in the orchestrator describe fails and triggers retries, each retry spins up a new worker, hits deploy({ forceUpdate: true }), and does a full redeployment from scratch. With the 5 retries configured in CI (retries: process.env.CI ? 5 : 0), what should be a quick retry could turn into 20-30+ minutes of waiting for redeployments.

What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants