feat: add VRF support #271
Conversation
|
I committed the changes you recommended and reviewed everything, I am going to mark this as "Ready for Review". |
|
@irusoeqx Have a look at the comments GitHub Actions left. These should help guide development to some extent. Continue to look at the other resources for inspiration. Something to be aware of is that we've been migrating resources in this provider from packngo to metal-go. Thanks again for opening this PR, responding to the feedback, and learning Go in the process 🚀 |
internal/vrf/create.go
Outdated
| cmd.SilenceUsage = true | ||
|
|
||
| req := &packngo.VRFCreateRequest{ | ||
| Metro: metro, |
There was a problem hiding this comment.
Metro in which to create the VLAN. Mutually exclusive with Facility. We are supporting Facility too.
There was a problem hiding this comment.
VRFs can only be created in metros. The underlying API for this feature doesn't support facility.
internal/vrf/vrf.go
Outdated
| root.PersistentPreRun(cmd, args) | ||
| } | ||
| } | ||
| c.Service = c.Servicer.API(cmd).ProjectVRFs //Not sure about this endpoint |
There was a problem hiding this comment.
@irusoeqx if you're up for it, you can convert your PR to use metal-go by rebasing on the latest commit from the metal-cli main branch. As of that commit, there is a MetalAPI function that provides a metal-go client.
With metal-go, this line would become something like
c.Service = c.Servicer.MetalAPI(cmd).VRFsAPIDocs for VRFsApi are here (note that the code examples in docs are auto-generated and serve as examples of syntax, not of fully-working code): https://github.com/equinix-labs/metal-go/blob/main/docs/VRFsApi.md
There was a problem hiding this comment.
Yes il do that, I just need a little time.
Hey there, was wondering if you were going to be able to come back to this or not. It's fine if not, we'll just close this and do a new PR for VRF support on our own. |
|
Hey Chris,
Sorry I kind of fell off on this one, I got busy with so many other things, you may go ahead and create one on your own.
Apologies guys 🙁
…-Ilkka
________________________________
From: Chris Privitere ***@***.***>
Sent: Tuesday, August 15, 2023 12:02 AM
To: equinix/metal-cli ***@***.***>
Cc: Ilkka David Ruso ***@***.***>; Mention ***@***.***>
Subject: [EXTERNAL] Re: [equinix/metal-cli] update to vrf (PR #271)
Yes il do that, I just need a little time.
Hey there, was wondering if you were going to be able to come back to this or not. It's fine if not, we'll just close this and do a new PR for VRF support on our own.
—
Reply to this email directly, view it on GitHub<https://urldefense.com/v3/__https://github.com/equinix/metal-cli/pull/271*issuecomment-1678056644__;Iw!!PcPv50trKLWG!wn4Avb_NfF4X2TVuTssxZfNf22fTZlHhZtzxcz-jRyE7-cnBgGYD0YroEsshWqffL4YKZYalYdOLUY-r1n2zFkQ$>, or unsubscribe<https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AYRADJ3YAQGGM5AAHHQHPFDXVKG7RANCNFSM6AAAAAAYNVJYXU__;!!PcPv50trKLWG!wn4Avb_NfF4X2TVuTssxZfNf22fTZlHhZtzxcz-jRyE7-cnBgGYD0YroEsshWqffL4YKZYalYdOLUY-rpVEDHM8$>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
I'm not sure how this was closed. I force pushed some changes and then the convention commits bot chimed in. We'll see if we can pull these commits in to the work when we revisit the VRF features in Metal CLI, soon. |
|
I see what I did wrong: I reset the PR to match main, so the PR was closed as a no-op. |
|
Here's the branch with my changes to get VRF create basically working: https://github.com/equinix/metal-cli/compare/main...displague:irusoeqx/main?expand=1 I didn't run a full test because my project did not seem to have permission to create a VRF. |
VRF support
Hi
Go rookie here trying my hand and adding some VRF support to the metal-cli. I mostly just tried to just build off other functions and structure the code so that it uses the correct parameters and expects the right things to be fed to it. Really hoping to get some feedback here, I know my code is far from perfect but I am hoping to get some working knowledge of GoLang out of this. Any and all feedback is very welcome!
Import for
"github.com/equinix/metal-cli-vrfsupport/internal/vrf"in the cli.go file is meant to be just temporary for testing just so the whole things gets put together correctly. Not sure if this is necessary. Would also like to know if there is a better way to unit test the functions without manually downloading them, or including them in the main packages directory.Fixes #268