Replace boot command parser with PEG parser. #6129

mwhooker · 2018-04-12T17:45:47Z

This PR replaces the custom boot command parser with a PEG parser (https://github.com/mna/pigeon). This should make sure our parsing logic is correct and is consistent across the builders.

Closes #6034
Closes #2855

blocks on hashicorp/middleman-hashicorp#58

Still need to

mwhooker · 2018-04-12T23:42:25Z

parallels done but it's very slow. May need to add the optimization I talk about in the comments

mwhooker · 2018-04-13T06:07:18Z

#2855 talks about virtualbox not working when under heavy load because it sends one key to each invocation of vboxmanage. Updated code to group codes properly

mwhooker · 2018-04-13T19:30:32Z

I think this is in a state where the checked off builders can start being tested. I'd like to solicit community help here, since i want to test a wide range of boot commands. I've been testing with the boxes on https://www.packer.io/community-tools.html#templates and they're working, but I don't think that's enough.

@mcandre, I would greatly appreciate your help testing this patch if you feel obliged. I know you've been doing a lot of work in this area.

mwhooker · 2018-04-14T02:57:27Z

wondering if I can break any of this with unicode. The CharToggle expression might have some weird edge cases

rickard-von-essen

Some basic testing with Bento and everything seems fine, but the VNC driver seems like 3x slower than the PC-XT driver?

I think there is room for improvement on speed. My gut feeling is that the Parallels builder was faster before this change.

rickard-von-essen · 2018-04-14T14:08:24Z

common/boot_command/boot_command_ast.go

+
+/*
+TODO:
+	* pc-at abstraction


yeah missed this in a2cc17f, but since this is a todo comment I'll let it fall off when it gets done

rickard-von-essen · 2018-04-14T14:17:30Z

common/boot_command/boot_command_ast_test.go

+
+func Test_parse(t *testing.T) {
+	in := "<wait><wait20><wait3s><wait4m2ns>"
+	in += "foo/bar > one 界"


界 is it correct to handle UTF characters? To my knowledge there are no scan codes for non US character. All special character (such as the Swedish ÅÄÖ) are handled in a software key maps i.e. the OS translate the key code for the characters on that position on an US keyboard into the current used language (for the previous mentioned characters left curly bracket, double quote, and colon respectively).

yeah this is a good point. I don't know that actually sending a unicode key will work or make sense. I'm mostly doing this here to make sure I'm decoding utf8 correctly.

rickard-von-essen · 2018-04-14T14:25:06Z

website/source/partials/builders/_boot-command.html.md

+
+-   `<waitXX>` - Add an arbitrary pause before sending any additional keys. The
+    format of `XX` is a possibly signed sequence of decimal numbers, each with
+    optional fraction and a unit suffix, such as `300ms`, `-1.5h` or `2h45m`.


What does <wait-1.5h> do?

copied and pasted this from the go docs and missed that. for sure we should only accept positive values.

rickard-von-essen · 2018-04-14T14:26:15Z

website/source/partials/builders/_boot-command.html.md

+    that is started serving the directory specified by the `http_directory`
+    configuration parameter. If `http_directory` isn't specified, these will be
+    blank!
+*   `Name` - The name of the VM.


mwhooker · 2018-04-14T19:58:32Z

Some basic testing with Bento and everything seems fine, but the VNC driver seems like 3x slower than the PC-XT driver?

This is a limitation of the VNC client and I think was the behavior before this patch. It unfortunately sends one key event at a time, waiting up to keyInterval in between.

with patch:

2018/04/15 21:53:23 ui: ==> ubuntu-1604-vmware: Typing the boot command over VNC...
2018/04/15 21:54:58 packer: 2018/04/15 21:54:58 [INFO] Waiting for SSH, up to timeout: 20m0s

1m35s

without patch:

2018/04/15 21:57:11 ui: ==> ubuntu-1604-vmware: Typing the boot command over VNC...
2018/04/15 21:58:42 ui: ==> ubuntu-1604-vmware: Waiting for SSH to become available...

1m31s

so a little slower, and perhaps within the margin of error. I think one reason why is that the existing code does not wait keyInterval before sending the shift key, whereas this patch waits after every key press.

I think there is room for improvement on speed. My gut feeling is that the Parallels builder was faster before this change.

I ran the below config with and without this patch applied. I just used the logs so I only had 1-second accuracy. Packer with this patch applied took 1 second longer to go from Typing the boot command... to Waiting for SSH. I assume this is because I added the keyInterval wait to the pc-xt driver, since at least with virtualbox I was getting odd errors that made me think it hadn't processed the previous commands before new ones were sent.

I suppose one improvement that could be made is to join the boot command array into a single string. That way the driver can decide how many keys to send at one time. I already do this in the hyper-v builder.

 {
      "name": "{{ user `build_name` }}-parallels",
      "vm_name": "{{ user `build_name` }}-parallels",
      "boot_command": [
        "<enter><f6><esc><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs><bs>",
        "preseed/url=http://{{ .HTTPIP }}:{{ .HTTPPort }}/preseed.cfg ",
        "debian-installer=en_US auto locale=en_US kbd-chooser/method=us ",
        "hostname={{ user `build_name` }}-vmware ",
        "fb=false debconf/fronten=d=noninteractive ",
        "keyboard-configuration/modelcode=SKIP keyboard-configuration/layout=USA keyboard-configuration/variant=USA console-setup/ask_detect=false ",
        "initrd=/install/initrd.gz -- <enter>"
      ],
      "boot_wait": "5s",
...

mwhooker · 2018-04-19T00:58:48Z

I think this is code complete. now just on to testing

Replaces #5989

mwhooker · 2018-04-20T00:06:02Z

check that the vbox key changes from #6067 work on parallels and hyper-v

turns out this wasn't working. basically all the 2 byte scan codes weren't working for at least parallels and probably hyper-v

fixed in 7e9af82

ghost · 2020-03-29T02:21:52Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

mwhooker added enhancement builder/virtualbox builder/vmware core Core components of Packer builder/qemu builder/parallels builder/hyperv labels Apr 12, 2018

mwhooker requested a review from a team as a code owner April 12, 2018 17:45

mwhooker mentioned this pull request Apr 12, 2018

homoginize and dry up boot command code #6034

Closed

mwhooker force-pushed the bc_replace branch from ebe323d to 83e5a2a Compare April 12, 2018 23:10

mwhooker requested a review from taliesins as a code owner April 13, 2018 19:50

mwhooker force-pushed the bc_replace branch from 0d0a0f2 to 498ff42 Compare April 13, 2018 22:08

mwhooker mentioned this pull request Apr 14, 2018

boot_command syntax for sending custom key codes #5634

Open

rickard-von-essen reviewed Apr 14, 2018

View reviewed changes

mwhooker force-pushed the bc_replace branch 2 times, most recently from 6e92906 to e75a965 Compare April 18, 2018 20:25

mwhooker mentioned this pull request Apr 18, 2018

Mention network config for HTTPIP template variable #6159

Closed

mwhooker force-pushed the bc_replace branch 2 times, most recently from d3bf90a to 2ff3881 Compare April 18, 2018 23:31

mwhooker mentioned this pull request Apr 18, 2018

Add menu key #5989

Closed

mwhooker force-pushed the bc_replace branch 4 times, most recently from 7a77ebd to 284de35 Compare April 19, 2018 00:58

mwhooker added 16 commits April 19, 2018 14:26

fix formatting

9a839c4

Stop sending boot commands when context canceled.

add7e8a

negative wait WIP

9a8acbb

remove nonsensical negative waits.

84894ca

flatten boot command config and implement for vmware

408eba8

Implement boot config struct for parallels

e662927

fix boot command config struct name

7990966

s/boot_command/bootcommand/

c629997

implement config struct for qemu

c8e76ce

implement boot config struct for virtualbox

a0c9ddb

implement boot config struct for hyperv

1d36ef0

sort keycode lists

e9e2794

add menu key.

7d43324

Replaces #5989

Perform validation of boot command.

aa69bdf

fix vnc config

df6224d

remove outdated comment

31d04f1

mwhooker force-pushed the bc_replace branch from 0aaceb7 to 31d04f1 Compare April 19, 2018 21:26

mwhooker added 4 commits April 19, 2018 16:09

better log message

374d8f0

remove unnecessary log line.

11de2b9

flush scancodes when we wait

675eae1

fix 2 byte scancodes for pc-xt VMs.

7e9af82

fix test

b5a97e4

mwhooker changed the title ~~[WIP]Replace boot command parser with PEG parser.~~ Replace boot command parser with PEG parser. Apr 20, 2018

mwhooker merged commit 024fac4 into master Apr 20, 2018

mwhooker deleted the bc_replace branch April 20, 2018 19:21

DanHam mentioned this pull request Apr 21, 2018

Fix error with VMware vmx builder at compacting stage #6074

Merged

mwhooker mentioned this pull request May 1, 2018

boot_wait ignored in 1.2.3 for QEMU provider #6225

Closed

ghost locked and limited conversation to collaborators Mar 29, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace boot command parser with PEG parser. #6129

Replace boot command parser with PEG parser. #6129

mwhooker commented Apr 12, 2018 •

edited

Loading

mwhooker commented Apr 12, 2018

mwhooker commented Apr 13, 2018

mwhooker commented Apr 13, 2018

mwhooker commented Apr 14, 2018

rickard-von-essen left a comment

rickard-von-essen Apr 14, 2018

mwhooker Apr 14, 2018

rickard-von-essen Apr 14, 2018

mwhooker Apr 14, 2018

rickard-von-essen Apr 14, 2018

mwhooker Apr 14, 2018

rickard-von-essen Apr 14, 2018

mwhooker commented Apr 14, 2018 •

edited

Loading

mwhooker commented Apr 19, 2018

mwhooker commented Apr 20, 2018

ghost commented Mar 29, 2020

Replace boot command parser with PEG parser. #6129

Replace boot command parser with PEG parser. #6129

Conversation

mwhooker commented Apr 12, 2018 • edited Loading

mwhooker commented Apr 12, 2018

mwhooker commented Apr 13, 2018

mwhooker commented Apr 13, 2018

mwhooker commented Apr 14, 2018

rickard-von-essen left a comment

Choose a reason for hiding this comment

rickard-von-essen Apr 14, 2018

Choose a reason for hiding this comment

mwhooker Apr 14, 2018

Choose a reason for hiding this comment

rickard-von-essen Apr 14, 2018

Choose a reason for hiding this comment

mwhooker Apr 14, 2018

Choose a reason for hiding this comment

rickard-von-essen Apr 14, 2018

Choose a reason for hiding this comment

mwhooker Apr 14, 2018

Choose a reason for hiding this comment

rickard-von-essen Apr 14, 2018

Choose a reason for hiding this comment

mwhooker commented Apr 14, 2018 • edited Loading

mwhooker commented Apr 19, 2018

mwhooker commented Apr 20, 2018

ghost commented Mar 29, 2020

mwhooker commented Apr 12, 2018 •

edited

Loading

mwhooker commented Apr 14, 2018 •

edited

Loading