Fetch machine details #45

virajindasrao · 2018-04-17T10:14:13Z

This code will allow a user to update terraform configuration at deployment level after deployment succeed. To get field values of deployment, a user has to keep resource_configuration value equals to an empty string.
After request success, details will be fetched on terraform refresh.

Signed-off-by: Viraj Indasrao [email protected]

raghavav

It would be really useful to have an example blueprint in this review. It would also be good to add comments in the code about the structure of the template retrieved from the catalog item. This would make it much easier to read the review and to check for assumptions.

g1ps · 2018-05-18T06:36:44Z

I grabbed actions.go and resource.go from Viraj's pull request and re-ran dep, re-compiled, re-installed, re-ran init and apply but show doesn't return anything different. The example files aren't necessary for compilation, are they? What have I missed?

Thanks.

Update: I added the examples for completeness. No change.

Update 2: I pulled the entire branch and built that. No change. It doesn't work at all, even when providing the empty ip_address var I later found in an example file.

g1ps · 2018-05-23T08:48:02Z

Any help here?? How do I get this working?

Thanks.

raghavav · 2018-05-23T17:48:24Z

Viraj is on a personal time off for this week. He should be able to get to this shortly. I have not had a chance to play with this pull request yet. I will start trying this change (as well as reviewing it) shortly.
No the examples are not necessary for functionality.

g1ps · 2018-05-24T06:39:49Z

Thank you, @raghavav.

DanielVaknin · 2018-05-29T13:56:44Z

Is there an update on this PR?

Thanks.

virajindasrao · 2018-05-29T14:55:19Z

Working on it.

raghavav · 2018-06-02T18:50:21Z

vrealize/resource.go

