refactor: rename template to cvm_deploy#2
Merged
richm merged 1 commit intolinux-system-roles:mainfrom Jan 21, 2026
Merged
Conversation
Reviewer's GuideRenames the Ansible role from the generic 'template' name to 'cvm_deploy', updating variables, internal identifiers, documentation, tests, and CI references to reflect the new role name and purpose for confidential computing machine deployments. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 4 issues, and left some high level feedback:
- In
examples/simple.ymlthe second variable undervarsis still namedcvm_deploy_config1instead ofcvm_deploy_config2, which will override the first value and likely doesn’t match the intended usage. - The test in
tests/tests_default.ymlnow checks for/etc/cvm_deploy.conf, buttasks/main.ymlstill renders the template to/etc/{{ __cvm_deploy_foo_config }}(defaultfoo.conf), so either the dest path or the test expectation should be updated for consistency.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `examples/simple.yml` the second variable under `vars` is still named `cvm_deploy_config1` instead of `cvm_deploy_config2`, which will override the first value and likely doesn’t match the intended usage.
- The test in `tests/tests_default.yml` now checks for `/etc/cvm_deploy.conf`, but `tasks/main.yml` still renders the template to `/etc/{{ __cvm_deploy_foo_config }}` (default `foo.conf`), so either the dest path or the test expectation should be updated for consistency.
## Individual Comments
### Comment 1
<location> `examples/simple.yml:7` </location>
<code_context>
- template_foo: example variable value
- template_bar: false
+ cvm_deploy_config1: example variable value
+ cvm_deploy_config1: false
roles:
- - linux-system-roles.template
</code_context>
<issue_to_address>
**issue (bug_risk):** The second variable in the example playbook should likely be `cvm_deploy_config2` instead of a duplicate `cvm_deploy_config1`.
In `defaults/main.yml` the role defines both `cvm_deploy_config1` and `cvm_deploy_config2`, but this example playbook sets `cvm_deploy_config1` twice, so the first value is overridden and `cvm_deploy_config2` is never shown. Please rename the second line to `cvm_deploy_config2: false` to demonstrate both variables correctly.
</issue_to_address>
### Comment 2
<location> `tasks/set_vars.yml:8-10` </location>
<code_context>
- name: Determine if system is ostree and set flag
- when: not __template_is_ostree is defined
+ when: not __cvm_deploy_is_ostree is defined
block:
- name: Check if system is ostree
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The `when` condition for checking `__cvm_deploy_is_ostree` is using a potentially ambiguous Jinja expression.
`not __cvm_deploy_is_ostree is defined` is parsed as `(not __cvm_deploy_is_ostree) is defined`, which is different from what you likely intend and can behave oddly when the variable is undefined. Please change this to `__cvm_deploy_is_ostree is not defined` to express the condition clearly and avoid precedence issues.
```suggestion
- name: Determine if system is ostree and set flag
when: __cvm_deploy_is_ostree is not defined
block:
```
</issue_to_address>
### Comment 3
<location> `tasks/main.yml:27` </location>
<code_context>
backup: true
mode: "0400"
- notify: Handler for template to restart services
+ notify: Handler for cvm_deploy to restart services
</code_context>
<issue_to_address>
**issue (bug_risk):** The handler name in the notify statement does not match the actual handler name, so the handler will never be triggered.
In `handlers/main.yml` the handler is named `Handler to restart services`, but this task notifies `Handler for cvm_deploy to restart services`. Because Ansible matches handlers by name, the restart will never run. Update the notify string or the handler name so they match.
</issue_to_address>
### Comment 4
<location> `contributing.md:15` </location>
<code_context>
**Bugs and needed implementations** are listed on
-[Github Issues](https://github.com/linux-system-roles/template/issues).
+[Github Issues](https://github.com/linux-system-roles/cvm_deploy/issues).
Issues labeled with
-[**help wanted**](https://github.com/linux-system-roles/template/issues?q=is%3Aissue+is%3Aopen+label%3A%22help+wanted%22)
</code_context>
<issue_to_address>
**nitpick (typo):** Use the correct capitalization "GitHub" instead of "Github" in the link text.
To match the official branding and existing usage, please update the link text to `[GitHub Issues]`.
Suggested implementation:
```
**Bugs and needed implementations** are listed on
[GitHub Issues](https://github.com/linux-system-roles/cvm_deploy/issues).
Issues labeled with
```
```
**Code** is managed on [GitHub](https://github.com/linux-system-roles/cvm_deploy), using
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
8
to
10
| - name: Determine if system is ostree and set flag | ||
| when: not __template_is_ostree is defined | ||
| when: not __cvm_deploy_is_ostree is defined | ||
| block: |
There was a problem hiding this comment.
suggestion (bug_risk): The when condition for checking __cvm_deploy_is_ostree is using a potentially ambiguous Jinja expression.
not __cvm_deploy_is_ostree is defined is parsed as (not __cvm_deploy_is_ostree) is defined, which is different from what you likely intend and can behave oddly when the variable is undefined. Please change this to __cvm_deploy_is_ostree is not defined to express the condition clearly and avoid precedence issues.
Suggested change
| - name: Determine if system is ostree and set flag | |
| when: not __template_is_ostree is defined | |
| when: not __cvm_deploy_is_ostree is defined | |
| block: | |
| - name: Determine if system is ostree and set flag | |
| when: __cvm_deploy_is_ostree is not defined | |
| block: |
|
|
||
| **Bugs and needed implementations** are listed on | ||
| [Github Issues](https://github.com/linux-system-roles/template/issues). | ||
| [Github Issues](https://github.com/linux-system-roles/cvm_deploy/issues). |
There was a problem hiding this comment.
nitpick (typo): Use the correct capitalization "GitHub" instead of "Github" in the link text.
To match the official branding and existing usage, please update the link text to [GitHub Issues].
Suggested implementation:
**Bugs and needed implementations** are listed on
[GitHub Issues](https://github.com/linux-system-roles/cvm_deploy/issues).
Issues labeled with
**Code** is managed on [GitHub](https://github.com/linux-system-roles/cvm_deploy), using
richm
force-pushed
the
template-rename
branch
from
da4fb00 to
a2b2930
Compare
Signed-off-by: Rich Megginson <rmeggins@redhat.com>
richm
force-pushed
the
template-rename
branch
from
a2b2930 to
a543d5f
Compare
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
1 participant
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.
Signed-off-by: Rich Megginson rmeggins@redhat.com
Summary by Sourcery
Rename the Ansible role from a generic template to cvm_deploy and align metadata, variables, examples, and tests with its new purpose of managing confidential computing machine deployments.
Enhancements:
Documentation:
Tests: