-
Notifications
You must be signed in to change notification settings - Fork 0
Automated Test: feature-rolling-updates-implementation #309
Automated Test: feature-rolling-updates-implementation #309
Conversation
Closes #36840 Signed-off-by: Pedro Ruivo <pruivo@redhat.com>
📝 WalkthroughWalkthroughThe PR introduces a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
operator/src/test/java/org/keycloak/operator/testsuite/integration/UpgradeTest.java (1)
102-117:⚠️ Potential issue | 🔴 CriticalROLLING_UPDATES feature is not enabled when
updateStrategyisnull, but rolling upgrade behavior is still expected.When
updateStrategyisnull, the method returns at Line 106 without enabling theROLLING_UPDATESfeature. However,testCacheMaxCount(null)expectseventuallyRollingUpgradeStatus()(line 94-95) for non-RECREATE strategies.The
update-compatibilityCLI commands checkProfile.isFeatureEnabled(Profile.Feature.ROLLING_UPDATES)and exit with code 4 (FEATURE_DISABLED) if the feature is disabled. If the operator or any downstream reconciliation path invokes these commands, the test will fail whenupdateStrategyisnulldespite expecting rolling upgrade behavior.
🤖 Fix all issues with AI agents
In `@docs/guides/operator/advanced-configuration.adoc`:
- Around line 446-450: Update the CAUTION block in advanced-configuration.adoc
to use consistent grammar: replace the phrase "While on preview stage, the
feature `rolling-updates` must be enabled." with "While in the preview stage,
the feature `rolling-updates` must be enabled." so it matches the wording used
in update-compatibility.adoc; locate the CAUTION block containing
`rolling-updates` and update that sentence only.
In `@docs/guides/server/update-compatibility.adoc`:
- Around line 12-16: Update the sentence in the docs that reads "While on
preview stage, the feature `rolling-updates` must be enabled." to use correct
grammar: change "While on preview stage" to "While in the preview stage" so the
full sentence becomes "While in the preview stage, the feature `rolling-updates`
must be enabled." Reference the exact phrase "While on preview stage" in the
file and replace it accordingly.
🧹 Nitpick comments (2)
docs/guides/templates/kc.adoc (1)
50-55: Hardcoded--features=rolling-updateswill need removal when the feature graduates from preview.This is a reasonable approach for the preview phase. Just be aware that when
ROLLING_UPDATESmoves toType.DEFAULT, this template addition should be reverted to avoid showing an unnecessary flag in all documentation examples.quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/UpdateCompatibilityMetadata.java (1)
46-50: Consider extracting the feature-flag guard intoAbstractUpdatesCommand.This guard block is identical to the one in
UpdateCompatibilityCheck.run(). You could use a template method in the base class to avoid duplicating this check in every subcommand:// In AbstractUpdatesCommand `@Override` public final void run() { if (!Profile.isFeatureEnabled(Profile.Feature.ROLLING_UPDATES)) { printFeatureDisabled(); picocli.exit(CompatibilityResult.FEATURE_DISABLED); return; } printPreviewWarning(); doRun(); } protected abstract void doRun();That said, with only two subcommands this is minor and the current explicit approach is fine.
| [CAUTION] | ||
| ==== | ||
| While on preview stage, the feature `rolling-updates` must be enabled. | ||
| Otherwise, the {project_name} Operator will fail. | ||
| ==== |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same grammar nit: "While on preview stage" → "While in the preview stage".
For consistency with the fix suggested in update-compatibility.adoc.
Proposed fix
[CAUTION]
====
-While on preview stage, the feature `rolling-updates` must be enabled.
+While in the preview stage, the feature `rolling-updates` must be enabled.
Otherwise, the {project_name} Operator will fail.
====📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| [CAUTION] | |
| ==== | |
| While on preview stage, the feature `rolling-updates` must be enabled. | |
| Otherwise, the {project_name} Operator will fail. | |
| ==== | |
| [CAUTION] | |
| ==== | |
| While in the preview stage, the feature `rolling-updates` must be enabled. | |
| Otherwise, the {project_name} Operator will fail. | |
| ==== |
🤖 Prompt for AI Agents
In `@docs/guides/operator/advanced-configuration.adoc` around lines 446 - 450,
Update the CAUTION block in advanced-configuration.adoc to use consistent
grammar: replace the phrase "While on preview stage, the feature
`rolling-updates` must be enabled." with "While in the preview stage, the
feature `rolling-updates` must be enabled." so it matches the wording used in
update-compatibility.adoc; locate the CAUTION block containing `rolling-updates`
and update that sentence only.
| [CAUTION] | ||
| ==== | ||
| While on preview stage, the feature `rolling-updates` must be enabled. | ||
| Otherwise, the commands will fail. | ||
| ==== |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor grammar nit: "While on preview stage" → "While in the preview stage".
Proposed fix
[CAUTION]
====
-While on preview stage, the feature `rolling-updates` must be enabled.
+While in the preview stage, the feature `rolling-updates` must be enabled.
Otherwise, the commands will fail.
====📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| [CAUTION] | |
| ==== | |
| While on preview stage, the feature `rolling-updates` must be enabled. | |
| Otherwise, the commands will fail. | |
| ==== | |
| [CAUTION] | |
| ==== | |
| While in the preview stage, the feature `rolling-updates` must be enabled. | |
| Otherwise, the commands will fail. | |
| ==== |
🤖 Prompt for AI Agents
In `@docs/guides/server/update-compatibility.adoc` around lines 12 - 16, Update
the sentence in the docs that reads "While on preview stage, the feature
`rolling-updates` must be enabled." to use correct grammar: change "While on
preview stage" to "While in the preview stage" so the full sentence becomes
"While in the preview stage, the feature `rolling-updates` must be enabled."
Reference the exact phrase "While on preview stage" in the file and replace it
accordingly.
This pull request was automatically created by
@coderabbitai/e2e-reviewer.Batch created pull request.
Summary by CodeRabbit
New Features
Documentation
Chores