-
-	"encoding/json"
-
-	"github.com/hashicorp/terraform/helper/schema"
 )

 //ResourceViewsTemplate - is used to store information
 //related to resource template information.
 type ResourceViewsTemplate struct {


Rename this type. It is not a 'template'.
It is just 'Resource' or 'ResourceView'.

i know the same change is happening in PR #27. I think it is worth pulling that rename change here.

raghavav · 2018-06-02T21:05:16Z

vrealize/resource.go

-					usedConfigKeys = append(usedConfigKeys, configKey)
-				} else {
-					log.Printf("%s was not replaced", configKey)
+		if resourceConfiguration[configKey] != nil && resourceConfiguration[configKey] != "" {


Not worth the additional indent here.
Just use

if resourceConfiguration[configKey] == nil || resourceConfiguration[configKey] == "" {
break;
}

Better yet, create an 'isEmpty(string) boolean utility function. There you can also check for other types of empty strings. That function will likely need to be reused in other places too.

raghavav · 2018-06-02T22:53:16Z

vrealize/resource.go

@@ -394,6 +396,135 @@ func updateResource(d *schema.ResourceData, meta interface{}) error {
 	return nil
 }

+func fetchResourceFieldsValues(d *schema.ResourceData, meta interface{}) error {
+	//Get requester machine ID from schema.dataresource
+	requestMachineID := d.Id()


Rename field to 'catalogRequestId'.

raghavav · 2018-06-02T22:57:48Z

vrealize/resource.go

+	//Get client handle
+	client := meta.(*APIClient)
+	//Fetch resource details from vRA
+	templateResources, errTemplate := client.GetResourceViews(requestMachineID)


not 'remplateResources'.
Call it 'resourceView'

raghavav · 2018-06-03T17:55:38Z

vrealize/resource.go

+		"{com.vmware.csp.component.iaas.proxy.provider@resource.action.name.machine.Reconfigure}"
+
+	//Read vRA resource content one by one
+	for item := range templateResources.Content {


for index, component := range templateResources.Content

raghavav · 2018-06-04T23:31:23Z

vrealize/resource.go

@@ -394,6 +396,135 @@ func updateResource(d *schema.ResourceData, meta interface{}) error {
 	return nil
 }

+func fetchResourceFieldsValues(d *schema.ResourceData, meta interface{}) error {
+	//Get requester machine ID from schema.dataresource


PR #27 has an implementation of this same method. Can you please rationalize the two PRs? I am not sure which one you want to go forward with.

I will put in some review comments here, but I think the two look different and you have to decide which way you want to go. In my opinion it is best to go with the version in PR #27.

raghavav · 2018-06-04T23:35:19Z

vrealize/resource.go

+			//Iterate over TF resource > resource_configuration
+			for configKey := range resourceConfiguration {
+				//Check if any TF resource > resource_configuration is empty or null
+				if resourceConfiguration[configKey] == nil || resourceConfiguration[configKey] == "" {


There is clearly an opportunity here to use that 'isEmpty(string) boolean' utility function that I mentioned earlier in this review.

raghavav · 2018-06-04T23:36:06Z

vrealize/resource.go

+				//Check if any TF resource > resource_configuration is empty or null
+				if resourceConfiguration[configKey] == nil || resourceConfiguration[configKey] == "" {
+					//Compare vRA child resource and TF resource > resource_configuration names
+					if strings.Contains(configKey, componentName+".") {


Should be using HasPrefix and TrimPrefix functions as mentioned in the reviews in PR #55

raghavav · 2018-06-04T23:37:43Z

vrealize/resource.go

+						if returnValue != nil {
+							//If value returned from getTemplateValue is in float64 format then
+							//convert it in string format
+							if reflect.ValueOf(returnValue).Kind() == reflect.Float64 {


As mentioned in the review in PR #27, please fold all this type conversions directly into the getTemplateValue function. That function can return a string instead of returning an interface{}.
Also, I like the idea of that function telling you if a value has changed.

raghavav · 2018-06-04T23:39:06Z

vrealize/resource.go

+		//If value type is map then set recursive call which will fiend field in one level down of map interface
+		if reflect.ValueOf(templateInterface[i]).Kind() == reflect.Map {
+			template, _ := templateInterface[i].(map[string]interface{})
+			returnValue := getTemplateValue(template, field)


As I said in the review for PR #27, I am not sure what the use-case is for this kind of a recursive retrieval from multiple levels of a map. I think it is not necessary or even a good idea.

Signed-off-by: Viraj Indasrao <[email protected]>

got vet Signed-off-by: Viraj Indasrao <[email protected]>

Signed-off-by: Viraj Indasrao <[email protected]>

Minor fixes Signed-off-by: Viraj Indasrao <[email protected]>

Signed-off-by: Viraj Indasrao <[email protected]>

vmwclabot · 2018-06-14T12:50:40Z

@virajindasrao, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding Signed-off-by: John Doe <[email protected]> to the last line of each Git commit message. The e-mail address used to sign must match the e-mail address of the Git author. Click here to view the Developer Certificate of Origin agreement.

Commit 2e463f4 must be signed by [email protected]

Signed-off-by: Viraj Indasrao <[email protected]>

got vet Signed-off-by: Viraj Indasrao <[email protected]>

Signed-off-by: Viraj Indasrao <[email protected]>

Minor fixes Signed-off-by: Viraj Indasrao <[email protected]>

Signed-off-by: Viraj Indasrao <[email protected]>

vmwclabot · 2018-06-25T06:46:51Z

@virajindasrao, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding Signed-off-by: John Doe <[email protected]> to the last line of each Git commit message. The e-mail address used to sign must match the e-mail address of the Git author. Click here to view the Developer Certificate of Origin agreement.

Commit 6dc3544 must be signed by [email protected]

…-details Signed-off-by: Viraj Indasrao <[email protected]>

Signed-off-by: Viraj Indasrao <[email protected]>

g1ps · 2018-07-05T12:38:13Z

This now works for me using the master branch, passing the empty ip_address var in resource_configuration and setting the variable.

output "ip" {
  value = "${vra7_resource.machine2.resource_configuration.RHEL7.4.ip_address}"
}
...
resource "vra7_resource" "machine2" {
  catalog_name = "Red Hat Enterprise Linux 7.4"

  resource_configuration = {
    RHEL7.4.cpu         = "2"
    RHEL7.4.memory      = "4096"
    RHEL7.4.description = "test"
    RHEL7.4.ip_address  = ""
  }
...

Thanks.

UPDATE: Spoke too soon. It worked once. See #17. 😦

Signed-off-by: Viraj Indasrao <[email protected]>

nferro · 2019-03-15T15:14:40Z

vrealize/resource.go

@@ -274,12 +275,17 @@ func createResource(d *schema.ResourceData, meta interface{}) error {
 	//Update request template field values with values from user configuration.
 	resourceConfiguration, _ := d.Get("resource_configuration").(map[string]interface{})
 	for configKey, configValue := range resourceConfiguration {
+		if len(configValue.(string)) == 0 {
+			break


This should be a continue instead of break.

virajindasrao requested a review from markpeek April 17, 2018 10:14

raghavav reviewed Apr 18, 2018

View reviewed changes

virajindasrao mentioned this pull request Apr 20, 2018

get out machine name #20

Closed

raghavav suggested changes Jun 4, 2018

View reviewed changes

virajindasrao added 9 commits June 13, 2018 11:24

Update blank resource_configuration property values on refresh

65abdeb

Signed-off-by: Viraj Indasrao <[email protected]>

Go fmt

ed3dbf8

got vet Signed-off-by: Viraj Indasrao <[email protected]>

Fetch deployment details example

cb18b41

Signed-off-by: Viraj Indasrao <[email protected]>

Example with output and comments added

bbb4fb2

Minor fixes Signed-off-by: Viraj Indasrao <[email protected]>

Blueprint added in example

826d086

Signed-off-by: Viraj Indasrao <[email protected]>

go vet/go fmt changes

028502d

Signed-off-by: Viraj Indasrao <[email protected]>

Example bluepint YAML files

46e2e28

Signed-off-by: Viraj Indasrao <[email protected]>

Example tf config file minor changes

01b0d20

Signed-off-by: Viraj Indasrao <[email protected]>

Update resourc_configuration restructuring

52f2559

Signed-off-by: Viraj Indasrao <[email protected]>

vmwclabot added the dco-required label Jun 14, 2018

virajindasrao added 6 commits June 14, 2018 18:27

Update blank resource_configuration property values on refresh

f3e50dd

Signed-off-by: Viraj Indasrao <[email protected]>

Go fmt

ec2d0de

got vet Signed-off-by: Viraj Indasrao <[email protected]>

Fetch deployment details example

8e461c3

Signed-off-by: Viraj Indasrao <[email protected]>

Example with output and comments added

332d02a

Minor fixes Signed-off-by: Viraj Indasrao <[email protected]>

Blueprint added in example

783503b

Signed-off-by: Viraj Indasrao <[email protected]>

Example bluepint YAML files

7bd4d8f

Signed-off-by: Viraj Indasrao <[email protected]>

virajindasrao force-pushed the viraj-fetch-machine-details branch from 2e463f4 to 7bd4d8f Compare June 14, 2018 13:03

vmwclabot removed the dco-required label Jun 14, 2018

vmwclabot added the dco-required label Jun 25, 2018

Merge remote-tracking branch 'origin/master' into viraj-fetch-machine…

a94dc77

…-details Signed-off-by: Viraj Indasrao <[email protected]>

virajindasrao force-pushed the viraj-fetch-machine-details branch from 6dc3544 to a94dc77 Compare June 26, 2018 07:46

vmwclabot added dco-required and removed dco-required labels Jun 26, 2018

merged with master

ca55172

Signed-off-by: Viraj Indasrao <[email protected]>

virajindasrao force-pushed the viraj-fetch-machine-details branch from b1d0298 to ca55172 Compare June 27, 2018 12:26

vmwclabot removed the dco-required label Jun 27, 2018

virajindasrao added 3 commits June 28, 2018 17:25

Fixes after master merge

7086a79

Signed-off-by: Viraj Indasrao <[email protected]>

flow change

0c380e3

Signed-off-by: Viraj Indasrao <[email protected]>

merged with master

d470a6e

Signed-off-by: Viraj Indasrao <[email protected]>

vmware-archive deleted a comment from vmwclabot Jul 5, 2018

master merged

8c601d6

Signed-off-by: Viraj Indasrao <[email protected]>

nferro reviewed Mar 15, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fetch machine details #45

Fetch machine details #45

virajindasrao commented Apr 17, 2018

raghavav left a comment

g1ps commented May 18, 2018 •

edited

Loading

g1ps commented May 23, 2018

raghavav commented May 23, 2018 •

edited

Loading

g1ps commented May 24, 2018

DanielVaknin commented May 29, 2018

virajindasrao commented May 29, 2018

raghavav Jun 2, 2018

raghavav Jun 2, 2018

raghavav Jun 2, 2018

raghavav Jun 2, 2018

raghavav Jun 3, 2018

raghavav Jun 4, 2018

raghavav Jun 4, 2018

raghavav Jun 4, 2018

raghavav Jun 4, 2018

raghavav Jun 4, 2018

vmwclabot commented Jun 14, 2018

vmwclabot commented Jun 25, 2018

g1ps commented Jul 5, 2018 •

edited

Loading

nferro Mar 15, 2019

Fetch machine details #45

Are you sure you want to change the base?

Fetch machine details #45

Conversation

virajindasrao commented Apr 17, 2018

raghavav left a comment

Choose a reason for hiding this comment

g1ps commented May 18, 2018 • edited Loading

g1ps commented May 23, 2018

raghavav commented May 23, 2018 • edited Loading

g1ps commented May 24, 2018

DanielVaknin commented May 29, 2018

virajindasrao commented May 29, 2018

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

vmwclabot commented Jun 14, 2018

vmwclabot commented Jun 25, 2018

g1ps commented Jul 5, 2018 • edited Loading

Choose a reason for hiding this comment

g1ps commented May 18, 2018 •

edited

Loading

raghavav commented May 23, 2018 •

edited

Loading

g1ps commented Jul 5, 2018 •

edited

Loading