boxcutter / windows

Virtual machine templates for Windows written in legacy JSON and Batch Scripting/JScript
Apache License 2.0
753 stars 266 forks source link

Not using Makefile causes errors due to incorrectly formatted `shutdown` comment string and other things #255

Closed dragon788 closed 3 years ago

dragon788 commented 3 years ago

It appears that the iso_url, iso_checksum, and shutdown_command and several other things (CM variable) are overridden from the templates defaults by the Makefile, meaning there are possibly a lot of partially invalid templates that can't actually be built directly with packer build --only virtualbox-iso eval-win10x64-enterprise.json.

It is non-intuitive that a user can't just look at the json template, they also have to dive into the Makefile and figure out which environment variables correspond to options, if they even realize that they can do overrides with environment variables. The use of environment variables makes sense if you are always running in a CI pipeline where they can persist between runs and be logged as part of the build, but if you only build a box occasionally or are building something locally it is time lost looking up the differences from the actual json pretty much every time.

The json templates contains the iso_url and iso_checksum and some sane defaults for headless and update (but sadly not cm which is set to chef which is a great tool but fails to install sometimes) but the fact that the Makefile overrides any changes a user might make to these core settings in the template json seems bad and has caught me out several times when trying to make changes.

It appears the root of issue described in the subject here is that the version of the shutdown_command in the templates has a space in the /c shutdown comment, where as you can see below the make executed version uses an override to swaps in an underscore to eliminate the space and removes the need for the quotes.

While debugging this I also discovered much to my dismay that if not using make the default CM option coming from the template files is chef and due to some infrastructure quirks the omnitruck site throws a 500 error causing builds to fail hard unless explicitly setting -var 'cm=nocm' when calling packer build.

Assembled command from make virtualbox/eval-win10x64-enterprise

packer build -on-error=cleanup -only=virtualbox-iso -var 'cm=nocm' -var 'version=1.0.4' -var 'update=false' -var 'headless=false' -var "shutdown_command=shutdown /s /t 10 /f /d p:4:1 /c Packer_Shutdown" -var "iso_url=https://long.url.string/file.iso" -var "iso_checksum=aCheckSum" eval-win10x64-enterprise.json

Many templates have it using single quotes

shutdown /s /t 10 /f /d p:4:1 /c 'Packer Shutdown'

A couple have escaped double quotes

shutdown /s /t 10 /f /d p:4:1 /c \"Packer Shutdown\"

Example of error I was getting

==> virtualbox-iso: Gracefully halting virtual machine...
    virtualbox-iso: Usage: shutdown [/i | /l | /s | /sg | /r | /g | /a | /p | /h | /e | /o] [/hybrid] [/soft] [/fw] [/f]
    virtualbox-iso:     [/m \\computer][/t xxx][/d [p|u:]xx:yy [/c "comment"]]
    virtualbox-iso:
    virtualbox-iso:     No args    Display help. This is the same as typing /?.
    virtualbox-iso:     /?         Display help. This is the same as not typing any options.
    virtualbox-iso:     /i         Display the graphical user interface (GUI).
    virtualbox-iso:                This must be the first option.
    virtualbox-iso:     /l         Log off. This cannot be used with /m or /d options.
    virtualbox-iso:     /s         Shutdown the computer.
    virtualbox-iso:     /sg        Shutdown the computer. On the next boot, if Automatic Restart Sign-On
    virtualbox-iso:                is enabled, automatically sign in and lock last interactive user.
    virtualbox-iso:                After sign in, restart any registered applications.
    virtualbox-iso:     /r         Full shutdown and restart the computer.
    virtualbox-iso:     /g         Full shutdown and restart the computer. After the system is rebooted,
    virtualbox-iso:                if Automatic Restart Sign-On is enabled, automatically sign in and
    virtualbox-iso:                lock last interactive user.
    virtualbox-iso:                After sign in, restart any registered applications.
    virtualbox-iso:     /a         Abort a system shutdown.
    virtualbox-iso:                This can only be used during the time-out period.
    virtualbox-iso:                Combine with /fw to clear any pending boots to firmware.
    virtualbox-iso:     /p         Turn off the local computer with no time-out or warning.
    virtualbox-iso:                Can be used with /d and /f options.
    virtualbox-iso:     /h         Hibernate the local computer.
    virtualbox-iso:                Can be used with the /f option.
    virtualbox-iso:     /hybrid    Performs a shutdown of the computer and prepares it for fast startup.
    virtualbox-iso:                Must be used with /s option.
    virtualbox-iso:     /fw        Combine with a shutdown option to cause the next boot to go to the
    virtualbox-iso:                firmware user interface.
    virtualbox-iso:     /e         Document the reason for an unexpected shutdown of a computer.
    virtualbox-iso:     /o         Go to the advanced boot options menu and restart the computer.
    virtualbox-iso:                Must be used with /r option.
    virtualbox-iso:     /m \\computer Specify the target computer.
    virtualbox-iso:     /t xxx     Set the time-out period before shutdown to xxx seconds.
    virtualbox-iso:                The valid range is 0-315360000 (10 years), with a default of 30.
    virtualbox-iso:                If the timeout period is greater than 0, the /f parameter is
    virtualbox-iso:                implied.
    virtualbox-iso:     /c "comment" Comment on the reason for the restart or shutdown.
    virtualbox-iso:                Maximum of 512 characters allowed.
    virtualbox-iso:     /f         Force running applications to close without forewarning users.
    virtualbox-iso:                The /f parameter is implied when a value greater than 0 is
    virtualbox-iso:                specified for the /t parameter.
    virtualbox-iso:     /d [p|u:]xx:yy  Provide the reason for the restart or shutdown.
    virtualbox-iso:                p indicates that the restart or shutdown is planned.
    virtualbox-iso:                u indicates that the reason is user defined.
    virtualbox-iso:                If neither p nor u is specified the restart or shutdown is
    virtualbox-iso:                unplanned.
    virtualbox-iso:                xx is the major reason number (positive integer less than 256).
    virtualbox-iso:                yy is the minor reason number (positive integer less than 65536).
    virtualbox-iso:
    virtualbox-iso: Reasons on this computer:
    virtualbox-iso: (E = Expected U = Unexpected P = planned, C = customer defined)
    virtualbox-iso: Type        Major   Minor   Title
    virtualbox-iso:
    virtualbox-iso:  U          0       0       Other (Unplanned)
    virtualbox-iso: E           0       0       Other (Unplanned)
    virtualbox-iso: E P         0       0       Other (Planned)
    virtualbox-iso:  U          0       5       Other Failure: System Unresponsive
    virtualbox-iso: E           1       1       Hardware: Maintenance (Unplanned)
    virtualbox-iso: E P         1       1       Hardware: Maintenance (Planned)
    virtualbox-iso: E           1       2       Hardware: Installation (Unplanned)
    virtualbox-iso: E P         1       2       Hardware: Installation (Planned)
    virtualbox-iso: E           2       2       Operating System: Recovery (Unplanned)
    virtualbox-iso: E P         2       2       Operating System: Recovery (Planned)
    virtualbox-iso:   P         2       3       Operating System: Upgrade (Planned)
    virtualbox-iso: E           2       4       Operating System: Reconfiguration (Unplanned)
    virtualbox-iso: E P         2       4       Operating System: Reconfiguration (Planned)
    virtualbox-iso:   P         2       16      Operating System: Service pack (Planned)
    virtualbox-iso:             2       17      Operating System: Hot fix (Unplanned)
    virtualbox-iso:   P         2       17      Operating System: Hot fix (Planned)
    virtualbox-iso:             2       18      Operating System: Security fix (Unplanned)
    virtualbox-iso:   P         2       18      Operating System: Security fix (Planned)
    virtualbox-iso: E           4       1       Application: Maintenance (Unplanned)
    virtualbox-iso: E P         4       1       Application: Maintenance (Planned)
    virtualbox-iso: E P         4       2       Application: Installation (Planned)
    virtualbox-iso: E           4       5       Application: Unresponsive
    virtualbox-iso: E           4       6       Application: Unstable
    virtualbox-iso:  U          5       15      System Failure: Stop error
    virtualbox-iso:  U          5       19      Security issue (Unplanned)
    virtualbox-iso: E           5       19      Security issue (Unplanned)
    virtualbox-iso: E P         5       19      Security issue (Planned)
    virtualbox-iso: E           5       20      Loss of network connectivity (Unplanned)
    virtualbox-iso:  U          6       11      Power Failure: Cord Unplugged
    virtualbox-iso:  U          6       12      Power Failure: Environment
    virtualbox-iso:   P         7       0       Legacy API shutdown
