RHTAP-5637: Deployment Tool Flags #1278
Conversation
Passing the flags, force, debug and dry-run to the MCP tool, so the installer job uses the same settings. Also, adapting the `tssc_status` tool to recognize a dry-run job.
|
@otaviof: This pull request references RHTAP-5637 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 story to target the "4.21.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. |
WalkthroughThe installer job logic now builds arguments dynamically, supports debug/dry-run flags, and enforces single-deployment-per-cluster with optional force deletion. A new Run method replaces Create. Deploy tool wiring parses debug/dry-run/force flags, validates topology, and calls Run, updating messages and constants accordingly. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Tool as DeployTool
participant Validator as TopologyValidator
participant Job as InstallerJob
participant K8s as Kubernetes API
User->>Tool: Invoke deploy (args: debug, dryRun, force)
Tool->>Validator: Validate topology
alt invalid topology
Validator-->>Tool: Error
Tool-->>User: Return tool error with guidance
else valid
Tool->>Job: Run(ctx, debug, dryRun, force, ns, image)
Job->>K8s: GetState()
alt Existing job is dry-run-only
K8s-->>Job: Present (marked NotFound)
end
alt No existing deployment
Job->>K8s: createJob(debug/dryRun args)
K8s-->>Job: Created
Job-->>Tool: OK
Tool-->>User: Deployment started / dry-run note
else Deployment exists
alt force = true
Job->>K8s: deleteJob()
K8s-->>Job: Deleted
Job->>K8s: createJob(debug/dryRun args)
K8s-->>Job: Created
Job-->>Tool: OK
Tool-->>User: Re-deployment started (forced)
else force = false
Job-->>Tool: Error (already exists)
Tool-->>User: Error with logs command
end
end
end
sequenceDiagram
autonumber
participant Job as InstallerJob
participant K8s as Kubernetes API
Job->>K8s: Get existing Job spec
K8s-->>Job: Job with containers/args
alt Single container with --dry-run
Note right of Job: Treat as NotFound state
else Otherwise
Note right of Job: Use actual state
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
|
|
@otaviof: This pull request references RHTAP-5637 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 story to target the "4.21.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. |
![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
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
pkg/installer/job.go(8 hunks)pkg/mcptools/deploytools.go(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
pkg/mcptools/deploytools.go (2)
pkg/constants/contants.go (1)
AppName(7-7)pkg/mcptools/statustool.go (1)
StatusToolName(31-31)
🔇 Additional comments (1)
pkg/mcptools/deploytools.go (1)
68-86: Flag parsing reads cleanlyThe boolean flag extraction cleanly defaults to false and respects any values supplied by the MCP request. Nicely done.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dperaza4dustbit, otaviof 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 |
| the TSSC components sequentially.`, | ||
| Deploys TSSC components to the cluster, using the cluster configuration to deploy | ||
| the components sequentially. Note the "dry-run" flag: the deployment process will | ||
| only be initiated when the "dry-run" flag is set to "false". By default, this flag |
There was a problem hiding this comment.
I find it strange that dry-run is the default. I can't think of another tools that is setup this way.
There was a problem hiding this comment.
Yes, that's true. Given the nature of platform engineering, the idea is to have the user explicitly confirm that the deployment should happen. Let's validate this idea and change it if we need to.
|
/retest |
openshift-merge-bot
bot
merged commit bf6e820
into
redhat-appstudio:main
4 participants
Passing the flags, force, debug and dry-run to the MCP tool, so the installer job uses the same settings. Also, adapting the
tssc_statustool to recognise a dry-run job.Summary by CodeRabbit