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

Implement setting bandwidth_limit on a servers network interface #206

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

89Q12
Copy link
Member

@89Q12 89Q12 commented Oct 24, 2024

Description

After the go-anxcloud release, we now implement the bandwidth_limit field on the server network interface.
This currently is rebased onto the dependabot branch to be able access the newly added field in go-anxcloud.

Checklist

  • added release notes to Unreleased section in CHANGELOG.md, if user facing change

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

dependabot bot and others added 2 commits October 21, 2024 19:17
Bumps [go.anx.io/go-anxcloud](https://github.com/anexia-it/go-anxcloud) from 0.7.3 to 0.7.4.
- [Release notes](https://github.com/anexia-it/go-anxcloud/releases)
- [Changelog](https://github.com/anexia-it/go-anxcloud/blob/main/CHANGELOG.md)
- [Commits](anexia-it/go-anxcloud@v0.7.3...v0.7.4)

---
updated-dependencies:
- dependency-name: go.anx.io/go-anxcloud
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
@89Q12 89Q12 self-assigned this Oct 24, 2024
@nachtjasmin
Copy link
Contributor

nachtjasmin commented Oct 24, 2024

The fact that the new autogenerated documentation is losing a lot of information in comparison to the previous iteration makes me disapprove this PR immediately. :/

Haven't checked the code itself yet, gonna do that in a sec. 👀

For now I'd prefer it if we somehow manually adjust the documentation and ensure that it matches the autogenerated version in a second PR.

@89Q12
Copy link
Member Author

89Q12 commented Oct 24, 2024

I actually didn't give much thought to the docs generator lol Just said it will replace all files and hence I didnt look deeper into why they all changed.
I adjusted the docs accordingly and its all good now

Copy link
Contributor

@nachtjasmin nachtjasmin left a comment

Choose a reason for hiding this comment

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

The code would violate the acceptance criteria right now.

Since it is possible to change the network speed by scaling the VM via the Engine, it also should be possible via the Terraform provider.

Besides that: Thanks for the quick documentation fix! 👍

anxcloud/resource_virtual_server.go Show resolved Hide resolved
@nachtjasmin
Copy link
Contributor

Tried to test it manually, and the creation worked fine. However, a second invocation of terraform plan did not work.

To reproduce it:

  1. Copy the example code from the official docs to a local .tf file somewhere.
  2. Compile the binary, together with a version suffix.
  • I just patched the Makefile locally and ran make build, see below.
  1. Point terraform to the override.
  2. Run terraform apply. The server is built as intended and everything works, you can see them in the engine.
  3. Run terraform plan again, after everything has been created.

I got the following output:

tf apply -auto-approve
╷
│ Warning: Provider development overrides are in effect
│ 
│ The following provider development overrides are set in the CLI configuration:
│  - anexia-it/anxcloud in /home/joster/repos/anexia-it/terraform-provider-anxcloud
│ 
│ The behavior may therefore not match any released version of the provider and applying changes may cause the state to become incompatible with published releases.
╵
data.anxcloud_core_location.anx04: Reading...
data.anxcloud_core_location.anx04: Read complete after 1s [id=52b5f6b2fd3a4a7eaaedf1a7c019e9ea]
anxcloud_vlan.example: Refreshing state... [id=c13d44bd070c4477a7cc9b1f182f445f]
anxcloud_network_prefix.v6: Refreshing state... [id=076ef87b174942df90085b20a5fae8d1]
anxcloud_network_prefix.v4: Refreshing state... [id=da9eaa5ca2db4ac6a10b54390a6e9082]
anxcloud_ip_address.v6: Refreshing state... [id=1873d26ab47a4adcaec023ece59f871e]
anxcloud_ip_address.v4: Refreshing state... [id=79f592ce58ac4d6894fcc40e4220224d]
anxcloud_virtual_server.example: Refreshing state... [id=e7a4f32fe52c484a8dc90678af06e3eb]

Planning failed. Terraform encountered an error while generating this plan.

╷
│ Error: Request cancelled
│ 
│ The plugin6.(*GRPCProvider).PlanResourceChange request was cancelled.
╵
error: Recipe `apply` failed on line 2 with exit code 1

addendum: makefile.patch

diff --git a/Makefile b/Makefile
index 8dace30908..ee4d0bdd79 100644
--- a/Makefile
+++ b/Makefile
@@ -5,8 +5,8 @@
 HOSTNAME=hashicorp.com
 NAMESPACE=anexia-it
 NAME=anxcloud
-BINARY=terraform-provider-${NAME}
-VERSION=0.3.1
+VERSION=0.7.0
+BINARY=terraform-provider-${NAME}_v${VERSION}
 OS_ARCH=linux_amd64
 GOLDFLAGS= -s -X github.com/anexia-it/terraform-provider-anxcloud.version=$(VERSION)

@89Q12
Copy link
Member Author

89Q12 commented Nov 6, 2024

Everything should be addressed now

Copy link
Contributor

@nachtjasmin nachtjasmin left a comment

Choose a reason for hiding this comment

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

It's working as intended, wonderful!

If you could add one additional known limitation, which informs our users about the forced replacements for bandwidth_limit changes, that'd be splendid.

A simple line like "Changing the speed on a network interface forces a replacement of the VM" is sufficient IMO.

Good work! 🥳

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.

2 participants