[SREP-940] Add rollout blocking feature#888
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (20)
WalkthroughThis PR refactors promote utilities into pkg/promote, adds component lookup and blocked-version APIs, implements a new osdctl promote block command (with docs), and migrates existing promote commands/tests to use the shared package. ChangesPackage Reorganization and Blocked Version Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
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 `@cmd/promote/blocked/blocked.go`:
- Around line 150-160: The code saves application changes before creating the
new git branch, which can leave uncommitted changes on the wrong branch if
CheckoutNewBranch fails; fix by generating branchName (using ops.serviceId and
ops.gitHash) and calling appInterfaceClone.CheckoutNewBranch(branchName) before
calling application.Save(), then proceed to persist changes (application.Save())
and any subsequent commit/operations; ensure errors from CheckoutNewBranch are
handled and reported just as currently done for Save().
In `@pkg/promote/service.go`:
- Around line 80-108: The loop in CodeComponent.AddBlockedVersion uses
elem.MustString(), which can panic on malformed YAML; replace this with
elem.String() and handle the returned (string, error), validating that the node
is a scalar string and returning a clear error (including c.filePath and the
offending value/index) if conversion fails or yields an empty/invalid value,
before comparing to blockedVersion and proceeding to append; ensure you update
the error messages where applicable so malformed YAML does not cause a crash.
🪄 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: 09c1b1d6-6dd1-485f-968b-6d5946fac99c
📒 Files selected for processing (17)
cmd/promote/blocked/blocked.gocmd/promote/cmd.gocmd/promote/dynatrace/dt_utils.gocmd/promote/dynatrace/dynatrace.gocmd/promote/managedscripts/managed_scripts.gocmd/promote/managedscripts/managed_scripts_test.gocmd/promote/saas/saas.gocmd/promote/saas/saas_test.gopkg/promote/app_interface_clone.gopkg/promote/app_interface_clone_test.gopkg/promote/git_repo.gopkg/promote/service.gopkg/promote/service_test.gopkg/promote/services_registry.gopkg/promote/services_registry_test.gopkg/promote/test_tools.gopkg/promote/utils_test.go
5936387 to
5e258bd
Compare
MateSaary
left a comment
There was a problem hiding this comment.
/label tide/merge-method-squash
/lgtm
|
/retest |
…romote Move all domain types (Service, Application, CodeComponent, AppInterfaceClone, ServicesRegistry, Repo) and business logic (Promote engine, PromoteCallbacks, target filtering, commit message formatting) from cmd/promote/utils/ to pkg/promote/. This follows the established project convention (pkg/controller/rotatesecret.go) of separating business logic from CLI wiring. The cmd/promote/ subcommands now import from pkg/promote instead of a sibling cmd/ package. Package renamed from 'utils' to 'promote' to give the domain its proper identity. No logic changes - pure move and import path update. All existing tests pass unchanged.
Add the ability to block specific SHA versions from being promoted through progressive delivery by adding them to codeComponents[].blockedVersions in the application's app.yaml file. New domain methods in pkg/promote: - Application.GetComponentByName(name) - find a component by name (vs URL) - CodeComponent.AddBlockedVersion(hash) - append to blockedVersions with deduplication; creates the field if absent, errors on duplicates New CLI command: osdctl promote blocked --serviceId <svc> --component <name> --gitHash <sha> The command locates the app.yaml through the SaaS service file, finds the named component, appends the hash to blockedVersions, then branches and commits in app-interface. Includes 9 new unit tests covering: - GetComponentByName: found, not found, multiple components - AddBlockedVersion: create new field, append to existing, reject duplicates, append to multi-entry list
Add Application.GetComponentNames() to pkg/promote which returns all component names from the app.yaml codeComponents array. The blocked command now supports: osdctl promote blocked --serviceId <service> --list This lets users discover available component names without needing to manually inspect app-interface. The --list flag is mutually exclusive with --component and --gitHash. The --component error message now also hints at --list: --component is required (use --list to see available components) The --list path skips the clean-check on the app-interface clone since it is a read-only operation.
…rvice Add Application.GetAllComponents() to pkg/promote which returns all CodeComponent instances from the app.yaml codeComponents array. The blocked command now supports: osdctl promote blocked --serviceId <service> --all --gitHash <sha> This blocks the given SHA in every component of the service's app.yaml in a single commit, instead of requiring one invocation per component. --all and --component are mutually exclusive, enforced both by manual validation and cobra's MarkFlagsMutuallyExclusive. Commit messages adapt to the mode: --component: 'Block version <sha> for <component>' --all: 'Block version <sha> for all components of <service>' Includes a new unit test for GetAllComponents.
- Rename 'osdctl promote blocked' to 'osdctl promote block' - Rename NewCmdBlocked() to NewCmdBlock() for consistency - Move CheckoutNewBranch() before any YAML modifications so the branch is created on a clean state and all file changes happen on the new branch - Update all example strings to use the new command name
3d8fa40 to
159aae4
Compare
|
@bergmannf: 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. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bergmannf, MateSaary 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 |
This feature allows blocking a rollout for single or all components of a service.
Summary by CodeRabbit
New Features
osdctl promote blockto append a git SHA to a component’s blockedVersions (per-component or all components).--listmode shows available services and components.--alland--component, and rejects running on repos with uncommitted changes.Documentation
promote blockcommand and usage examples.