added TINKERBELL_SKIP_NETWORKING to ./generate_env.sh

[JamesPGriffith]

Description

The allows vagrant up provisioner to complete.

Why is this needed

Without this change, the provisioner VM fails to complete.

Fixes: #91

How Has This Been Tested?

Executing vagrant up provisioner with the variable as an empty string:

[[TRUNCATED]]
    provisioner: ++ export TINKERBELL_SKIP_NETWORKING=
    provisioner: ++ TINKERBELL_SKIP_NETWORKING=
    provisioner: + make_certs_writable
[[TRUNCATED]]
    provisioner: ++ export TINKERBELL_SKIP_NETWORKING=
    provisioner: ++ TINKERBELL_SKIP_NETWORKING=
    provisioner: + [[ -z '' ]]
    provisioner: + setup_networking ubuntu 18.04
[[TRUNCATED]]

Removed the .env file and populate the variable string in the ./generate-env.sh and rebuilt:

[[TRUNCATED]
    provisioner: ++ export TINKERBELL_SKIP_NETWORKING=true
    provisioner: ++ TINKERBELL_SKIP_NETWORKING=true
[[TRUNCATED]
    provisioner: + [[ -z true ]]
    # IT WAS SKIPPED
    provisioner: + setup_osie
[[TRUNCATED]

This failed with running in Vagrant as expected but provides the skip behavior desired.

Note: I am not able to test this on bare-metal for which the option was originally created.

How are existing users impacted? What migration steps/scripts do we need?

Existing users are likely not impacted unless the .env was removed and recreated after #88 was merged. I was pondering whether or not the .generate_env.sh file should remove and recreate the .env file on each execution.

Checklist:

I have:

• [ No ] updated the documentation and/or roadmap (if required)
  However, the new option appears to need to be added to the 'On Bare Metal with Docker' section of the documentation.
• [ No ] added unit or e2e tests
  I honestly don't know enough Go to contribute a test for Vagrant completion.
• [ No ] provided instructions on how to upgrade
  There's nothing to upgrade beyond a git pull.

[markjacksonfishing]

/lgtm
Holding for another maintainer to review.

[markjacksonfishing]

Once review is complete, please remove do not merge label

[dadrus]

Is it really the way to go? I mean the generate_env.sh is supposed to be used for local docker setup as well. According to my understanding, the given change might break that. What about just adding the required variable to the Vagrant file like this?

provisioner.vm.provision :shell,
                        path: './scripts/tinkerbell.sh',
                        env: {
                          'TINKERBELL_CONFIGURE_NAT': configure_nat,
                          'TINKERBELL_SKIP_NETWORKING': ""
                        }

The change is then local to the vagrant setup only.

[damdo]

What's you opinion on this last comment Marky? :)

[tstromberg]

PR looks great. Can you merge this up to master and run shfmt on generate_env.sh? The linter shows a diff.

[jacobweinstock]

Hey @JamesPGriffith, #90 just landed and has changed the sandbox significantly. Would you mind having a look at the changes and confirm if this change is still something you'd like to see?

[splaspood]

In light of @jacobweinstock 's comment and its age I'm going to move that we close this PR without merge at this time.

[jacobweinstock]

Please do re-open this if necessary.