Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignition provider #6189

Merged
merged 17 commits into from
Jan 3, 2017
Merged

Ignition provider #6189

merged 17 commits into from
Jan 3, 2017

Conversation

mcuadros
Copy link
Contributor

@mcuadros mcuadros commented Apr 15, 2016

This is a WIP version of the ignition provider, the reason behind opening of a WIP feature is just because I am looking for validate the usage of the provider.

Ignition counts with a lot of "resources": disk, partitions, users, groups, networks, unit from systemd, etc. The chance to need to build different ignition configs, reusing "resources", is very high, this is why I am implementing this "resources" as real terraform resources.

A small example of the suggested usage:

resource "ignition_config" "test" {
    users = [
        "${ignition_user.foo.id}",
        "${ignition_user.qux.id}",
    ]
}

resource "ignition_user" "foo" {
    name = "foo"
    password_hash = "foo"
}

resource "ignition_user" "qux" {
    name = "qux"
    password_hash = "qux"
}

The resources are cached by id internally and the ignition_config resource builds the Config object and generates the JSON in the rendered containing the ignition generated file.

New resources:

  • ignition_config
  • ignition_disk
  • ignition_raid
  • ignition_filesystem
  • ignition_file
  • ignition_systemd_unit
  • ignition_networkd_unit
  • ignition_user
  • ignition_group

@mcuadros mcuadros changed the title Ignition provider [WIP] Ignition provider Apr 15, 2016
@jen20
Copy link
Contributor

jen20 commented Apr 15, 2016

Hi @mcuadros! This looks like a great start from a quick scan over the code - we can definitely review more thoroughly once you designate it no longer WIP. In order to help us understand the trajectory, it might be helpful to know what else you plan on adding? Thanks for opening a pull request!

@mcuadros
Copy link
Contributor Author

@jen20 I am planing to give full support. About the method I am using to link the resources... is this ok?

@mcuadros mcuadros changed the title [WIP] Ignition provider Ignition provider Apr 17, 2016
@mcuadros
Copy link
Contributor Author

This provider is completed and documented. /cc @jen20

@mcuadros
Copy link
Contributor Author

@jen20 some news?

@mcuadros
Copy link
Contributor Author

mcuadros commented Jun 9, 2016

ping @jen20

@rdeusser
Copy link
Contributor

It's been a few months now. Any movement on this?

@mcuadros
Copy link
Contributor Author

mcuadros commented Aug 1, 2016

@jen20 is complete with tests, since months.

@adamcstephens
Copy link

What's the status of this?

@jen20
Copy link
Contributor

jen20 commented Jan 3, 2017

Hi @mcuadros! Sorry for not responding to this sooner, I no longer work full-time on Terraform. An initial scan of this looks good to me. Since it is using the Go library for generating ignition config I think the testing strategy is fine. There are a few predictable conflicts from this having been left for so long, but these are easily resolved from our side. Perhaps @stack72 can pick this up, it looks like a reasonably quick win to merge.

@mcuadros
Copy link
Contributor Author

mcuadros commented Jan 3, 2017

I just did the rebase, will be good having this merged, since ignition is suggested way to provision CoreOS and cloud-config has being discontinued.

@stack72
Copy link
Contributor

stack72 commented Jan 3, 2017

Hi @mcuadros

thanks for the rebase here - just tried to run the tests and they failed - thoughts?

% make testacc TEST=./builtin/providers/ignition
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/01/03 08:32:48 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/ignition -v  -timeout 120m
=== RUN   TestProvider
--- PASS: TestProvider (0.00s)
=== RUN   TestValidateUnit
--- PASS: TestValidateUnit (0.00s)
=== RUN   TestIngnitionFileReplace
--- FAIL: TestIngnitionFileReplace (0.02s)
	testing.go:265: Step 0 error: Check failed: json: cannot unmarshal string into Go value of type types.Config
=== RUN   TestIngnitionFileAppend
--- FAIL: TestIngnitionFileAppend (0.02s)
	testing.go:265: Step 0 error: Check failed: json: cannot unmarshal string into Go value of type types.Config
=== RUN   TestIngnitionDisk
--- FAIL: TestIngnitionDisk (0.02s)
	testing.go:265: Step 0 error: Check failed: json: cannot unmarshal string into Go value of type types.Config
=== RUN   TestIngnitionFile
--- FAIL: TestIngnitionFile (0.03s)
	testing.go:265: Step 0 error: Check failed: json: cannot unmarshal string into Go value of type types.Config
=== RUN   TestIngnitionFilesystem
--- FAIL: TestIngnitionFilesystem (0.04s)
	testing.go:265: Step 0 error: Check failed: json: cannot unmarshal string into Go value of type types.Config
=== RUN   TestIngnitionGroup
--- FAIL: TestIngnitionGroup (0.03s)
	testing.go:265: Step 0 error: Check failed: json: cannot unmarshal string into Go value of type types.Config
