-
Notifications
You must be signed in to change notification settings - Fork 46
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
chore: Migrate ports command from packngo to metal-go client #355
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add a test to ensure that the convert/layer-3, convert/layer-2, disbond, and bond, endpoints are reached when the associated arguments are supplied.
Signed-off-by: Ayush Rangwala <[email protected]>
@aayushrangwala There are a few error logs and test failures related to these changes in the most recent E2E run:
|
Rerunning since the failed test (createdevice) could be a flake. If the helper hasn't been modified I would definitely assume this was a flake. |
@displague Yes it was flaky test. Tests passed in this run |
@aayushrangwala Re: this note in the PR description:
@sbhatnagar-equinix did some focused work earlier this year to align include/exclude parameter declarations in the spec with actual parameter support in the API. Surbhit can confirm whether these missing methods are expected to be missing. If we'd like to validate the API behavior ourselves, one thing to try is to make a # Run commands like the following and see if the request changes due to the addition of exclude param
$ curl -v -H 'X-Auth-Token: <some_token>' https://api.equinix.com/metal/v1/<path_to_endpoint>
$ curl -v -H 'X-Auth-Token: <some_token>' https://api.equinix.com/metal/v1/<path_to_endpoint>?excludes=<something> If we do find differences we can put those details in a ticket for the API team. |
I can't seem to use this command to convert a port to Layer 3.
^ Fortunately, for this PR, I can't seem to do that with the existing 1.16 binary either 😬 While looking at the
|
@ctreatma I remember we discussing this and as you correctly pointed out, there was some work on this but still after that only After checking, exclude doesnot work via curl as well hence the spec and API doc is correct
|
@displague #374 Created this issue |
Signed-off-by: Ayush Rangwala <[email protected]>
@displague Can you please re-trigger the E2E tests. I think was a flaky scenario |
VLANS Assign and Un-assign
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why port convert from layer2 to layer3 bond with IPV4 and IPV6 shows some Junk data
Vasubabus-MacBook-Pro:bin vasubabu$ ./metal port convert -i d414a2ca-2e91-4e2d-a598-42180c354e9d -2=false -b -4 -6
2023/11/07 23:29:57 Converting port d414a2ca-2e91-4e2d-a598-42180c354e9d to layer-3 with addresses [{0x140002c2540 0x140002c2544 map[]} {0x140002c2540 0x140002c2544 map[]} {0x140002c2540 0x140002c2544 map[]}]
Its returning the pointers for IP addresses objects. Will fix it |
One E2E test failed:
|
Going to merge this to unblock other work. Ahead of the next release (ahead of merging any |
Issue Task as part of migrating metal-cli from packngo to metal-go client, added the support of ports subcommand to use metal-go
Part of: #333
Discussion:
As of metal-go 0.22.2 there is 1 issue which needs api support
FindPortById
,BondPort
,DisbondPort
,CreatePortVlanAssignmentBatch
etc needs to supportExclude([]string)
parameter as well. It only supportsIncludes([]string)