init node location updater #2427
Conversation
- add updater package - expose GetState from registerer - fix longitude spelling
Omarabdul3ziz
requested review from
muhamadazmy,
ashraffouda,
rawdaGastan,
xmonader and
Eslam-Nawara
as code owners
There was a problem hiding this comment.
While i am not sure why zos needs to update location every 24 hours i don't believe this is the right way.
The registration service should be the only service to issue an update node (or create node) start to update the node outside this context can lead to unmanaged and hard to debug code.
Registration service already has a mechanism to update the node when some changes are detected (like ip change) and i think this should be included there
|
the issue is updates happen to some ips on the MaxMind database it may be a change in the country name from English chars to native chars like the suggestion was to update the node location periodically. since the changes in the database aren't frequent i thought once a day is a good period to check. i moved the code for update location to be part of the register package |
pkg/registrar/registrar.go
Outdated
| @@ -146,6 +152,10 @@ func (r *Registrar) register(ctx context.Context, cl zbus.Client, env environmen | |||
| if err := r.reActivate(ctx, cl, env); err != nil { | |||
| log.Error().Err(err).Msg("failed to reactivate account") | |||
| } | |||
| case <-time.After(updateLocationInterval): | |||
| if err := r.updateLocation(ctx, cl); err != nil { | |||
There was a problem hiding this comment.
Code in general looks way better. But why just not call register() here. It already collects the location, then compare if the node information has changed before calling update.
Checklist