SDCICD-1766: Add must-gather link to krknai report if enabled#3142
SDCICD-1766: Add must-gather link to krknai report if enabled#3142openshift-merge-bot[bot] merged 1 commit intoopenshift:mainfrom
Conversation
|
There are test jobs defined for this repository which are not configured to run automatically. Comment |
openshift-ci-robot
added
the
jira/valid-reference
|
@minlei98: This pull request references SDCICD-1766 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThe changes add optional must-gather collection after chaos test execution, controlled by a new Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant CLI as CLI Handler
participant Cluster as Cluster
participant Engine as Analysis Engine
participant Template as HTML Template
User->>CLI: Run test with --skip-must-gather flag
CLI->>Cluster: Execute chaos test
Cluster->>Cluster: Test completes
alt skip-must-gather not set
Cluster->>Cluster: PostProcessCluster: Phase=MustGatherPhase
Cluster->>Cluster: RunMustGather()
Cluster->>Cluster: Phase restored
end
Cluster->>Engine: Invoke analysis
Engine->>Engine: Compute mustGatherRelativePath
Engine->>Template: Render HTML with MustGatherPath
Template->>Template: Generate report with must-gather link
Template-->>User: HTML report
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@minlei98: This pull request references SDCICD-1766 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
/pipeline required |
|
Scheduling required tests: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/osde2e/krknai/cmd.go`:
- Around line 82-93: The skip-must-gather flag binding in cmd.go causes
artifacts to be collected after reporting because
pkg/krknai/analysisengine/engine.go resolves must-gather during orch.Report(ctx)
but the command still calls orch.PostProcessCluster(ctx) afterward; change the
command flow so that when args.skipMustGather is false you perform the
must-gather/PostProcessCluster step before calling orch.Report(ctx) (or refactor
Report to invoke the must-gather resolution early), updating the invocation
order around orch.Report(ctx) and orch.PostProcessCluster(ctx) to ensure
artifacts are collected prior to report generation and keeping the flag binding
(config.SkipMustGather) intact.
In `@pkg/krknai/krknai.go`:
- Around line 372-374: The deferred viper.Set currently clears the global
config.Phase which can clobber an existing phase; modify the code around the
must-gather block (where viper.Set(config.Phase, MustGatherPhase) is called) to
first read and store the current phase (e.g., prev :=
viper.GetString(config.Phase)), then set MustGatherPhase, and in the defer
restore the saved value (defer viper.Set(config.Phase, prev)) so the original
phase is reinstated instead of being emptied; update any surrounding comments to
reflect restoring previous state.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 5abea0a8-c6cf-4575-81e5-978a86f65749
📒 Files selected for processing (5)
cmd/osde2e/krknai/cmd.gopkg/krknai/analysisengine/engine.gopkg/krknai/analysisengine/engine_test.gopkg/krknai/analysisengine/prompts/report.htmlpkg/krknai/krknai.go
|
@minlei98: This pull request references SDCICD-1766 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
@minlei98: This pull request references SDCICD-1766 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
@minlei98: This pull request references SDCICD-1766 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
openshift-merge-robot
added
needs-rebase
openshift-merge-robot
added
the
needs-rebase
openshift-merge-robot
removed
the
needs-rebase
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: minlei98, ritmun The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
/override hypershift-pr-check |
|
@ritmun: /override requires failed status contexts, check run or a prowjob name to operate on.
Only the following failed contexts/checkruns were expected:
If you are trying to override a checkrun that has a space in it, you must put a double quote on the context. DetailsIn response to this:
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-sigs/prow repository. |
|
/hold |
openshift-ci
bot
added
the
do-not-merge/hold
openshift-merge-robot
added
the
needs-rebase
openshift-merge-robot
removed
the
needs-rebase
minlei98
force-pushed
the
SDCICD-1766
branch
2 times, most recently
from
121afa6 to
b2a8493
Compare
|
/override ci/prow/hypershift-pr-check |
|
/hold cancel |
openshift-ci
bot
removed
the
do-not-merge/hold
|
@ritmun: Overrode contexts on behalf of ritmun: ci/prow/hypershift-pr-check DetailsIn response to this:
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-sigs/prow repository. |
|
@minlei98: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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-sigs/prow repository. I understand the commands that are listed here. |
5 participants
Enables cluster must-gather for the krkn-ai (KAIAAS) flow so users can inspect cluster state when reviewing chaos test results. Reuses the existing osde2e must-gather path and adds a link to it in the krkn-ai HTML report.
Summary by CodeRabbit
--skip-must-gatherflag to optionally bypass must-gather collection after chaos tests (enabled by default).