==> virtualbox-iso: Timeout while waiting for machine to shutdown.
arizvisa commented 3 years ago

Yea, I definitely don't use "chef" and set it to nocm by default. Actually, I'd like to default to nocm tbh...but since that's what the original maintainers have set, I don't think I'm allowed to change it.

My build-system that's based on this is doing the following using the default shutdown w/o issues, so in my testing this should 100% should work without the Makefile because I don't use it at all (it's garbage imo).

packer build -only 'vmware-iso' -var 'headless=false' -var-file 'build/boxcutter-vars.json' -var-file '/home/user/audit/nitro/lol/build.dir/boxcutter-eval-win10x86-enterprise-iso-vars.json' -var 'box_directory=/home/user/audit/nitro/lol/build.dir/boxcutter-' 'build/boxcutter-windows/eval-win10x86-enterprise.json'

with boxcutter-vars.json being set to:

{
  "cpus": "1",
  "disk_size": "102400",
  "memory": "4096",
  "cm": "nocm",
  "cm_version": "",
  "update": "false"
}

Nonetheless definitely let me know if you encounter any other inconsistency issues. I'm glad someone else is interested in helping maintain this! :)

arizvisa commented 3 years ago

Also please create an issue for those chef unreliability issues that you mentioned, and I'll see what I can do.

PR #229 sort of affects the reliability issues w/ script/cmtool.bat due to the author ensuring all downloads in that script actually use the _download.cmd script (as currently it's using powershell), but the contributor seems to have dropped it. Really all of this repo's downloads should be funneled through the download script so that all available methods are attempted, but depending on the issue it might be instead worth refactoring script/cmtool.batso that it cycles through only once the download succeeds...

dragon788 commented 3 years ago

The chef issue probably isn't fixable via code changes. It is a 500 error from their omnitruck site due to being connected to a VPN into an AWS account and the request coming from that AWS account rather than the normal internet facing gateway that all their stuff is probably expecting and being tested for.

arizvisa commented 3 years ago

Got it.

Thx for your contribution, good sir.

dragon788 commented 3 years ago

@tas50 It just occurred to me that the issue I'm seeing with pulling chef might be related to IPv6 being enabled in our environments. I'd run into this in the past and ended up working around it by explicitly disabling IPv6 on the node when we were pulling the bootstrap but in the current case If I want the built box to support IPv6 after it's been created.

https://github.com/chef/chef/issues/5186

Is there a bootstrap URL that the scripts could hit to pull down a script to install chef at a specific version or with specific options?