Initial check in for Icinga2 Provider/Resource #8306

lrsmith · 2016-08-19T00:57:46Z

Initial check in for review and comment for an Icinga2 Provider. New to GO and test all comments/critiques appreciated.

Need to add one more resource for Icinga2 Service Checks.
Need to address disabling SSL VERIFY option. disabled for now due to self signed cert.

…S Certs will skip verification. Controllable via provider or env variable being set

jtopjian · 2016-08-31T14:52:51Z

Icinga is something I have on my list of things to explore, so this is interesting.

Ideally, I think all of the Icinga interactions would be wrapped up into a separate Go Icinga client library, so in the Terraform provider, you'd just end up doing things like:

checkResult, err := icinga.CreateCheck(...)

As well, all of the JSON bodies could be wrapped into some Go structs and you could have Go do most of the JSON building for you.

There are a few providers in Terraform that you can use as examples. The first one I found was Grafana. Note how the Grafana interactions are in a separate library and how that library structures the data.

Just some thoughts! I think this would be awesome to have in Terraform and if I had more time, I'd love to jump in and help 😄

stack72 · 2016-10-26T15:16:43Z

Hi @lrsmith

Any thoughts on how this PR is progressing? Is this something that you are going to continue working on?

Paul

lrsmith · 2016-10-26T15:57:46Z

@stack72

Paul

I'm refactoring based on some feedback to make it more like the grafana and rundeck providers. So I've been working on https://github.com/lrsmith/go-icinga2-api and I will modify the icinga2 provider to use that.

I am continuing to work on it just got pulled away for a bit. I should be back to it his week and have something for review soon.

stack72 · 2016-10-26T18:44:28Z

Very nice - look forward to trying it out. Please let me know when you need another review / testing

Paul

…alidates basic host creation and deletion

stack72 · 2016-12-01T13:25:23Z

Hi @lrsmith

Just checking back in on this :)

Paul

lrsmith · 2016-12-03T20:05:22Z

I am finishing up the services code and once I commit that should be ready for review and comments. I'm shooting for next week to commit the code.

lrsmith · 2016-12-06T04:11:52Z

Ready for initial review

stack72

Hi @lrsmith

Thanks for the work here - as you can see, from the first 2 resources, there are similar comments in place. If you can fix those up across the entire set of resources that would be excellent

Also, the PR needs documentation - right now, users won't understand what is required and what is not

Thanks

Paul

stack72 · 2016-12-06T12:32:04Z

builtin/providers/icinga2/provider.go

+				Type:        schema.TypeBool,
+				Optional:    true,
+				DefaultFunc: EnvBoolDefaultFunc("ICINGA2_INSECURE_SKIP_TLS_VERIFY", false),
+				Description: descriptions["insecure"],


I can't see "insecure" in the descriptions map?

stack72 · 2016-12-06T12:33:21Z

builtin/providers/icinga2/provider.go

+		d.Get("insecure").(bool),
+	)
+
+	return config, nil


Are there any conditions we can actually throw an error when creating a new Client?

Still need to look at this one.

stack72 · 2016-12-06T12:33:34Z

builtin/providers/icinga2/provider.go

