sealights steps on pr workflow added

[npotluri-rh]

Description

• Integrate Sealights in RHDH Github Actions Pipelines
• Pre-req is to add SEALIGHTS_AGENT_TOKEN as secret to repo

Tested:

Which issue(s) does this PR fix or relate to

https://issues.redhat.com/browse/RHIDP-6915

PR acceptance criteria

• Tests
• Documentation

How to test changes / Special notes to the reviewer

[openshift-ci]

[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 assign gazarenkov 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:

• OWNERS

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

[rm3l]

@npotluri-rh Is there a related JIRA for this?

[npotluri-rh]

@npotluri-rh Is there a related JIRA for this?

RHIDP-6915

[npotluri-rh]

/assign @subhashkhileri

[gazarenkov]

Let's decide what's the best option to include it.

[gazarenkov]

Since it is planned to add it to nightly (#1136) do we really need it on PR check? Who's going to use it for each PR?

[rm3l]

Since it is planned to add it to nightly (#1136) do we really need it on PR check? Who's going to use it for each PR?

@gazarenkov My understanding is that it would also make sense on PRs. See the email from Shai titled "Why Sealights?". Having some insights like this on PRs could be beneficial for us, I think.

Let's see what we can get from this integration first and eventually revisit this decision with justified data if it does not prove useful.

[psrna]

Like I said, I understand the reason and support this tool. My concern is about:

• unpredictable impact on tests run time (downloading). Our goal always been to make it as short as possible for PR.
• impacting this integration on supportability. Note: the amount of integration Sealights code is pretty big. That's always the risk of catching errors not related to PR itself (updating PR I'd prefer to work on code changes and not on serving infra)
  I am not saying the bad thing will happen, I just prefer to see first and then make a conclusion. And starting with Nightly checks looks like a good candidate for it as we'll do pretty much the same providing the same information (daily is pretty good frequence for making analysis, no?) but with less impact on dev performance.

@gazarenkov Thanks for raising these concerns. I appreciate your thoughtful approach. I do understand the hesitation, especially around test runtime and supportability. That said, I’d still recommend moving forward with this PR for a few reasons:

• Nightly runs have been consistently red for the past three weeks, which suggests they’re not currently giving us reliable or actionable feedback. That limits their value as a testing ground.
• Running this only in nightlies wouldn’t allow us to evaluate its usefulness for PR-level insights, particularly around code change analysis. To validate its potential, we need that visibility at the PR level.
• We currently have a supporting team available to help us integrate this. If we miss this window, it’s likely our team would need to take on the work later, which could add to our load.
• Since this has VP-level mandate, it's much easier for me to have productive conversations at that level when I can point to concrete data showing actual impact, whether positive or negative. If we hold back based on hypothetical concerns, it becomes harder to justify our position. By enabling it now, we can gather real evidence, and if it does cause significant slowdown in PRs, I’ll fully support reverting the change.

Happy to revisit or roll it back if the data shows a strong negative impact, but I believe it's important to try it first so we can make decisions based on evidence, not assumptions.

[rm3l]

FYI, we just merged the other PR for nightlies (#1136), and I triggered a manual run to try it: https://github.com/redhat-developer/rhdh-operator/actions/runs/15050090648/job/42302508633

[gazarenkov]

Like I said, I understand the reason and support this tool. My concern is about:

• unpredictable impact on tests run time (downloading). Our goal always been to make it as short as possible for PR.
• impacting this integration on supportability. Note: the amount of integration Sealights code is pretty big. That's always the risk of catching errors not related to PR itself (updating PR I'd prefer to work on code changes and not on serving infra)
  I am not saying the bad thing will happen, I just prefer to see first and then make a conclusion. And starting with Nightly checks looks like a good candidate for it as we'll do pretty much the same providing the same information (daily is pretty good frequence for making analysis, no?) but with less impact on dev performance.

@gazarenkov Thanks for raising these concerns. I appreciate your thoughtful approach. I do understand the hesitation, especially around test runtime and supportability. That said, I’d still recommend moving forward with this PR for a few reasons:

• Nightly runs have been consistently red for the past three weeks, which suggests they’re not currently giving us reliable or actionable feedback. That limits their value as a testing ground.
• Running this only in nightlies wouldn’t allow us to evaluate its usefulness for PR-level insights, particularly around code change analysis. To validate its potential, we need that visibility at the PR level.
• We currently have a supporting team available to help us integrate this. If we miss this window, it’s likely our team would need to take on the work later, which could add to our load.
• Since this has VP-level mandate, it's much easier for me to have productive conversations at that level when I can point to concrete data showing actual impact, whether positive or negative. If we hold back based on hypothetical concerns, it becomes harder to justify our position. By enabling it now, we can gather real evidence, and if it does cause significant slowdown in PRs, I’ll fully support reverting the change.

Happy to revisit or roll it back if the data shows a strong negative impact, but I believe it's important to try it first so we can make decisions based on evidence, not assumptions.

Thanks @psrna, what I am saying is that we need to do our best to mitigate the risk of applying experimental (yet undoubtedly useful) integration to live project (with GA status but pretty active).

For red nightly jobs (and not only nightly btw) - this is exactly the example of why I prefer less but supported and reliable jobs and tools under more. Flaky tests leads to the state when failing tests become "the norm", the same avoiding running the tests due to theirs slowness become normal. All of this just kill the QA/CI idea IMO.

[psrna]

Thanks @psrna, what I am saying is that we need to do our best to mitigate the risk of applying experimental (yet undoubtedly useful) integration to live project (with GA status but pretty active).

@gazarenkov In the meantime, I looked into how long it takes to set up Sealights and run the scan in the nightly build, as you previously suggested, now that the PR for nightlies has been merged. It took 1 minute and 22 seconds. That seems quite reasonable to me, so I believe we can move forward with it. From my perspective, the concern about its impact on execution time should no longer be an issue.

[gazarenkov]

@gazarenkov In the meantime, I looked into how long it takes to set up Sealights and run the scan in the nightly build, as you previously suggested, now that the PR for nightlies has been merged. It took 1 minute and 22 seconds. That seems quite reasonable to me, so I believe we can move forward with it. From my perspective, the concern about its impact on execution time should no longer be an issue.

@psrna
Good, I can see this:
Scanning completed within 1m15.776514775s , I also can see 2 files ~8.5M each downloaded, which should not take a lot of time (pretty much nothing as for the nightly job I can see)

What does it scan?
Only E2E tests as said "RunSeaLights scan for E2E tests"(which probably does not make a lot of sense) or all the tests (Units and integration as well)?

[npotluri-rh]

@gazarenkov In the meantime, I looked into how long it takes to set up Sealights and run the scan in the nightly build, as you previously suggested, now that the PR for nightlies has been merged. It took 1 minute and 22 seconds. That seems quite reasonable to me, so I believe we can move forward with it. From my perspective, the concern about its impact on execution time should no longer be an issue.

@psrna Good, I can see this: Scanning completed within 1m15.776514775s , I also can see 2 files ~8.5M each downloaded, which should not take a lot of time (pretty much nothing as for the nightly job I can see)

What does it scan? Only E2E tests as said "RunSeaLights scan for E2E tests"(which probably does not make a lot of sense) or all the tests (Units and integration as well)?

Currently on nightly it is only E2E tests but on PR it does Units and integration without E2E

[gazarenkov]

@gazarenkov In the meantime, I looked into how long it takes to set up Sealights and run the scan in the nightly build, as you previously suggested, now that the PR for nightlies has been merged. It took 1 minute and 22 seconds. That seems quite reasonable to me, so I believe we can move forward with it. From my perspective, the concern about its impact on execution time should no longer be an issue.

@psrna Good, I can see this: Scanning completed within 1m15.776514775s , I also can see 2 files ~8.5M each downloaded, which should not take a lot of time (pretty much nothing as for the nightly job I can see)
What does it scan? Only E2E tests as said "RunSeaLights scan for E2E tests"(which probably does not make a lot of sense) or all the tests (Units and integration as well)?

Currently on nightly it is only E2E tests but on PR it does Units and integration without E2E

Why only E2E tests?
Nightly job reflects the current state of code in main (or release) branch (unlike PR branch which until merged is just a transition state). We are interesting in full coverage, not only several E2E smoke tests I believe.