Make it easier to plug in other providers and add a mock provider#8
Make it easier to plug in other providers and add a mock provider#8jamielinux merged 3 commits intomainfrom
Conversation
| // HetznerID is the unique ID of the server in Hetzner. | ||
| HetznerID int64 | ||
| // ServerID is the unique ID of the server in the cloud provider. | ||
| ServerID int64 |
There was a problem hiding this comment.
The reason for the HetznerID is that in Hetzner these IDs are int64s, but at a different provider it may be something else (e.g. I wouldn't be surprised if it's a string in most places).
I would propose to add a field per provider, so if we would add support for OVH (or whichever), the ID() function would have a switch on the ProviderName to decide which one to return. (That's also why ID() returns a string, which any ID should be castable to).
There was a problem hiding this comment.
Ahhh right of course, thanks! 🚧
There was a problem hiding this comment.
I think I've implemented your suggestion 🤔 Assuming I interpreted your approach correctly!
gzuidhof
left a comment
There was a problem hiding this comment.
Looks great :)
Could you update the PR name before merging (just for bookkeeping so it's not confusing in the changelog)
mock provider
Before I submitted the Add post_plan_delay option PR, I wanted a way to test it locally. So I created a "mock" provider.
Otherwise I wasn't really sure how to easily test it without using actual hetzner credentials, unless I missed something!
HetznerID
I noticed that
HetznerIDcould be more generic, so I changed that toServerID(or something else appropriate).Wait() called on empty errgroup
I think there's a typo here:
flipper/monitor/monitor.go
Lines 68 to 80 in 8ec4926
This causes a wait on the empty
errgrp(instead oferrggrp).I noticed this because running
./flipper --config mock-flipper.example.yaml monitorwas exiting immediately with no logs and0exit status.I think this doesn't really affect production use:
Watch()returns immediately and the caller thinks monitoring is completed.Group.Start()goroutines are still blocking in their for-select loops (and their child goroutines likeg.healthkeeper.Start()are also still running).server.ListenAndServe()keeps the process alive, so those orphaned goroutines continue working sort of by accident.It's a small fix, but I also added a test.