+func init() {
+	descriptions = map[string]string{
+		"api_url":      "The address of the Icinga2 server.\n",
+		"api_user":     "The user to authenticate to the Iccinga2 Server as.\n",


Typo in Iccinga

stack72 · 2016-12-06T12:34:38Z

builtin/providers/icinga2/provider_test.go

+		t.Fatal("ICINGA2_API_PASSWORD must be set for acceptance tests")
+	}
+
+	v = os.Getenv("ICINGA2_INSECURE_SKIP_TLS_VERIFY")


Is this 100% required? It defaults to false right?

stack72 · 2016-12-06T12:34:49Z

builtin/providers/icinga2/resource_icinga2_checkcommand.go

+				Type:     schema.TypeString,
+				Required: true,
+				ForceNew: true,
+				//Elem:     &schema.Schema{Type: schema.TypeString},


Cleanup old comments please

stack72 · 2016-12-06T12:38:02Z

builtin/providers/icinga2/resource_icinga2_checkcommand.go

+	return nil
+}
+
+func resourceIcinga2CheckcommandUpdate(d *schema.ResourceData, meta interface{}) error {


IF we don't need an Update, then we can omit this

stack72 · 2016-12-06T12:38:19Z

builtin/providers/icinga2/resource_icinga2_checkcommand.go

+
+	name := d.Get("name").(string)
+
+	return client.DeleteCheckcommand(name)


does this not return any errors if failure?

DeleteCheckCommand will return an error or nil, so I was just passing that back up.

stack72 · 2016-12-06T12:40:04Z

builtin/providers/icinga2/resource_icinga2_host.go

+
+	for _, host := range hosts {
+		if host.Name == hostname {
+			d.SetId(hostname)


We should error out if we don't find our host

Should be addressed

See CheckCommand suggestion on this

stack72 · 2016-12-06T12:40:19Z

builtin/providers/icinga2/resource_icinga2_host.go

+		}
+	}
+
+	return nil


Create -> Read func

stack72 · 2016-12-06T12:40:33Z

builtin/providers/icinga2/resource_icinga2_host.go

+	return nil
+}
+
+func resourceIcinga2HostUpdate(d *schema.ResourceData, meta interface{}) error {


Omit if not required

…shicorp

lrsmith · 2016-12-07T01:21:12Z

Went through and made changes on other resource per comments

stack72

Hi @lrsmith

I have made 1 last comment and then I think we are good to go! I am just going to run the tests against the docker icinga2 container to make sure

Paul

stack72 · 2016-12-08T10:38:40Z

builtin/providers/icinga2/resource_icinga2_checkcommand.go

+
+	for _, checkcommand := range checkcommands {
+		if checkcommand.Name == name {
+			d.SetId(name)


I think maybe a better way to handle this would be to do something like this

found := false for _, checkcommand := range checkcommands { if checkcommand.Name == name { d.SetId(name) found = true } } if !found { return fmt.Errorf("Failed to create Checkcommand %s : %s", name, err) }

stack72 · 2016-12-08T10:39:03Z

builtin/providers/icinga2/resource_icinga2_host.go

+
+	for _, host := range hosts {
+		if host.Name == hostname {
+			d.SetId(hostname)


See CheckCommand suggestion on this

stack72 · 2016-12-08T10:39:22Z

builtin/providers/icinga2/resource_icinga2_hostgroup.go

+		return err
+	}
+
+	for _, hostgroup := range hostgroups {


See CheckCommand for how I think this should be addressed

stack72 · 2016-12-08T10:39:37Z

builtin/providers/icinga2/resource_icinga2_service.go

+		return err
+	}
+
+	for _, service := range services {


see CheckCommand for how I think this should be addressed

stack72 · 2016-12-08T12:01:08Z

Hi @lrsmith

I just ran the tests and got a panic. I set up the tests as follows:

docker run -d -ti --name icinga2-api -p 4080:80 -p 4665:5665 icinga/icinga2
export ICINGA2_API_URL=https://192.168.99.100:4665
export ICINGA2_API_USER=root
export ICINGA2_API_PASSWORD=icinga

Then the command make testacc TEST=./builtin/providers/icinga2 meant this happened:

% make testacc TEST=./builtin/providers/icinga2                                   ✭
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/12/08 12:59:03 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/icinga2 -v  -timeout 120m
=== RUN   TestProvider
--- PASS: TestProvider (0.00s)
=== RUN   TestProviderImpl
--- PASS: TestProviderImpl (0.00s)
=== RUN   TestAccCreateCheckcommand
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x48 pc=0x1b1df6]

goroutine 208 [running]:
panic(0x4f2460, 0xc4200101c0)
	/usr/local/Cellar/go/1.7.3/libexec/src/runtime/panic.go:500 +0x1a1
github.com/hashicorp/terraform/vendor/github.com/lrsmith/go-icinga2-api/iapi.(*Server).NewAPIRequest(0xc4201de900, 0x582997, 0x3, 0xc4201a1800, 0x34, 0xc4202c88c0, 0xc3, 0x126, 0x0, 0x0, ...)
	/Users/stacko/Code/go/src/github.com/hashicorp/terraform/vendor/github.com/lrsmith/go-icinga2-api/iapi/client.go:99 +0x3e6
github.com/hashicorp/terraform/vendor/github.com/lrsmith/go-icinga2-api/iapi.(*Server).CreateCheckcommand(0xc4201de900, 0xc4200f8b21, 0x1d, 0xc4200f8b61, 0x1c, 0xc4202b3260, 0xc4201fd000, 0xc4201fd000, 0xc420186820, 0xdbdd1, ...)
	/Users/stacko/Code/go/src/github.com/hashicorp/terraform/vendor/github.com/lrsmith/go-icinga2-api/iapi/checkcommands.go:55 +0x29a
github.com/hashicorp/terraform/builtin/providers/icinga2.resourceIcinga2CheckcommandCreate(0xc42019cf00, 0x561200, 0xc4201de900, 0x800, 0xd394)
	/Users/stacko/Code/go/src/github.com/hashicorp/terraform/builtin/providers/icinga2/resource_icinga2_checkcommand.go:56 +0x32c
github.com/hashicorp/terraform/helper/schema.(*Resource).Apply(0xc420018de0, 0xc4201aa870, 0xc42019e640, 0x561200, 0xc4201de900, 0xc4201a1701, 0x3a, 0x0)
	/Users/stacko/Code/go/src/github.com/hashicorp/terraform/helper/schema/resource.go:162 +0x30e
github.com/hashicorp/terraform/helper/schema.(*Provider).Apply(0xc420018ea0, 0xc420196460, 0xc4201aa870, 0xc42019e640, 0x1, 0xc420243708, 0x9b702)
	/Users/stacko/Code/go/src/github.com/hashicorp/terraform/helper/schema/provider.go:212 +0x9b
github.com/hashicorp/terraform/terraform.(*EvalApply).Eval(0xc4201df540, 0x7d8580, 0xc42024b520, 0x2, 0x2, 0x58302d, 0x4)
	/Users/stacko/Code/go/src/github.com/hashicorp/terraform/terraform/eval_apply.go:67 +0x34b
github.com/hashicorp/terraform/terraform.EvalRaw(0x7cd8a0, 0xc4201df540, 0x7d8580, 0xc42024b520, 0x541d60, 0x0, 0x0, 0x0)
	/Users/stacko/Code/go/src/github.com/hashicorp/terraform/terraform/eval.go:53 +0x17f
github.com/hashicorp/terraform/terraform.(*EvalSequence).Eval(0xc420292b80, 0x7d8580, 0xc42024b520, 0x2, 0x2, 0x58302d, 0x4)
	/Users/stacko/Code/go/src/github.com/hashicorp/terraform/terraform/eval_sequence.go:10 +0x6c
github.com/hashicorp/terraform/terraform.EvalRaw(0x7ce260, 0xc420292b80, 0x7d8580, 0xc42024b520, 0x7ce260, 0xc420292b80, 0xc420292ca0, 0x1a)
	/Users/stacko/Code/go/src/github.com/hashicorp/terraform/terraform/eval.go:53 +0x17f
github.com/hashicorp/terraform/terraform.Eval(0x7ce260, 0xc420292b80, 0x7d8580, 0xc42024b520, 0xc420292b80, 0x7ce260, 0xc420292b80, 0x0)
	/Users/stacko/Code/go/src/github.com/hashicorp/terraform/terraform/eval.go:34 +0x53
github.com/hashicorp/terraform/terraform.(*Graph).walk.func1(0x563e00, 0xc42002d998, 0x0, 0x0)
	/Users/stacko/Code/go/src/github.com/hashicorp/terraform/terraform/graph.go:301 +0xe7c
github.com/hashicorp/terraform/dag.(*AcyclicGraph).Walk.func3(0xc420265400, 0xc42023abe0, 0xc420265410, 0xc4202555f0, 0xc420265420, 0x563e00, 0xc42002d998, 0xc420239e00, 0xc420239f20)
	/Users/stacko/Code/go/src/github.com/hashicorp/terraform/dag/dag.go:246 +0x251
created by github.com/hashicorp/terraform/dag.(*AcyclicGraph).Walk
	/Users/stacko/Code/go/src/github.com/hashicorp/terraform/dag/dag.go:255 +0x6a6
exit status 2
FAIL	github.com/hashicorp/terraform/builtin/providers/icinga2	0.113s
make: *** [testacc] Error 1

lrsmith · 2016-12-08T22:36:46Z

Paul, thanks I will take a look.

I definitely need to put some more code around catching this and I'll add that shortly. The API_URL is missing a bit, it needs "/v1" on the end. My original intent was to require it on that line since it looks like in was there to allow for v2 of the api etc. Thinking about it now, I will look at making the a variable that can be over-written and will default to v1. I'll also work on the changes above. Update PR coming soon.

I pulled down the docker container and it worked with it set to
export ICINGA2_API_URL=https://127.0.0.1:4665/v1

Looks like the 'default' hosts in the docker image are different then the vagrant too, so some of the acceptance tests will fail. I'll use the docker container and adjust and add the in too.

Thanks
Len

lrsmith · 2016-12-10T05:15:23Z

Paul,
I added code to verify the URL and handle the case where the server is unavailable. I also modified the code to test if the resource was found when iterating through the list. I also added a timeout and test cases in the underlying go-icinga2-api code.

Set a bad port on server, via ENV variable

--- FAIL: TestAccCreateCheckcommand (0.00s)
testing.go:265: Step 0 error: Error refreshing: 1 error(s) occurred:

	* Get https://127.0.0.1:4666/v1: dial tcp 127.0.0.1:4666: getsockopt: connection refused

Forget to add /v1 or invalid URL path to ENV variable

--- FAIL: TestAccCreateCheckcommand (0.00s)
testing.go:265: Step 0 error: Error refreshing: 1 error(s) occurred:

	* Error : Invalid API version  specified. Only v1 is currently supported.

or
* Error : Invalid API version /v2 specified. Only v1 is currently supported.

Server is down

--- FAIL: TestAccCreateCheckcommand (60.01s)
testing.go:265: Step 0 error: Error refreshing: 1 error(s) occurred:

	* Get https://127.0.0.2:4665/v1: net/http: request canceled while waiting for connection (Client.Timeout exceeded while awaiting headers)

stack72 · 2016-12-12T15:24:41Z

Hi @lrsmith

Thanks for making the changes here - do you know of a docker container I can use to test this now? When i run it against the official container, I get the following:

% make testacc TEST=./builtin/providers/icinga2
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/12/12 15:19:44 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/icinga2 -v  -timeout 120m
=== RUN   TestProvider
--- PASS: TestProvider (0.00s)
=== RUN   TestProviderImpl
--- PASS: TestProviderImpl (0.00s)
=== RUN   TestAccCreateCheckcommand
--- FAIL: TestAccCreateCheckcommand (0.02s)
	testing.go:265: Step 0 error: Error refreshing: 1 error(s) occurred:

		* Error : Invalid API version  specified. Only v1 is currently supported.
=== RUN   TestAccCreateBasicHost
--- FAIL: TestAccCreateBasicHost (0.02s)
	testing.go:265: Step 0 error: Error refreshing: 1 error(s) occurred:

		* Error : Invalid API version  specified. Only v1 is currently supported.
=== RUN   TestAccCreateVariableHost
--- FAIL: TestAccCreateVariableHost (0.02s)
	testing.go:265: Step 0 error: Error refreshing: 1 error(s) occurred:

		* Error : Invalid API version  specified. Only v1 is currently supported.
=== RUN   TestAccCreateBasicHostGroup
--- FAIL: TestAccCreateBasicHostGroup (0.02s)
	testing.go:265: Step 0 error: Error refreshing: 1 error(s) occurred:

		* Error : Invalid API version  specified. Only v1 is currently supported.
=== RUN   TestAccCreateService
--- FAIL: TestAccCreateService (0.02s)
	testing.go:265: Step 0 error: Error refreshing: 1 error(s) occurred:

		* Error : Invalid API version  specified. Only v1 is currently supported.
FAIL
exit status 1
FAIL	github.com/hashicorp/terraform/builtin/providers/icinga2	0.108s
make: *** [testacc] Error 1

stack72 · 2016-12-12T15:28:15Z

Ignore that comment - I should have added /v1 to the end of the URL!

This LGTM now :)

% make testacc TEST=./builtin/providers/icinga2
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/12/12 15:27:32 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/icinga2 -v  -timeout 120m
=== RUN   TestProvider
--- PASS: TestProvider (0.00s)
=== RUN   TestProviderImpl
--- PASS: TestProviderImpl (0.00s)
=== RUN   TestAccCreateCheckcommand
--- PASS: TestAccCreateCheckcommand (0.19s)
=== RUN   TestAccCreateBasicHost
--- PASS: TestAccCreateBasicHost (0.42s)
=== RUN   TestAccCreateVariableHost
--- PASS: TestAccCreateVariableHost (0.46s)
=== RUN   TestAccCreateBasicHostGroup
--- PASS: TestAccCreateBasicHostGroup (0.25s)
=== RUN   TestAccCreateService
--- PASS: TestAccCreateService (0.43s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/icinga2	1.760s

Thanks for all the work here

Paul

jtopjian · 2016-12-12T16:34:43Z

Cool!

@lrsmith very nice work!

ghost · 2020-04-18T02:34:30Z

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.

Initial checkin for PR request

197b9d6

radeksimko added enhancement new-provider labels Aug 19, 2016

Added an argument to provider to allow control over whether or not TL…

5bacacc

…S Certs will skip verification. Controllable via provider or env variable being set

stack72 added the waiting-response An issue/pull request is waiting for a response from the community label Oct 26, 2016

stack72 removed the waiting-response An issue/pull request is waiting for a response from the community label Oct 26, 2016

lrsmith added 12 commits October 31, 2016 21:51

Merge branch 'master' into icinga2

28f921b

Initial check-in to use refactored module

0ca432b

Checkin of very MVP for creating/deleting host test which works and v…

8e3c344

…alidates basic host creation and deletion

Check in with support for creating hosts with variables working

19aafbd

Checking in work to date

9afb8a3

Remove code that causes travis CI to fail while I debug

db683ab

Adjust create to accept multivale

ed00007

Back on track. Working basic tests. go-icinga2-api needs more test too

a2d04f8

Squashing

cbc9e45

Back on track. Working basic tests. go-icinga2-api needs more test too

9b8faaf

Merge branch 'icinga2' of github.com:lrsmith/terraform into icinga2

5291662

Check in refactored hostgroup support

cb97063

lrsmith added 3 commits December 4, 2016 16:48

Check in refactored check_command, hosts, and hsotgroup with a few test

4b573ad

Checking in service code

732e83f

Add in dependency for icinga2 provider

daa7471

lrsmith changed the title ~~[WIP] Initial check in for Icinga2 Provider/Resource~~ Initial check in for Icinga2 Provider/Resource Dec 6, 2016

stack72 suggested changes Dec 6, 2016

View reviewed changes

stack72 added the waiting-response An issue/pull request is waiting for a response from the community label Dec 6, 2016

Add documentation. Refactor, fix and extend based on feedback from Ha…

7c4101d

…shicorp

stack72 suggested changes Dec 8, 2016

View reviewed changes

Added checking and validation around invalid URL and unavailable server

faee2b9

stack72 approved these changes Dec 12, 2016

View reviewed changes

stack72 merged commit 015e96d into hashicorp:master Dec 12, 2016

ghost locked and limited conversation to collaborators Apr 18, 2020


		name := d.Get("name").(string)

		return client.DeleteCheckcommand(name)

Initial check in for Icinga2 Provider/Resource #8306

Initial check in for Icinga2 Provider/Resource #8306

Conversation

lrsmith commented Aug 19, 2016 • edited Loading

jtopjian commented Aug 31, 2016

stack72 commented Oct 26, 2016

lrsmith commented Oct 26, 2016 • edited Loading

stack72 commented Oct 26, 2016

stack72 commented Dec 1, 2016

lrsmith commented Dec 3, 2016

lrsmith commented Dec 6, 2016

stack72 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lrsmith commented Dec 7, 2016

stack72 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stack72 commented Dec 8, 2016

lrsmith commented Dec 8, 2016 • edited Loading

lrsmith commented Dec 10, 2016 • edited Loading

Set a bad port on server, via ENV variable

Forget to add /v1 or invalid URL path to ENV variable

Server is down

stack72 commented Dec 12, 2016

stack72 commented Dec 12, 2016

jtopjian commented Dec 12, 2016

ghost commented Apr 18, 2020

lrsmith commented Aug 19, 2016 •

edited

Loading

lrsmith commented Oct 26, 2016 •

edited

Loading

lrsmith commented Dec 8, 2016 •

edited

Loading

lrsmith commented Dec 10, 2016 •

edited

Loading