provider/cloudstack: add project support for CloudStack Instances

[jalemieux]

Add the ability to specify the project an instance should belong to. This is contingent on https://github.com/xanzy/go-cloudstack/pull/21/files getting merged.

[svanharmelen]

@jalemieux I just merged PR xanzy/go-cloudstack#21 so we're good to move forward with this one.

I'll do a review in just a bit, but noticed you only added a parameter project_name to the cloudstack_instance resource. I think if you would really want to work with the CloudStack provider and projects, we should add a (preferably called just project) parameter to all resources.

Otherwise when you create a VPC/network and want to create an instance in there, it will not be possible as the VPC/network will not be part of the project. Not sure how CS will handle that, but I would assume that you would want all needed resources to be created within the same project right?

But no need to do everything at once of course, so we can just start with this first resource...

[svanharmelen]

Just a small nit, please add an empty line between parameter declarations and I would prefer to keep the name a little shorter to just project...

[svanharmelen]

@jalemieux please see the inline comments for some pointers and suggestions. Also I'm missing a line like setValueOrUUID(d, "project", vm.Project, vm.Projectid) in the resourceCloudStackInstanceRead function so the value is updated when we do a terraform refresh.

So there's still some work to do before we can merge this in, but I really like to direction you are going by adding project support to the CloudStack provider. So thanks for opening the PR and let's make sure we get this feature in!

[jalemieux]

@svanharmelen thanks for the feedback!
I think I have most your suggestions in. I wasn't able to change the project of a VM, so ForceNew makes sense. Also Added project support in retrieveUUID, which makes sense too.
Did a few successful round of testing, but noticed some weirdness once when removing the project from a resource in my test .tf. I wasn't able to reproduce more than once however.
Maybe you can give me some hints on what happens when the engine realize a previous configuration is missing, couldn't find anything in helper/schema that seemed helpful for this.

[svanharmelen]

@jalemieux nice updates, looks good! Think we should probably add a test for this as well (or extend one of the existing tests).

What happens (or should happen) if TF notices a configuration option is no longer set, is that it will check the new config against your (refreshed) state and if there is a diff it should (when running apply) try to update whatever is needed to get in sync again.

So in this case if the instance in member of a project and you remove the project attribute from your config, I think it should notice that and recreate the instance outside a project...

[jalemieux]

@svanharmelen thanks, I will work on the tests.

Regarding the removal of project attribute, what you described is in line with my expectation. However, what I see is different. For example, I have project = "terraform" in my .tf, for a given cloustack_instance resource. If i removed project = "terraform" from the .tf, and rerun plan i will get the expected

~ cloudstack_instance.tf4
    project: "terraform" => ""`

However, applying the plan doesn't seem to have any effect. After applying, rerunning plan will produce the same output:

~ cloudstack_instance.tf4
    project: "terraform" => ""`

I figured other providers must run into similar corner cases. There ought to be something that in TF looks for removed attributes and recreate resources if necessary. Or something in the attribute schema itself, that, like Forcenew, forces the resource recreation when the attribute is removed (vs changed).

[svanharmelen]

Thanks for the clear explanation. I'll have a closer look at this one to see if I can see why this happens.

[jalemieux]

@svanharmelen i added a test for create instance with a project's name and project's id. I tested the former here, but can't test the latter. Worked well with project name.

[svanharmelen]

Typo breaking Travis...

[jalemieux]

@svanharmelen please review. Tested both cases.

[svanharmelen]

LGTM!

[ghost]

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.