docs(upgrade): document LogFileMetricExporter delete/recreate step for v5-to-v6 migration - #3356
docs(upgrade): document LogFileMetricExporter delete/recreate step for v5-to-v6 migration#3356Clee2691 wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughChangesLogging 6.0 upgrade
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/administration/upgrade/v6.0_changes.adoc`:
- Around line 97-123: Update the “Before Upgrading” procedure to make removal of
the logfilesmetricexporter DaemonSet an explicit prerequisite after deleting the
LogFileMetricExporter CR. Instruct administrators to verify the DaemonSet is
absent and use the documented oc delete daemonset command if it remains, before
recreating the CR; keep the existing post-upgrade guidance consistent.
- Around line 98-115: Update the LogFileMetricExporter recreation instructions
to preserve the existing v5 resource spec: have administrators save the current
spec before deletion and reapply it when recreating the CR. State that spec: {}
should be used only if the original resource had an empty spec.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 0ccfc178-6257-46fa-b398-0a48940311c7
📒 Files selected for processing (1)
docs/administration/upgrade/v6.0_changes.adoc
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Clee2691, jcantrill 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 |
7d2dd23 to
ffb245b
Compare
|
/retest |
2 similar comments
|
/retest |
|
/retest |
|
/cc @r2d2rnd |
|
/override "Spell Checking" |
|
/lgtm |
|
@jcantrill: /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. |
|
/retest |
|
@Clee2691: 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. |
| oc delete logfilemetricexporters instance -n openshift-logging | ||
| ---- | ||
|
|
||
| .Verify the DaemonSet is absent |
There was a problem hiding this comment.
I'd change the steps for being:
- Verify the Daemonset is absent
oc get daemonset logfilesmetricexporter -n openshift-logging
- Backup it
oc get daemonset logfilesmetricexporter -n openshift-logging -o yaml > logfilemetricsexporter.yaml
- Delete it
oc delete daemonset logfilesmetricexporter -n openshift-logging
- Recreate it....
There was a problem hiding this comment.
The point of doing the second command is IF the daemonset still exists, then delete it. If both these steps are combined then there is no need to verify first, just delete instead. I am inclined to leave it as is.
There was a problem hiding this comment.
Hello @Clee2691 ,
I'm not combinating any step, in fact, I'm adding the backup step, the 2 before deleting it
There was a problem hiding this comment.
@r2d2rnd Oh I was seeing the original comment. I see the edited one now.
Instead of backing up the daemonset, we should be backing up the LFME CR before deleting it. We want the CLO to reconcile the daemonset here.
There was a problem hiding this comment.
@Clee2691 be aware that I should expect to have this step incorporated to the Upgrade documentation [0] as a sorted (numbered step) as it's in the KCS [1]
[0] https://docs.redhat.com/en/documentation/red_hat_openshift_logging/6.4/html/upgrading_logging/upgrading-to-logging-6
[1] https://access.redhat.com/articles/7089860
There was a problem hiding this comment.
Please raise this as a documentation bug/feature in the appropriate JIRA project
r2d2rnd
left a comment
There was a problem hiding this comment.
Pull Request review from 2026-07-28
…r v5-to-v6 migration
|
New changes are detected. LGTM label has been removed. |
Description
This PR documents the processes needed to ensure that the LFME functions after an upgrade from Logging v5 to Logging v6.
/cc @vparfonov
/assign @jcantrill
Links
Summary by CodeRabbit