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

Add Synergy Support for Logical Interconnect Groups #56

Merged
merged 6 commits into from
Jan 11, 2019
Merged

Add Synergy Support for Logical Interconnect Groups #56

merged 6 commits into from
Jan 11, 2019

Conversation

adarobin
Copy link
Contributor

I am not able to create Logical Interconnect Groups with Terraform as described in #47

I am working to resolve this issue, though it appears there are multiple fields needed to successfully create a LIG.

Signed-off-by: Adam Robinson <adarobin@umich.edu>
Signed-off-by: Adam Robinson <adarobin@umich.edu>
Signed-off-by: Adam Robinson <adarobin@umich.edu>
Signed-off-by: Adam Robinson <adarobin@umich.edu>
Signed-off-by: Adam Robinson <adarobin@umich.edu>
@adarobin adarobin changed the title WIP: Add Synergy Support for Logical Interconnect Groups Add Synergy Support for Logical Interconnect Groups Dec 19, 2018
@adarobin
Copy link
Contributor Author

I'm not sure why CI is failing. The file it is complaining about has been run through gofmt.

@patrickdappollonio
Copy link
Member

Hello @adarobin,

Can you run this command and tell me if it reports any Go files?

find . -type f -name "*.go" -not -path "./vendor/*" | sed "s|^\./||" | xargs gofmt -l

@adarobin
Copy link
Contributor Author

@patrickdappollonio

m-c02tv2lthtd7:terraform-provider-oneview adarobin$ find . -type f -name "*.go" -not -path "./vendor/*" | sed "s|^\./||" | xargs gofmt -l
oneview/resource_ethernet_network.go
oneview/resource_fc_network.go
oneview/resource_fcoe_network.go
oneview/resource_network_set.go

Interesting that the files listed are not what is in the output from CI as having the error. I'll fix them up and do another commit.

@adarobin
Copy link
Contributor Author

The only thing I'm not happy with is that I need to go back and manually set enclosure_indexes to [1] for all of my C7000 enclosures. I'd like to make [1] the default value, but that is not possible in the version of Terraform that this plugin is linked against.

@adarobin
Copy link
Contributor Author

@patrickdappollonio
Now the files that were listed as needing fixed on my machine are listed in the CI. It appears to be a difference in gofmt behavior between versions. See golang/go#26228

m-c02tv2lthtd7:terraform-provider-oneview adarobin$ go version
go version go1.11.2 darwin/amd64

Signed-off-by: Adam Robinson <adarobin@umich.edu>
@soodpr
Copy link
Member

soodpr commented Jan 3, 2019

@adarobin - Hi Adam. I have reviewed your PR and it looks good to me. May i know for which OneView API version you have tested this code?

@soodpr
Copy link
Member

soodpr commented Jan 3, 2019

The only thing I'm not happy with is that I need to go back and manually set enclosure_indexes to [1] for all of my C7000 enclosures. I'd like to make [1] the default value, but that is not possible in the version of Terraform that this plugin is linked against.

Hello @adarobin - I agree we should use latest Terraform version. May be we can take this as an enhancement in future.

Copy link
Member

@soodpr soodpr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@adarobin
Copy link
Contributor Author

adarobin commented Jan 3, 2019

@soodpr I tested with API version 600.

@soodpr soodpr merged commit 49926c4 into HewlettPackard:master Jan 11, 2019
@adarobin adarobin deleted the fix_issue_47 branch January 15, 2019 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants