-
Notifications
You must be signed in to change notification settings - Fork 83
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
Support For Managing Reserved IP Addresses #579
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.
LGTM. Great test coverage! Just one small comment:
// Verify that we can't reserve more IPs | ||
_, exceedErr := client.ReserveIPAddress(context.Background(), ReserveIPOptions{Region: "us-east"}) | ||
if exceedErr == nil { | ||
t.Errorf("Expected error when exceeding reservation limit, got nil") | ||
} |
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.
The reservation limit seems can be expanded. I think we might want to remove this verification to avoid unexpected failure.
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.
Removed this check and re-recorded the fixture for this test.
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.
I think it is still a good test coverage to have. I pushed a commit separating into a new test case.
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.
The new test case works well, thank you @ykim-1!
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.
Tested and works well on my end, thank you for the contribution!
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.
LGTM, test pass locally and thank you for the contribution!
…d, InsuffecientPermission, ReserveIP, GetReservedIP, getReservedIPs, DeleteReservedIPs
…y appropriate error message along with code
…ord user with adequate permissions
…d fatalf wherever necessary
…es with latest error messages
… feature. Removed for loop to reserve IPs till limit is reached.
…d fixtures to reflect new error messages
* build(deps): bump golang.org/x/oauth2 from 0.22.0 to 0.23.0 Bumps [golang.org/x/oauth2](https://github.com/golang/oauth2) from 0.22.0 to 0.23.0. - [Commits](golang/oauth2@v0.22.0...v0.23.0) --- updated-dependencies: - dependency-name: golang.org/x/oauth2 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> * Ran make tidy --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: ezilber-akamai <ezilber@akamai.com>
* build(deps): bump golang.org/x/text from 0.17.0 to 0.18.0 Bumps [golang.org/x/text](https://github.com/golang/text) from 0.17.0 to 0.18.0. - [Release notes](https://github.com/golang/text/releases) - [Commits](golang/text@v0.17.0...v0.18.0) --- updated-dependencies: - dependency-name: golang.org/x/text dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> * make tidy --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Ye Chen <yechen@akamai.com>
…ing endpoints (linode#573) * Add LKE types endpoints * Support base struct; add NB types endpoints * Add volume types * Add network transfer prices * Add price and region price structs * Revert IPv6 fixtures * Add missing fixtures
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.
Nice work! Tests are passing on my end. Thanks for the contribution!
📝 Description
This PR introduces comprehensive functionality and tests for Reserved IP Addresses in the Linodego client. The changes are necessary to provide robust support for managing Reserved IP addresses through the Linode API. The changes include:
Implementation of core Reserved IP operations:
Test coverage:
✔️ How to Test
What are the steps to reproduce the issue or verify the changes?
To verify the changes related to Reserved IP functionality, follow these steps:
export LINODE_TOKEN="your_token_here"
Additionally you need to set the following if you want to test it in a different environment:
Navigate to the test directory within the linodego project. Update the LINODE_TOKEN in the Makefile:
LINODE_TOKEN="your_token_here"
To run all integration tests:
make testint
To run only Reserved IP related tests:
go test -v ./integration -run TestReservedIPAddresses
To run a specific test:
go test -v ./integration -run TestReservedIPAddresses_EndToEndTest
Note:
Ensure you have proper permissions and sufficient quota in your Linode account to perform Reserved IP operations. Some tests may create and delete resources, so use a testing environment if possible.