=== RUN   TestIngnitionNetworkdUnit
--- FAIL: TestIngnitionNetworkdUnit (0.02s)
	testing.go:265: Step 0 error: Check failed: json: cannot unmarshal string into Go value of type types.Config
=== RUN   TestIngnitionRaid
--- FAIL: TestIngnitionRaid (0.03s)
	testing.go:265: Step 0 error: Check failed: json: cannot unmarshal string into Go value of type types.Config
=== RUN   TestIngnitionSystemdUnit
--- FAIL: TestIngnitionSystemdUnit (0.04s)
	testing.go:265: Step 0 error: Check failed: json: cannot unmarshal string into Go value of type types.Config
=== RUN   TestIngnitionUser
--- FAIL: TestIngnitionUser (0.04s)
	testing.go:265: Step 0 error: Check failed: json: cannot unmarshal string into Go value of type types.Config
FAIL
exit status 1
FAIL	github.com/hashicorp/terraform/builtin/providers/ignition	0.316s
make: *** [testacc] Error 1

Paul

@stack72
Copy link
Contributor

stack72 commented Jan 3, 2017

Hi @mcuadros
The issue here is that the vendoring isn't correct. I jsut resolved that test issue by running this command:

govendor fetch github.com/coreos/ignition/...

This then creates a new error:

% make testacc TEST=./builtin/providers/ignition                                                                      130 ↵ ✹ ✭
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/01/03 09:08:41 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/ignition -v  -timeout 120m
# github.com/hashicorp/terraform/builtin/providers/ignition
builtin/providers/ignition/resource_ignition_filesystem.go:109: cannot use types.Path(d.Get("path").(string)) (type types.Path) as type *types.Path in field value
builtin/providers/ignition/resource_ignition_filesystem_test.go:56: invalid operation: f.Path != "/foo" (mismatched types *types.Path and string)
FAIL	github.com/hashicorp/terraform/builtin/providers/ignition [build failed]
make: *** [testacc] Error 2

@stack72 stack72 self-assigned this Jan 3, 2017
@mcuadros
Copy link
Contributor Author

mcuadros commented Jan 3, 2017

Hi, @stack72

You found you problem the first want was when you updated the ignition library to the latest version, a inner type has change from a string to a types.Path being imposible to compile.

And the original error was a small problem, retrieving the module output, I was using OutputState.String instead of using the OutputState.Value

All test passing now.

@stack72
Copy link
Contributor

stack72 commented Jan 3, 2017

Fantastic :) This works now as expected

% make testacc TEST=./builtin/providers/ignition                                                                          130 ↵
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/01/03 11:27:14 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/ignition -v  -timeout 120m
=== RUN   TestProvider
--- PASS: TestProvider (0.00s)
=== RUN   TestValidateUnit
--- PASS: TestValidateUnit (0.00s)
=== RUN   TestIngnitionFileReplace
--- PASS: TestIngnitionFileReplace (0.02s)
=== RUN   TestIngnitionFileAppend
--- PASS: TestIngnitionFileAppend (0.02s)
=== RUN   TestIngnitionDisk
--- PASS: TestIngnitionDisk (0.02s)
=== RUN   TestIngnitionFile
--- PASS: TestIngnitionFile (0.03s)
=== RUN   TestIngnitionFilesystem
--- PASS: TestIngnitionFilesystem (0.04s)
=== RUN   TestIngnitionGroup
--- PASS: TestIngnitionGroup (0.03s)
=== RUN   TestIngnitionNetworkdUnit
--- PASS: TestIngnitionNetworkdUnit (0.02s)
=== RUN   TestIngnitionRaid
--- PASS: TestIngnitionRaid (0.02s)
=== RUN   TestIngnitionSystemdUnit
--- PASS: TestIngnitionSystemdUnit (0.04s)
=== RUN   TestIngnitionUser
--- PASS: TestIngnitionUser (0.04s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/ignition	0.307s

@stack72
Copy link
Contributor

stack72 commented Jan 3, 2017

Thanks for all of the work and the patience here!

@stack72 stack72 merged commit 85f0fba into hashicorp:master Jan 3, 2017
@mcuadros
Copy link
Contributor Author

mcuadros commented Jan 3, 2017

Awesome! 🎉
You know when will be the next release?

@stack72
Copy link
Contributor

stack72 commented Jan 3, 2017

The xmas vacation ended this morning for us at HashiCorp so it will be in the next week I'd take a guess at

@jwieringa
Copy link
Contributor

@mcuadros @stack72 It was fantastic to find the Ignition provider in Terraform, thank you!

@ghost
Copy link

ghost commented Apr 15, 2020

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.

@ghost ghost locked and limited conversation to collaborators Apr 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants