docs(fetcher): add v2.2 upgrade guide#1380
Merged
Merged
Conversation
Requested-by: @guimoreirar
WalkthroughAdds a new upgrade guide for the fetcher Helm chart documenting the migration from v2.1.1 to v2.2.0-beta.4, detailing probe configuration changes, a configuration comparison, and helm diff/upgrade examples. ChangesFetcher Chart Upgrade Documentation
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@charts/fetcher/docs/UPGRADE-2.2.md`:
- Around line 21-29: Update the UPGRADE-2.2.md to reconcile the “no changes
detected” statements with the modified template: either (A) add a short summary
of what changed in templates/manager/deployment.yaml (e.g., field names, default
settings, image/probe/service tweaks) and note whether those are
user-configurable or impact upgrades, referencing
"templates/manager/deployment.yaml"; or (B) explicitly state that the deployment
template was modified for internal/template-only reasons but that no user-facing
values.yaml keys, images, envs, probes, services, ports, or enablement flags
were added/removed/changed, and link that clarification to the existing “No
direct values.yaml key changes detected” sections and the UPGRADE-2.2.md header.
Ensure the chosen clarification replaces the contradictory "no changes detected"
phrasing so readers understand the impact.
- Line 3: The TOC line in UPGRADE-2.2.md is concatenated into a single line;
split the items into a proper Markdown list by placing each entry on its own
line prefixed with a list marker (e.g., "- [Overview](`#overview`)"), ensuring
spacing and newlines between items so the TOC renders as a readable vertical
list (apply this change to the existing concatenated line that contains the
Overview/Version changes/Configuration changes/Template changes/Migration
steps/Preview changes before upgrading/Command to upgrade entries).
- Line 50: Remove or clarify the contradictory Step 2 in UPGRADE-2.2.md: the
step that says "Add any required new values..." conflicts with the
"Configuration changes" note stating "No image, env, secret, probe, ingress,
service, port, or enablement changes detected in values.yaml." Edit the Step 2
wording (or delete it) so it aligns with the Configuration changes section—e.g.,
change Step 2 to a verification step ("Verify there are no required changes to
secrets, configmaps, probes, ingress, or services; if you have custom overrides,
add them") or remove the instruction entirely to avoid implying required action.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 048fae4c-aa24-4309-8ce4-235b18662b63
📒 Files selected for processing (1)
charts/fetcher/docs/UPGRADE-2.2.md
Requested-by: @guimoreirar
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 `@charts/fetcher/docs/UPGRADE-2.2.md`:
- Line 18: Update the UPGRADE-2.2.md text to make the readiness-probe change
explicit: replace the sentence that currently states "No breaking changes" with
a conditional/qualified statement such as "Potentially breaking change:
readiness probe default changed from `/health` to `/readyz` — ensure your app
serves `/readyz` or adjust values.yaml," and/or move the existing warning about
readiness probes into the Overview paragraph so it appears immediately; target
the sentence containing "No breaking changes" and the paragraph referencing the
readiness probe path ("/readyz") for this edit.
- Around line 64-73: The doc currently contradicts itself: "This upgrade
requires no manual migration steps" but step 1 asks users to "pin"
manager.readinessProbe.path, which is a manual change; update the Migration
Steps text to remove the contradiction by rephrasing the first sentence and step
1 to indicate this is optional/precautionary (e.g., "No manual migration
required for most users; if your fetcher-manager serves readiness on /health you
may explicitly set manager.readinessProbe.path: /health to avoid changes") and
mark step 1 as an optional/recommended action rather than a required migration.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ac60b94d-8900-4674-9236-684d592cfae1
📒 Files selected for processing (1)
charts/fetcher/docs/UPGRADE-2.2.md
|
|
||
| This guide covers the `fetcher` chart upgrade from `2.1.1` to `2.2.0-beta.4`. It is a minor maintenance release that makes the `manager` Deployment probes more flexible without changing existing behavior for users who do not override the new knobs. | ||
|
|
||
| The application image (`appVersion: 1.3.0`) is unchanged. No breaking changes, no required `values.yaml` modifications, and no data migration are needed. |
There was a problem hiding this comment.
Clarify the impact of the readiness probe default change.
The document states "No breaking changes," but the readiness probe default changed from /health to /readyz (documented in line 29). If users' applications don't serve the /readyz endpoint, pods will fail readiness checks and be removed from service endpoints, which is a breaking behavioral change for those deployments.
Consider either:
- Revising "No breaking changes" to "No breaking changes if your application serves
/readyz" or "Potentially breaking change: readiness probe path default changed" - Moving the critical warning from line 31 into the Overview section to make the impact immediately clear
📝 Proposed fix to clarify breaking change potential
-The application image (`appVersion: 1.3.0`) is unchanged. No breaking changes, no required `values.yaml` modifications, and no data migration are needed.
+The application image (`appVersion: 1.3.0`) is unchanged. No data migration is needed. **Important:** The readiness probe default path changed from `/health` to `/readyz`—verify your application serves this endpoint or set `manager.readinessProbe.path: /health` before upgrading (see [Configurable probe paths](`#1-configurable-liveness-and-readiness-probe-paths`)).🤖 Prompt for 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.
In `@charts/fetcher/docs/UPGRADE-2.2.md` at line 18, Update the UPGRADE-2.2.md
text to make the readiness-probe change explicit: replace the sentence that
currently states "No breaking changes" with a conditional/qualified statement
such as "Potentially breaking change: readiness probe default changed from
`/health` to `/readyz` — ensure your app serves `/readyz` or adjust
values.yaml," and/or move the existing warning about readiness probes into the
Overview paragraph so it appears immediately; target the sentence containing "No
breaking changes" and the paragraph referencing the readiness probe path
("/readyz") for this edit.
Comment on lines
+64
to
+73
| ## Migration Steps | ||
|
|
||
| This upgrade requires no manual migration steps. The Helm upgrade will roll the `manager` Deployment with the new probe template. | ||
|
|
||
| **Recommended upgrade process:** | ||
|
|
||
| 1. Confirm which path your `fetcher-manager` image serves readiness on. If it is `/health`, pin it explicitly: `manager.readinessProbe.path: /health`. | ||
| 2. Review the changes using the helm-diff plugin (see [Preview changes before upgrading](#preview-changes-before-upgrading)). | ||
| 3. Run the upgrade command during a maintenance window. | ||
| 4. Verify the manager pods are healthy after the upgrade: |
There was a problem hiding this comment.
Resolve contradiction between "no manual migration steps" and step 1.
Line 66 states "This upgrade requires no manual migration steps," but step 1 (line 70) instructs users to "pin it explicitly" if their application serves readiness on /health. Explicitly setting a configuration value to avoid breaking changes is a manual migration step.
📝 Proposed fix to resolve contradiction
-This upgrade requires no manual migration steps. The Helm upgrade will roll the `manager` Deployment with the new probe template.
+The Helm upgrade will automatically roll the `manager` Deployment with the new probe template. If your application serves readiness on `/health` rather than `/readyz`, you must pin the probe path before upgrading (see step 1 below).🤖 Prompt for 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.
In `@charts/fetcher/docs/UPGRADE-2.2.md` around lines 64 - 73, The doc currently
contradicts itself: "This upgrade requires no manual migration steps" but step 1
asks users to "pin" manager.readinessProbe.path, which is a manual change;
update the Migration Steps text to remove the contradiction by rephrasing the
first sentence and step 1 to indicate this is optional/precautionary (e.g., "No
manual migration required for most users; if your fetcher-manager serves
readiness on /health you may explicitly set manager.readinessProbe.path: /health
to avoid changes") and mark step 1 as an optional/recommended action rather than
a required migration.
guimoreirar
changed the title
guimoreirar
approved these changes
May 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Requested by: @guimoreirar