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 "network_data" support #54

Closed
wants to merge 2 commits into from

Conversation

NobbZ
Copy link

@NobbZ NobbZ commented Oct 5, 2021

This pull request adds support for network data by passing it through to gophercloud as is.

As the version of gophercloud used before did not support the NetworkData field before, I also bumbed the version to the lowest version that introduced this key.

@@ -111,6 +111,10 @@ func resourceNodeV1() *schema.Resource {
Optional: true,
Computed: true,
},
"network_data": {
Type: schema.TypeMap,
Copy link
Member

Choose a reason for hiding this comment

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

I seem to vaguely recall that terraform maps do not like really sophisticated nested types. Have you tested it with a real network data?

Copy link
Author

Choose a reason for hiding this comment

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

We did today, and you are right, we need a Map of lists, which can't be represented it seems.

We need to do some further inspection and probably do a complete rework to match our requirements, without breaking to much here.

I will close this PR until we have found a working solution internally.

Copy link
Member

Choose a reason for hiding this comment

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

In a similar situation we used a JSON string. Ugly but works.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, that is what we are going to do as a workaround now. The team will test my changes during the day. Though that is nothing we would upstream back here.

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

2 participants