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

feat: metal_vlan_info supports facility based vlans #198

Merged
merged 2 commits into from
Jul 19, 2024

Conversation

bersoare
Copy link
Contributor

@bersoare bersoare commented Jul 16, 2024

vlans can be facility based, but the current metal_vlan_info code doesn't handle that case - it fails since a facility based vlan doesn't have the metro field, which is treated as required by the VLAN_RESPONSE_ATTRIBUTE_MAP.
this PR changes the metro attribute to be optional, and adds a new optional attribute for the facility, so it can handle both cases

        {
            "description": "native vlan",
            "facility": { <<<
                "href": "/metal/v1/facilities/d2a72094-26c9-4372-8d65-051424bc370a",
                "id": "d2a72094-26c9-4372-8d65-051424bc370a"
            },
            "id": "abcd",
            "metro": "",
            "vxlan": 1004
        },
        {
            "description": "Public NAT",
            "facility": "",
            "id": "abcd",
            "metro": { <<<
                "href": "/metal/v1/locations/metros/d3d6b29f-042d-43b7-b3ce-0bf53d5754ca",
                "id": "d3d6b29f-042d-43b7-b3ce-0bf53d5754ca"
            },
            "vxlan": 1004
        },

addresses #195

@bersoare bersoare force-pushed the metal_vlan_with_facility branch from edee167 to 935b00d Compare July 19, 2024 09:07
@bersoare
Copy link
Contributor Author

rebased

@ctreatma ctreatma merged commit 6f1ee4b into equinix:main Jul 19, 2024
3 of 4 checks passed
Copy link

This PR is included in version 0.8.0 🎉

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.

3 participants