Add a partial kickstart install test (#308) #522

Merged

adamwill merged 1 commit from partial-kickstart into main 2026-04-21 01:14:09 +00:00

Conversation 7 Commits 1 Files changed 9 +135 -115

adamwill commented 2026-04-20 22:16:35 +00:00

Owner

Copy link

We also split out the _do_root_and_user stuff from
_do_install_and_reboot into a separate test module. This makes
the logic for the partial kickstart install easier, but it's also
just the right thing to do anyway; those things used to be part
of _do_install_and_reboot because you did them while the install
was actually running, but we stopped that since Fedora 31, so it
didn't make any sense for them to be in that module any more.

Signed-off-by: Adam Williamson awilliam@redhat.com

We also split out the _do_root_and_user stuff from _do_install_and_reboot into a separate test module. This makes the logic for the partial kickstart install easier, but it's also just the right thing to do anyway; those things used to be part of _do_install_and_reboot because you did them while the install was actually running, but we stopped that since Fedora 31, so it didn't make any sense for them to be in that module any more. Signed-off-by: Adam Williamson <awilliam@redhat.com>

adamwill force-pushed partial-kickstart from ca9576f7e6 to ff7a4caed6 2026-04-20 22:24:31 +00:00 Compare

adamwill added the

ai-review-please

label 2026-04-20 22:31:12 +00:00

forgejo-actions removed the

ai-review-please

label 2026-04-20 22:31:15 +00:00

forgejo-actions commented 2026-04-20 22:31:24 +00:00

Copy link

AI Code Review

📋 MR Summary

This MR splits out the user and root password creation logic from _do_install_and_reboot.pm into a new module _do_root_and_user.pm. It also introduces support for a PARTIAL_KICKSTART variable, which skips the user/root password creation step during partial kickstart installations.

• Key Changes:
  • Extracted user and root password setup logic into a standalone test module tests/_do_root_and_user.pm.
  • Added the PARTIAL_KICKSTART variable to VARIABLES.md and added logic in main.pm to conditionally skip user/root creation when it is set.
  • Updated comments in utils.pm and _graphical_wait_login.pm to reference the newly renamed module.
  • Added a new template configuration install_kickstart_partial in templates.fif.json.
• Impact: tests/_do_install_and_reboot.pm, tests/_do_root_and_user.pm, main.pm, lib/utils.pm, tests/_graphical_wait_login.pm, templates.fif.json, VARIABLES.md
• Risk Level: 🟢 Low - The changes mostly involve refactoring existing procedural code into a new file and adjusting conditional module loads. No complex logic is altered, and changes appropriately isolate specific test logic without introducing apparent structural issues.

Detailed Code Review

The refactoring is clean and effectively addresses the goal described in the MR. Splitting the user creation from the install step improves modularity and makes skipping it for partial kickstart testing straightforward. No critical security, performance, or logic issues were found during the review. Code transitions well, and references in other files were appropriately updated.

✅ Summary

• Overall Assessment: The changes are straightforward, logically sound, and successfully separate user creation from the installation process. No critical issues identified.

---

🤖 AI Code Review | Generated with ai-code-review | Model: gemini-3.1-pro-preview

⚠️ AI-generated suggestions may be incorrect. Verify before applying. Not a replacement for human review.

## AI Code Review ### 📋 MR Summary This MR splits out the user and root password creation logic from `_do_install_and_reboot.pm` into a new module `_do_root_and_user.pm`. It also introduces support for a `PARTIAL_KICKSTART` variable, which skips the user/root password creation step during partial kickstart installations. - **Key Changes:** - Extracted user and root password setup logic into a standalone test module `tests/_do_root_and_user.pm`. - Added the `PARTIAL_KICKSTART` variable to `VARIABLES.md` and added logic in `main.pm` to conditionally skip user/root creation when it is set. - Updated comments in `utils.pm` and `_graphical_wait_login.pm` to reference the newly renamed module. - Added a new template configuration `install_kickstart_partial` in `templates.fif.json`. - **Impact:** tests/_do_install_and_reboot.pm, tests/_do_root_and_user.pm, main.pm, lib/utils.pm, tests/_graphical_wait_login.pm, templates.fif.json, VARIABLES.md - **Risk Level:** 🟢 Low - The changes mostly involve refactoring existing procedural code into a new file and adjusting conditional module loads. No complex logic is altered, and changes appropriately isolate specific test logic without introducing apparent structural issues. ### Detailed Code Review The refactoring is clean and effectively addresses the goal described in the MR. Splitting the user creation from the install step improves modularity and makes skipping it for partial kickstart testing straightforward. No critical security, performance, or logic issues were found during the review. Code transitions well, and references in other files were appropriately updated. ### ✅ Summary - **Overall Assessment:** The changes are straightforward, logically sound, and successfully separate user creation from the installation process. No critical issues identified. --- 🤖 **AI Code Review** | Generated with [ai-code-review](https://gitlab.com/redhat/edge/ci-cd/ai-code-review) | **Model:** `gemini-3.1-pro-preview` ⚠️ *AI-generated suggestions may be incorrect. Verify before applying. Not a replacement for human review.*

adamwill force-pushed partial-kickstart from ff7a4caed6 to 1351991d07 2026-04-20 22:53:23 +00:00 Compare

adamwill force-pushed partial-kickstart from 1351991d07 to 4adffd25a7 2026-04-20 23:10:32 +00:00 Compare

adamwill force-pushed partial-kickstart from 4adffd25a7 to 7d719e8962 2026-04-20 23:29:56 +00:00 Compare

adamwill force-pushed partial-kickstart from 7d719e8962 to b4e92ee6c1 2026-04-20 23:42:11 +00:00 Compare

adamwill added the

ai-review-please

label 2026-04-20 23:53:08 +00:00

forgejo-actions removed the

ai-review-please

label 2026-04-20 23:53:11 +00:00

forgejo-actions commented 2026-04-20 23:53:20 +00:00

Copy link

AI Code Review

📋 MR Summary

This MR separates the root and user password setup logic from the main install and reboot module into a new _do_root_and_user test module. It also introduces support for a PARTIAL_KICKSTART installation by adjusting the test flow accordingly.

• Key Changes:
  • Added PARTIAL_KICKSTART variable to documentation and test templates.
  • Moved _do_root_and_user logic from tests/_do_install_and_reboot.pm to a new tests/_do_root_and_user.pm module.
  • Modified main.pm to conditionally load software selection and the new _do_root_and_user.pm module based on PARTIAL_KICKSTART.
  • Updated _boot_to_anaconda.pm to handle language/layout selection skipping if PARTIAL_KICKSTART is set.
  • Fixed references in various files from _do_install_and_reboot to _do_root_and_user.
• Impact: main.pm, lib/utils.pm, tests/_do_install_and_reboot.pm, tests/_do_root_and_user.pm, tests/_boot_to_anaconda.pm, tests/_software_selection.pm, templates.fif.json, VARIABLES.md
• Risk Level: 🟢 Low - The changes mostly involve a safe code refactoring (extracting a subroutine into its own file) and adding a new test execution path without altering the primary behavior of existing paths.

Detailed Code Review

The changes look solid and well-reasoned. Splitting out the user/root setup into a separate module cleans up the _do_install_and_reboot step logically. The addition of the PARTIAL_KICKSTART condition handles the test branching cleanly. Documentation has been properly updated for the newly introduced variable. No major issues were found, just a few cleanups and verifications regarding the conditional execution flows.

✅ Summary

• Overall Assessment: No critical issues identified. The code refactoring is clean, and the logic aligns well with the stated intent.
• Minor Suggestions:
  • Ensure tests/_do_root_and_user.pm is included in test distribution/installation manifests if applicable.
  • Verify whether install_kickstart_partial in templates.fif.json needs further variable settings comparable to standard kickstarts.

---

🤖 AI Code Review | Generated with ai-code-review | Model: gemini-3.1-pro-preview

⚠️ AI-generated suggestions may be incorrect. Verify before applying. Not a replacement for human review.

## AI Code Review ### 📋 MR Summary This MR separates the root and user password setup logic from the main install and reboot module into a new `_do_root_and_user` test module. It also introduces support for a `PARTIAL_KICKSTART` installation by adjusting the test flow accordingly. - **Key Changes:** - Added `PARTIAL_KICKSTART` variable to documentation and test templates. - Moved `_do_root_and_user` logic from `tests/_do_install_and_reboot.pm` to a new `tests/_do_root_and_user.pm` module. - Modified `main.pm` to conditionally load software selection and the new `_do_root_and_user.pm` module based on `PARTIAL_KICKSTART`. - Updated `_boot_to_anaconda.pm` to handle language/layout selection skipping if `PARTIAL_KICKSTART` is set. - Fixed references in various files from `_do_install_and_reboot` to `_do_root_and_user`. - **Impact:** main.pm, lib/utils.pm, tests/_do_install_and_reboot.pm, tests/_do_root_and_user.pm, tests/_boot_to_anaconda.pm, tests/_software_selection.pm, templates.fif.json, VARIABLES.md - **Risk Level:** 🟢 Low - The changes mostly involve a safe code refactoring (extracting a subroutine into its own file) and adding a new test execution path without altering the primary behavior of existing paths. ### Detailed Code Review The changes look solid and well-reasoned. Splitting out the user/root setup into a separate module cleans up the `_do_install_and_reboot` step logically. The addition of the `PARTIAL_KICKSTART` condition handles the test branching cleanly. Documentation has been properly updated for the newly introduced variable. No major issues were found, just a few cleanups and verifications regarding the conditional execution flows. ### ✅ Summary - **Overall Assessment:** No critical issues identified. The code refactoring is clean, and the logic aligns well with the stated intent. - **Minor Suggestions:** - Ensure `tests/_do_root_and_user.pm` is included in test distribution/installation manifests if applicable. - Verify whether `install_kickstart_partial` in `templates.fif.json` needs further variable settings comparable to standard kickstarts. --- 🤖 **AI Code Review** | Generated with [ai-code-review](https://gitlab.com/redhat/edge/ci-cd/ai-code-review) | **Model:** `gemini-3.1-pro-preview` ⚠️ *AI-generated suggestions may be incorrect. Verify before applying. Not a replacement for human review.*

adamwill referenced this pull request from a commit 2026-04-20 23:54:36 +00:00

Add wiki reporting for partial kickstart install test

adamwill referenced this pull request from quality/fedora_openqa 2026-04-20 23:54:47 +00:00

Add wiki reporting for partial kickstart install test #135

adamwill commented 2026-04-20 23:54:55 +00:00

Author

Owner

Copy link

Reporting PR: quality/fedora_openqa#135

Reporting PR: https://forge.fedoraproject.org/quality/fedora_openqa/pulls/135

adamwill commented 2026-04-20 23:55:41 +00:00

Author

Owner

Copy link

This works in staging, merging.

This [works in staging](https://openqa.stg.fedoraproject.org/tests/6282978), merging.

adamwill force-pushed partial-kickstart from b4e92ee6c1 to 9d0a1092e6 2026-04-20 23:59:44 +00:00 Compare

adamwill commented 2026-04-21 00:00:25 +00:00

Author

Owner

Copy link

Well, actually, I'll do a full compose run first to check the flow changes don't break anything else.

Well, actually, I'll do a [full compose run](https://openqa.stg.fedoraproject.org/tests/overview?version=44&distri=fedora&build=Fedora-44-20260420.1&groupid=1) first to check the flow changes don't break anything else.

adamwill commented 2026-04-21 00:02:56 +00:00

Author

Owner

Copy link

Note on dropping MEMCHECK from the _software_selection bail-out path in the move: there's only one test where MEMCHECK is set (memory_check), and in that test, PACKAGE_SET is set to "workstation". So we never actually hit this path. I probably put it in at a time when the memory_check test used "default" as the package set.

Note on dropping `MEMCHECK` from the _software_selection bail-out path in the move: there's only one test where `MEMCHECK` is set (`memory_check`), and in that test, `PACKAGE_SET` is set to "workstation". So we never actually hit this path. I probably put it in at a time when the memory_check test used "default" as the package set.

adamwill commented 2026-04-21 01:13:49 +00:00

Author

Owner

Copy link

Full compose run looks good. Merging.

Full compose run looks good. Merging.

adamwill merged commit 9d0a1092e6 into main 2026-04-21 01:14:09 +00:00

adamwill deleted branch partial-kickstart 2026-04-21 01:14:09 +00:00

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

ai-review-please

label for requesting AI code review

  

Closed As

Duplicate

Closed with the reason of Duplicate

  

Closed As

Fixed

Closed with the reason of Fixed

  

Closed As

Invalid

Closed with the reason of Invalid

  

easyfix

  

freeipa

issues or enhancement requests related to FreeIPA in some way

  

improvetest

some kind of improvement (but not bug fix) to an existing test

  

iot

issues relating to test enablement on Fedora IoT

  

newtest

requests for new tests

  

silverblue

issues relating to test enablement on Fedora Silverblue

  

tracker

tracker issue, only exists to track a set of related issues

  

Backlog Status

Needs Review

Work items that have been created or updated but need review, refinement, or approval before they can be considered ready for sprint planning.

  

Backlog Status

Ready

Work items that have been reviewed, refined, and are ready to be committed to a sprint. These items have clear requirements and can be worked on immediately when capacity is available.

  

chore

Work that needs to be done but doesn't add new functionality or fix bugs. Maintenance tasks that keep the system running smoothly and the codebase clean.

  

documentation

Work focused on creating, updating, or improving documentation for users, developers, or system administrators.

  

points

01

A task that is simple, small, and has very little uncertainty. It should be easy enough for any team member to pick up and complete quickly, without a lot of discussion or research.

  

points

02

This is a task that is slightly more complex or larger than a "1." It might involve two or three simple steps, but the outcome is still very clear and there's little uncertainty.

  

points

03

Medium-sized task with some noticeable complexity. It might require a bit more thought or collaboration. The steps are well-defined, but it's not a trivial change.

  

points

05

A significant piece of work. It has more complexity and a higher degree of uncertainty than smaller tasks. It might require some research, a design discussion, or collaboration across multiple teams.

  

points

08

Complex task with a lot of uncertainty. It's often the largest size that a team will consider for a single sprint. It likely requires a detailed breakdown and multiple team members to complete.

  

points

13

Any task that feels like it's a "13" or more is considered an Epic. These are too large and uncertain to be worked on in a single sprint and should be broken down.

  

Priority

Critical

This is for work that is urgent and of critical importance and so and must be addressed immediately with a higher priority than all other work, even if not originally planned for the current sprint.

  

Priority

High

This is for work that is critical and needs to be addressed in the next sprint. It's often for urgent bugs, important features, or a major piece of technical debt that is blocking other work.

  

Priority

Low

This is for work that is good to do but not urgent. It's a candidate for being worked on when there's extra time or when it rises in importance. It can stay in the backlog for a longer period.

  

Priority

Medium

This is for important work that should be done soon, but isn't as urgent as a high-priority item. It will likely be considered for the next 1-2 sprints.

  

Sprint Status

Blocked

Work items that cannot progress due to external dependencies, blockers, or issues that need resolution.

  

Sprint Status

Done

Work items that have been completed, reviewed, and integrated. No further work is needed within the current sprint.

  

Sprint Status

In Progress

Work items currently being worked on by a team member. Active development, research, or implementation is happening.

  

Sprint Status

Review

Work items that are complete but need review, testing, or approval before being marked as done.

  

Sprint Status

To Do

Work items that are committed to the sprint but haven't been started yet. These items are ready to be picked up by team members.

  

Technical Debt

Work that addresses shortcuts, workarounds, or suboptimal implementations that were done previously to meet deadlines. These items improve code quality, maintainability, or performance without adding new features.

  

Work Item

Bug

Issues where the system is not working as intended or designed. Bugs represent deviations from expected behavior.

  

Work Item

Epic

Large initiatives that are broken down into multiple smaller work items (user stories, tasks, bugs). Epics represent significant features or projects that span multiple sprints.

  

Work Item

Spike

Time-boxed research, investigation, or proof-of-concept work to reduce uncertainty or explore solutions. Spikes help inform future development decisions.

  

Work Item

Task

Used to break down larger user stories into smaller, manageable implementation pieces. They focus on the specific technical steps needed to deliver the user story's value.

  

Work Item

User Story

User-facing features or functionality that can be framed from a user perspective, describing what someone wants to accomplish and why.

No labels

ai-review-please

Closed As

Duplicate

Closed As

Fixed

Closed As

Invalid

easyfix

freeipa

improvetest

iot

newtest

silverblue

tracker

Backlog Status

Needs Review

Backlog Status

Ready

chore

documentation

points

01

points

02

points

03

points

05

points

08

points

13

Priority

Critical

Priority

High

Priority

Low

Priority

Medium

Sprint Status

Blocked

Sprint Status

Done

Sprint Status

In Progress

Sprint Status

Review

Sprint Status

To Do

Technical Debt

Work Item

Bug

Work Item

Epic

Work Item

Spike

Work Item

Task

Work Item

User Story

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

quality/os-autoinst-distri-fedora!522

No description provided.