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

implements metal connection, info and integration tests #118

Merged
merged 28 commits into from
Oct 9, 2023

Conversation

AlexBacho
Copy link
Contributor

@AlexBacho AlexBacho commented Aug 21, 2023

work in progress

Fixes #15
Fixes #54

@AlexBacho AlexBacho temporarily deployed to external September 4, 2023 20:49 — with GitHub Actions Inactive
@AlexBacho AlexBacho temporarily deployed to external September 5, 2023 21:26 — with GitHub Actions Inactive
@AlexBacho AlexBacho temporarily deployed to external September 5, 2023 21:49 — with GitHub Actions Inactive
@AlexBacho
Copy link
Contributor Author

I get a 500 unhandled exception when creating a shared primary connection, any idea what is causing this?

image

@t0mk
Copy link
Contributor

t0mk commented Sep 8, 2023

I get a 500 unhandled exception when creating a shared primary connection, any idea what is causing this?

image

This might be a misconfiguration for connection. Probably an API issue. TF doesn't sanitize this.

@AlexBacho AlexBacho changed the title WIP implements metal connection, info and integration tests implements metal connection, info and integration tests Sep 8, 2023
@AlexBacho
Copy link
Contributor Author

should be ready for review

README.md Outdated Show resolved Hide resolved
plugins/modules/metal_connection.py Show resolved Hide resolved
plugins/modules/metal_connection.py Show resolved Hide resolved
plugins/modules/metal_connection.py Outdated Show resolved Hide resolved
plugins/modules/metal_connection.py Outdated Show resolved Hide resolved
plugins/modules/metal_connection.py Outdated Show resolved Hide resolved
plugins/modules/metal_connection_info.py Outdated Show resolved Hide resolved
plugins/modules/metal_connection_info.py Outdated Show resolved Hide resolved
plugins/modules/metal_connection_info.py Outdated Show resolved Hide resolved
tests/integration/targets/metal_connection/tasks/main.yml Outdated Show resolved Hide resolved
@t0mk
Copy link
Contributor

t0mk commented Sep 21, 2023

@AlexBacho we will need to rework this a bit still. With the API update and with new equinix_metal pip package, you can see that InterconnectionCreateInput class is not genreated anymore. Instead it's defined as oneOf[0], and there is
for type == "dedicated": https://github.com/equinix-labs/metal-python/blob/main/equinix_metal/docs/DedicatedPortCreateInput.md
for type =="shared": https://github.com/equinix-labs/metal-python/blob/main/equinix_metal/docs/VlanFabricVcCreateInput.md

[0] https://github.com/equinix-labs/metal-python/blob/main/oas3.fetched/components/requestBodies/InterconnectionCreateInput.yaml

You will need to update equinix_metal to the latest release to see the new input classes. Please also fix equinix_metal to the newest equinix_metal release in requirements.yml.

@t0mk
Copy link
Contributor

t0mk commented Sep 21, 2023

@AlexBacho I looked a bit more on the API[0]. It seems that there's a way to create IC in project[1] or in an org[2]. The module should handle both. The API input to project can be any of the three [3], and input to org can be only DedicatedPort.

We have 4 combinations here which is not good. We need to think how to handle those in the api_router[4], because in the CREATOR routes, we have a combination of APImethod-Input, I think we'll need 4 entries there

metal_connection_project_dedicated
metal_connection_project_vlanfabric
metal_connection_project_vrf
metal_connection_organization_dedicated

.. in form

        ('`metal_connection_organization_vlanfabric', action.CREATE): spec_types.Specs(
            equinix_metal.InterconnectionsApi(mpc).metal_connection_organization_vlanfabric,
            {'organization_id': 'organization_id'},  # see the args of the function in [5] 
            equinix_metal.DedicatedPortCreateInput,
        ),

.. I think you understand what the configuration should be for the other routes.

You will need to distinguish those for the create event in the module. I do sth similar but much simpler in the metal_project, line 245 [6]. As for the GETTER and UPDATER, there's only one of each [7,8]. As for listers there will be 2 [9,10] (you will need them in metal_connection_info, where there should be two exclusive params project_id and organization_id.

You don't need to implement the fetch by name in metal_connection. Just by ID. It's a pretty bad practice anyway, I shouldn't have done in the collection.

I hope this is clear, let's talk tomorrow.

[0] https://deploy.equinix.com/developers/api/metal#tag/Interconnections
[1] https://deploy.equinix.com/developers/api/metal#tag/Interconnections/operation/createProjectInterconnection
[2] https://deploy.equinix.com/developers/api/metal#tag/Interconnections/operation/createOrganizationInterconnection
[3] https://github.com/equinix-labs/metal-python/blob/main/oas3.fetched/components/requestBodies/InterconnectionCreateInput.yaml
[4] https://github.com/equinix-labs/ansible-collection-equinix/blob/main/plugins/module_utils/metal/api_routes.py
[5] https://github.com/equinix-labs/metal-python/blob/main/equinix_metal/equinix_metal/api/interconnections_api.py#L223
[6] https://github.com/equinix-labs/ansible-collection-equinix/blob/main/plugins/modules/metal_project.py#L245
[7] https://deploy.equinix.com/developers/api/metal#tag/Interconnections/operation/getInterconnection
[8] https://deploy.equinix.com/developers/api/metal#tag/Interconnections/operation/updateInterconnection
[9] https://deploy.equinix.com/developers/api/metal#tag/Interconnections/operation/organizationListInterconnections
[10] https://deploy.equinix.com/developers/api/metal#tag/Interconnections/operation/projectListInterconnections

Copy link
Contributor

@t0mk t0mk left a comment

Choose a reason for hiding this comment

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

@AlexBacho Please fix my latests suggestions, regenerate docs and push.

You did improvements to vlan and project_ssh_key, it's a good intiative, but next time they should go to a separate PR.

plugins/module_utils/metal/spec_types.py Outdated Show resolved Hide resolved
plugins/module_utils/metal/spec_types.py Outdated Show resolved Hide resolved
plugins/modules/metal_vlan.py Outdated Show resolved Hide resolved
plugins/modules/metal_vlan.py Outdated Show resolved Hide resolved
@t0mk t0mk merged commit dfe2ae0 into equinix:main Oct 9, 2023
2 checks passed
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.

metal_connection metal_connection_info
3 participants