Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Duplicated code between step_run and step_create_linux_vm #127

Closed
torarnv opened this issue Mar 10, 2024 · 3 comments · Fixed by #139
Closed

Duplicated code between step_run and step_create_linux_vm #127

torarnv opened this issue Mar 10, 2024 · 3 comments · Fixed by #139
Assignees
Labels
enhancement New feature or request

Comments

@torarnv
Copy link
Contributor

torarnv commented Mar 10, 2024

Looks like the runInstaller part of step_create_linux_vm is duplicating code from step_run. I can clean this up, but want to verify if there are other plans in this area before I dig in. Thanks :)

@fkorotkov
Copy link
Contributor

Indeed! It seems we can reuse code from step_run and just handle FromISO as well. Just add

for _, iso := range config.FromISO {
  runArgs = append(runArgs, fmt.Sprintf("--disk=%s:ro", iso))
}

to step_run and I think we'll be good to go.

A PR will be much appreciated but if you don't have time we can also take it from you.

@fkorotkov fkorotkov added the enhancement New feature or request label Mar 10, 2024
@torarnv
Copy link
Contributor Author

torarnv commented Mar 10, 2024

Cool, thanks! I'll have a look tomorrow then :)

@torarnv
Copy link
Contributor Author

torarnv commented Mar 10, 2024

Doesn't look like I can assign issues, but consider the issue "assigned" 😄

torarnv added a commit to torarnv/packer-plugin-tart that referenced this issue Mar 30, 2024
The initial `tart create` logic could be shared between macOS
and Linux in step_create.

We still have dedicated Linux installation logic as part of the
creation step, which duplicates logic in step_run for macOS, so
this is only a first step towards cleaning this up.

See cirruslabs#127
torarnv added a commit to torarnv/packer-plugin-tart that referenced this issue Apr 1, 2024
The initial `tart create` logic could be shared between macOS
and Linux in step_create.

We still have dedicated Linux installation logic as part of the
creation step, which duplicates logic in step_run for macOS, so
this is only a first step towards cleaning this up.

See cirruslabs#127
torarnv added a commit to torarnv/packer-plugin-tart that referenced this issue Apr 1, 2024
The initial `tart create` logic could be shared between macOS
and Linux in step_create.

We still have dedicated Linux installation logic as part of the
creation step, which duplicates logic in step_run for macOS, so
this is only a first step towards cleaning this up.

See cirruslabs#127
torarnv added a commit to torarnv/packer-plugin-tart that referenced this issue Apr 1, 2024
The initial `tart create` logic could be shared between macOS
and Linux in step_create.

We still have dedicated Linux installation logic as part of the
creation step, which duplicates logic in step_run for macOS, so
this is only a first step towards cleaning this up.

See cirruslabs#127
torarnv added a commit to torarnv/packer-plugin-tart that referenced this issue Apr 1, 2024
The initial `tart create` logic could be shared between macOS
and Linux in step_create.

We still have dedicated Linux installation logic as part of the
creation step, which duplicates logic in step_run for macOS, so
this is only a first step towards cleaning this up.

See cirruslabs#127
torarnv added a commit to torarnv/packer-plugin-tart that referenced this issue Apr 1, 2024
The initial `tart create` logic could be shared between macOS
and Linux in step_create.

We still have dedicated Linux installation logic as part of the
creation step, which duplicates logic in step_run for macOS, so
this is only a first step towards cleaning this up.

See cirruslabs#127
fkorotkov pushed a commit that referenced this issue Apr 3, 2024
* Update Ubuntu 22.04 example ISO URL

* Update examples plugin minimum version

* Share VM creation logic of macOS and Linux VMs in step_create

The initial `tart create` logic could be shared between macOS
and Linux in step_create.

We still have dedicated Linux installation logic as part of the
creation step, which duplicates logic in step_run for macOS, so
this is only a first step towards cleaning this up.

See #127

* Use same steps for Linux installation as for macOS installation

The code for Linux installation was maintained separately in
stepCreateVM, even though it shared most of the logic with
the macOS installation flow of stepRun.

We now use the same flow for Linux and macOS installation:

 1. The VM is created in stepCreateVM
 2. We run the VM in stepRun, which based on the
    `boot_command` does automated installation of
    the OS
 3. Once the automated installation is done, and SSH
    access is enabled, SSH provisioning steps can
    proceed

To achieve this flow for Linux, the autoinstall cloud
config in the Ubuntu example had to be changed to from
"poweroff" to "reboot", so that the VM was rebooted into
a level where SSH is available. This is the default value
for the autoinstall shutdown key, and the only reason we
needed to override it previously was because we did the
installation (via boot_command) as part of stepCreateVM.

The SSH timeout in the example also had to be increased,
as we now wait for the installation to be completed,
just like we for macOS.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants