WIP add option to pass user_data url

[hardys]

Currently the installer passes a pointer/wrapper config which
contains a URL, this presents a problem when the target nodes
don't have networking up at the (early) stage of running ignition
in the ramdisk.

Instead, we can download the entire config, which it appears we
can pass to ironic since the instance_info is now defined as
longtext (which on mariadb is 4GB), this is a much higher limit
than the majority of other platforms which do require the pointer
config due to user_data size limits.

[hardys]

/cc @stbenjam @kirankt - interested to get your thoughts on this approach as we discussed it recently

It's untested but based on some hacky test code I pushed to https://github.com/hardys/dev-scripts/blob/get_go/get.go (that reads from the tfvars, we can probably do the equivalent thing inside the installer to pass these two new values).

[stbenjam]

I'd like to see the cert made optional. In the case of OpenShift, I think by the time the Ironic API is available bootstrap should be already serving the ignition but I'm not sure it's guaranteed - what do you think about having retry logic?

[stbenjam]

Test failures are legit. Imports are missing, and some minor lint things.

[hardys]

Test failures are legit. Imports are missing, and some minor lint things.

Ack I think I fixed them all now, thanks - I did notice some seemingly unrelated errors though:

ironic/resource_ironic_node_v1_test.go:17:26: undefined: testAccPreCheck
ironic/resource_ironic_node_v1_test.go:18:17: undefined: testAccProviders
ironic/resource_ironic_node_v1_test.go:106:18: undefined: testAccProvider
ironic/resource_ironic_node_v1_test.go:132:17: undefined: testAccProvider
FAIL	github.com/openshift-metal3/terraform-provider-ironic/ironic [build failed]
make: *** [Makefile:30: test] Error 2

[hardys]

Ok so I discovered make test doesn't work locally unless you set the same env vars as travis, and run the ironic services on the host - we should perhaps automate that or make the tests fail more clearly when run in a non-CI setup.

[kirankt]

Isn't the scope of this userData local to the if block. So in the end, aren't we still passing the stub userData rather than the one fetched in the if block?

[hardys]

Yes well spotted - I found the same issue running the tests locally, will fix 👍

[kirankt]

Another thought. Why can't we fetch the URL, CA cert from the stub ignition and directly create a config drive like @hardys' example here: https://github.com/hardys/dev-scripts/blob/get_go/get.go . Instead of creating two new TF vars, we can still keep user_data and change the provider's logic to send full ignition.

e.g. https://github.com/kirankt/terraform-provider-ironic/blob/full-ignition/ironic/resource_ironic_deployment.go

[stbenjam]

Another thought. Why can't we fetch the URL, CA cert from the stub ignition and directly create a config drive like @hardys' example here: hardys/dev-scripts:get.go@get_go . Instead of creating two new TF vars, we can still keep user_data and change the provider's logic to send full ignition.

e.g. kirankt/terraform-provider-ironic:ironic/resource_ironic_deployment.go@full-ignition

IMHO, I really don't want RHCOS or OpenShift-specific things in terraform-provider-ironic, it's a generic provider for Ironic usable by anyone. Having a variable for specifying a user data URL is more generic.

[hardys]

Another thought. Why can't we fetch the URL, CA cert from the stub ignition and directly create a config drive like @hardys' example here: https://github.com/hardys/dev-scripts/blob/get_go/get.go . Instead of creating two new TF vars, we can still keep user_data and change the provider's logic to send full ignition.

e.g. https://github.com/kirankt/terraform-provider-ironic/blob/full-ignition/ironic/resource_ironic_deployment.go

Yeah I did consider this, but decided against it for the reasons already mentioned by @stbenjam - I ran out of time last week to work on an installer PR, but I think that is the place to prepare the data needed by this PR, since the pointer/wrapper config is really an implementation detail specific to the installer.

[hardys]

This was included in #38