This repository has been archived by the owner on Sep 6, 2023. It is now read-only.
expose metal_port.vxlan_ids as computed attribute and alternative to vlan_ids #177
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…vlan_ids Signed-off-by: Marques Johansson <[email protected]>
|
This PR needs tests |
Signed-off-by: Tomas Karasek <[email protected]>
Signed-off-by: Tomas Karasek <[email protected]>
|
Note to myself, check error
EDIT: fixed, it was because |
Signed-off-by: Tomas Karasek <[email protected]>
Signed-off-by: Tomas Karasek <[email protected]>
|
In ac70dc0: |
|
Is the API sorting the vlans in a way that we don't expect? Maybe that's not a real problem and the tests and documentation should reflect it? |
|
On the other hand, maybe we should just reorder the indexes on these tests: (0 -> test1, 1 -> test2) |
Signed-off-by: Tomas Karasek <[email protected]>
|
@displague the problem was with the destroy check GETting a port in process of removal, returning 403 instead of 404. I added additional test step which removes the vlans first. I changed the vxland_ids check to Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("metal_port.bond0", "vxlan_ids.#", "2"),
resource.TestMatchResourceAttr("metal_port.bond0", "vxlan_ids.0",
regexp.MustCompile("1001|1002")),
resource.TestMatchResourceAttr("metal_port.bond0", "vxlan_ids.1",
regexp.MustCompile("1001|1002")),
),.. because I think the test should be independent on the order of the VLANs in the API resource. |
|
@displague this is ready for review |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I'm using this branch to identify shortcomings of the metal_port as discovered in real integration work.
Some of the changes proposed here may be unnecessary.
add metal_port.vxlan_ids
It may be helpful to refer to vxlan_ids because the user can know these in advance (they are user-configurable). These ids can be sent to userdata to get OS networking configured properly. Depending on the UUID makes need for more boilerplate.
I was converting a vlan_attachment resource to metal_port and accidentally used the vxlan_id field.
metal_port.vlan_idsinitially accepted the "1000" value, but on a refresh, Terraform wanted to reset this to the UUID.Will it be problematic to have a computed vlan_ids and vxlan_ids? Does this prevent new settings from taking effect or does this prevent unwanted VLANs from being detected and detached by Terraform?