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

LoadBalancer ID no longer a number #160

Open
t3hmrman opened this issue Jan 7, 2023 · 7 comments
Open

LoadBalancer ID no longer a number #160

t3hmrman opened this issue Jan 7, 2023 · 7 comments
Labels
awaiting-upstream The issue cannot be resolved without action in another repository (may be owned by Pulumi). kind/bug Some behavior is incorrect or out of spec

Comments

@t3hmrman
Copy link

t3hmrman commented Jan 7, 2023

What happened?

It seems like the Hetzner API has shifted over the years -- IDs for load balancers are no longer numbers, but instead numeric strings sometimes separated by dashes like the following:

nnnnnnn-nnnnnnn

Steps to reproduce

  • Create a LoadBalancer
  • Create a LoadBalancerService or LoadBalancerTarget

Expected Behavior

Creating a LoadBalancer with a LoadBalancerTarget and LoadBalancerServices should have succeeded when applied.

Actual Behavior

An error is produced:

Diagnostics:
  hcloud:index:LoadBalancerTarget (lb-tgt-0):
    error: hcloud:index/loadBalancerTarget:LoadBalancerTarget resource 'lb-tgt-0' has a problem: cannot parse '' as int: strconv.ParseInt: parsing "nnnnnnn-nnnnnnn": invalid syntax. Examine values at 'LoadBalancerTarget.LoadBalancerId'.                         

Output of pulumi about

CLI
Version 3.51.0
Go Version go1.19.4
Go Compiler gc

Plugins
NAME VERSION
aws 2.13.1
aws 2.13.1
hcloud 1.10.1
hcloud 1.10.1
nodejs unknown

Host
OS arch
Version "rolling"
Arch x86_64

Additional context

There is a work around which only works for LoadBalancerService and LoadBalancerNetwork:

const ctrlLBServiceK8s = new hcloud.LoadBalancerService(
  `lb-svc-k8s`,
  {
    loadBalancerId: lb.id.apply((v: string) => v as any),
    protocol: "tcp",
    destinationPort: xxxx,
    listenPort: xxxx,
  },
);

This works on the Typescript side and the resource is applied. I suspect this works since the load_balancer_id is a str in load_balancer_service.py. This trick does not work on the LoadBalancerTarget.

Contributing

Vote on this issue by adding a 👍 reaction.
To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

@t3hmrman t3hmrman added kind/bug Some behavior is incorrect or out of spec needs-triage Needs attention from the triage team labels Jan 7, 2023
@thomas11
Copy link
Contributor

thomas11 commented Jan 7, 2023

Thanks for reporting this, @t3hmrman! It's an odd one since in the official docs of the latest version of Hetzner's own Terraform provider, load balancer ids are still ints. Same for the load_balancer_id parameter of hcloud_load_balancer_target, whereas the same load_balancer_id of hcloud_load_balancer_service is a string.

We'll be looking into this. We are a bit behind the upstream provider, so it might make sense for Pulumi to upgrade first (#159, cc @guineveresaenger).

@thomas11 thomas11 removed the needs-triage Needs attention from the triage team label Jan 7, 2023
@t3hmrman
Copy link
Author

t3hmrman commented Jan 8, 2023

Hey @thomas11 thanks for taking a look -- I was really perplexed by this when it was coming up as an error (especially since the code had been working in a previous project) -- my first instinct was that something someone thought could be a "big int" ended up being much better as a string (and maybe eventually a UUID?) probably at the source :)

I wasn't able to find a work around (I could have either manually attached since the number of connections wasn't too huge, or used a different API from code, etc) but I'm sure someone motivated enough can either change the code or find a workaround.

@t0yv0
Copy link
Member

t0yv0 commented Jan 14, 2023

Hi @t3hmrman would it be possible to check if this is still an issue on the newly released v1.10.2? Thomas thought it might be fixed by the upstream provider upgrade.

@t3hmrman
Copy link
Author

Hey @t0yv0 absolutely! Sorry I haven't been able to get around to this yet -- I changed the infrastructure a little bit.

Should be able to get to this by the end of the week.

@t3hmrman
Copy link
Author

Finally got around to upgrading to v1.10.2 and the error is still there:


    VALUE: 1034544-2407574

  hcloud:index:LoadBalancerTarget (eu-central-0-ctrl-lb-tgt-0):
    error: hcloud:index/loadBalancerTarget:LoadBalancerTarget resource 'eu-central-0-ctrl-lb-tgt-0' has a problem: cannot parse '' as int: strconv.ParseInt: parsing "1034544-2407574": invalid syntax. Examine values at 'LoadBalancerTarget.LoadBalancerId'.

@t0yv0
Copy link
Member

t0yv0 commented Jan 17, 2023

Checked upstream again.

https://github.com/hetznercloud/terraform-provider-hcloud/blob/main/internal/loadbalancer/resource_target.go#L52 still an int

https://github.com/hetznercloud/terraform-provider-hcloud/blob/main/internal/loadbalancer/resource_service.go#L38 is a string here now

I believe the most straightforward way to fix this would be to submit a PR upstream to https://github.com/hetznercloud/terraform-provider-hcloud and have them do a patch release, then go through another round of upgrades here.

@t0yv0 t0yv0 added the awaiting-upstream The issue cannot be resolved without action in another repository (may be owned by Pulumi). label Jan 17, 2023
@mjeffryes
Copy link
Member

Seems the upstream provider doesn't feel this pain because they internally use a string (even through they declare it as an int in TF schema?) (see hetznercloud/terraform-provider-hcloud#862)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-upstream The issue cannot be resolved without action in another repository (may be owned by Pulumi). kind/bug Some behavior is incorrect or out of spec
Projects
None yet
Development

No branches or pull requests

4 participants