Add custom boot command that waits for text to appear #178
Conversation
A custom VNC driver is added, that will be the basis of further improvements to the boot command logic. For now it provides the ability to request and wait for a frame, which we use to ensure that the VM is ready before starting to type boot commands.
Results in much faster detection of new frames from the VNC server.
The syntax is supported without having to modify the packer-sdk bootcommand grammar by a simple regex replacement that uses two symbols from the Unicode Private Use Area as markers, combined with a custom VNC driver that handles the markers in SendKey(). The actual recognition of the text is handled by Vision.framework. To aid debugging the frames we analyze are written out as PNG files if packer is run with -debug.
|
Missing docs for new wait command, will add after initial feature review |
|
This allows e.g: diff --git a/macos/macos.pkrtpl.yaml b/macos/macos.pkrtpl.yaml
index bed5e73..305532e 100644
--- a/macos/macos.pkrtpl.yaml
+++ b/macos/macos.pkrtpl.yaml
@@ -1,14 +1,8 @@
-wait_for_boot:
- # Wait for VM too boot
- # FIXME: Ideally there would be some way to wait for the first
- # non-black frame from the VNC server.
- - <wait60s>
-
install_macos:
- # Hello, Hola, Bonjour, etc.
+ - <wait "Get Started">
- <spacebar>
- - <wait5s>
+ - <wait "Language"> |
|
Is the builder not a macOS machine? |
|
Hi @torarnv 👋 This is a pretty cool use of Apple's Vision framework. Have you considered submitting this to packer or packer-plugin-sdk directly? This way:
|
|
Thanks @edigaryev! I did initially considered building this feature into packer-plugin-sdk's An upstream feature in packer-plugin-sdk would make sense, but one challenge there is that Using github.com/otiai10/gosseract would be an option, but would add more dependencies compared to the fairly straight forward use of the built in Vision.framework. One possible upstream improvement would be to teach I'm still testing this feature on my nightly builds, so we can hold on on merging for now. Let me know if you have other concerns though! |
| @@ -133,140 +126,6 @@ func (s *stepRun) Cleanup(state multistep.StateBag) { | |||
| _, _ = cmd.Process.Wait() | |||
| } | |||
|
|
|||
| func typeBootCommandOverVNC( | |||
There was a problem hiding this comment.
Would it be possible to avoid moving this function and reduce the number changes made to this file?
This would greatly simplify the review process as it'd be clear what was actually changed.
There was a problem hiding this comment.
The first commit in the series (f7dd2bf) just moves the code (sans some minor language changes to the log messages), so the commits on top of that change all represent the actual changes.
I can remove the language changes if you want, so that the commit is a pure move of the function?
There was a problem hiding this comment.
Ie, the change (PR) is split into atomic commits, so it helps to review them individually, rather than the total change (https://github.com/cirruslabs/packer-plugin-tart/pull/178/files).
There was a problem hiding this comment.
I can remove the language changes if you want, so that the commit is a pure move of the function?
Since we squash all changes anyways I think it'd be better if you could just avoid moving this function altogether.
And these "some minor language changes" that were made as a part of moving the function would be instantly visible, without the need to sift through commits and/or handle force pushes.
There was a problem hiding this comment.
Sure, if that's how you prefer to review I can surely adopt that approach 😃
There was a problem hiding this comment.
To clarify, do you want me to still add the new logic to vnc.go, but keep typeBootCommandOverVNC in step_run.go, or to add all the new logic to step_run.go without any new file?
There was a problem hiding this comment.
To clarify, do you want me to still add the new logic to
vnc.go, but keeptypeBootCommandOverVNCinstep_run.go, or to add all the new logic tostep_run.gowithout any new file?
That's up to you 🙌
The goal I'm pursuing here is to make the cumulative diff (which will also be an atomic commit later) easier to understand.
There was a problem hiding this comment.
👍🏻
I'm investigating some details in how this behaves before pushing a new PR version. For some reason bootcommand sometimes feeds us keys with keycode 0 that doesn't match whatever the input text was. As this might result in looking for the wrong text I need to figure out what's going wrong.
As this is coming from bootcommand's grammar, I don't suppose you've seen similar issues where a boot command button press is somehow "lost"?
There was a problem hiding this comment.
As this is coming from
bootcommand's grammar, I don't suppose you've seen similar issues where a boot command button press is somehow "lost"?
No, I don't think I've encountered this while developing our boot_command.
Using
<wait "string">or<wait "(some|regex|here)">allows installing macOS without having to specify hard-coded timeout waits such as<wait10s>.We now also wait for the first frame, if
boot_waitis not specified.