SREP-715: Using the new --dump-guest-cluster-through-kube-service option of the hypershift CLI when dumping dataplane content for must-gathers#883
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@Nikokolas3270: This pull request references SREP-715 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 bug to target the "5.0.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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
WalkthroughChange the default --acm_image from a pinned must-gather tag to ChangesMust-Gather Image Update
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 10 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (10 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Review rate limit: 9/10 reviews remaining, refill in 6 minutes. Comment |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Nikokolas3270 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
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/hcp/mustgather/mustGather.go`:
- Around line 198-200: The HCP must-gather call hardcodes acmHyperShiftImage
("quay.io/stolostron/must-gather:latest") instead of using the configurable
image from the flag; change the image passed to createMustGather to use the
existing configuration value mg.acmMustGatherImage (the same value used earlier
for other gather targets) so the --acm_image flag is honored; update the
acmHyperShiftImage assignment or inline the value when building the args for
createMustGather (which is called with mcRestCfg, mcK8sCli,
[]string{"--dest-dir="+destDir, "--image="+<use mg.acmMustGatherImage>,
gatherScript}) to reference mg.acmMustGatherImage and leave gatherScript,
hcNamespace, hcName, and destDir unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 79676da4-8332-45c8-bfe0-0af1e1f796b0
📒 Files selected for processing (1)
cmd/hcp/mustgather/mustGather.go
|
|
||
| // TODO(ACM-16170): replace this with an official ACM release image once it's available | ||
| acmHyperShiftImage := "quay.io/rokejungrh/must-gather:v2.13.0-33-linux" | ||
| acmHyperShiftImage := "quay.io/stolostron/must-gather:latest" |
There was a problem hiding this comment.
Could we also use the same as CAD here? https://github.com/openshift/configuration-anomaly-detection/blob/main/pkg/investigations/mustgather/mustgather.go#L34
There was a problem hiding this comment.
@rolandmkunkel: to be checked.
Here is the script run through the image:
https://github.com/stolostron/must-gather/blob/main/collection-scripts/gather_spoke_logs
At some point this script will call this function:
https://github.com/stolostron/must-gather/blob/main/collection-scripts/gather_utils#L111
To be checked if CAD does the same or at least something similar.
I guess it would be nice to see if we could converge on what to do for must-gathers for CAD and osdctl. Of course this exceeds the scope of SREP-715 so I will create a new ticket to study that further.
…ion of the hypershift CLI when dumping dataplane content for must-gathers
|
@Nikokolas3270: The following test failed, say
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. |
3 participants
Related changes:
hypershiftCLI: SREP-715: New option to dump data plane content through the kube API server service exposed in HCP namespaces hypershift#6533What this PR does / why we need it:
Port forwarding is not possible as debug handlers are disabled on MC clusters. As a result data plane content (including worker nodes logs) is currently failing to be dumped.
The new --dump-guest-cluster-through-kube-service can be used to by-pass that limitation by targeting the kube-apiserver service exposed by HCP namespaces.
Remark that it is only suitable to use that option when:
Within a MC cluster (i.e. in a pod which has access to the service)
The MC cluster has the debug handlers disabled
Which issue(s) this PR fixes
Contributes to SREP-715
Disclaimer
Remark that using the
quay.io/stolostron/must-gather:latestimage is only a temporary solution. Long term fix is to use theregistry.redhat.io/rhacm2/acm-must-gather-rhel9:v2.17image which will at least hard code the code branch to use, sadly this "release" image does not exist yet and the proposed image is the only one with an acceptable lifetime.Long term fix is tracked there:
https://redhat.atlassian.net/browse/SREP-4777
Summary by CodeRabbit
Summary by CodeRabbit