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

Support update node location #995

Closed
0oM4R opened this issue Aug 20, 2024 · 4 comments
Closed

Support update node location #995

0oM4R opened this issue Aug 20, 2024 · 4 comments
Assignees
Labels
type_feature New feature or request
Milestone

Comments

@0oM4R
Copy link

0oM4R commented Aug 20, 2024

We need to support updating node locations.
changes on MaxMind Database lead to inconstant location data, threefoldtech/tfgrid-sdk-ts#3021 (comment)

threefoldtech/zos#2401

@sameh-farouk
Copy link
Member

It's already supported on the TFChain side. A node can update its location by calling the update_node.

@xmonader xmonader added the type_feature New feature or request label Sep 18, 2024
@xmonader xmonader added this to 3.15.x Sep 18, 2024
@xmonader xmonader moved this to Done in 3.15.x Sep 18, 2024
@xmonader xmonader added this to the 2.9.0 milestone Sep 18, 2024
@Omarabdul3ziz
Copy link

using update_node requires passing the full node object which can't be reconstructed each time we update the location. i tried to send the node object with the new location struct and the basic fields like nodeid/twinid/farmid but I got errors of missing fields like capacity
image

i see we handle the uptime report with a separate endpoint

func (s *Substrate) UpdateNodeUptime(identity Identity, uptime uint64) (hash types.Hash, err error) {

could we have a separate method that do the same for the location update?

@Omarabdul3ziz Omarabdul3ziz reopened this Sep 18, 2024
@sameh-farouk
Copy link
Member

using update_node requires passing the full node object which can't be reconstructed each time we update the location. i tried to send the node object with the new location struct and the basic fields like nodeid/twinid/farmid but I got errors of missing fields like capacity

I can't understand why the node object can't be reconstructed each time we update the location.
I'm not familiar with ZOS implementation, but the required data is local to the node and should be cheap and straightforward to retrieve. I also assume you are comparing the node location to what is stored on the chain, so you are already getting the full node info from the chain. You could then adjust and update it, and send it back.

It's already used when the node boots up; it compares the real node info with the on-chain info. If they don't match, it calls update_node with the new info.

for example, see this PR for a hint about how you can construct a node object
threefoldtech/zos@c3e183c

Also this:
https://github.com/threefoldtech/zos/blob/a19e11ae0ed76e3faabed863b686bf348ef14b4b/pkg/registrar/register.go#L168-L215

could we have a separate method that do the same for the location update?

It is not optimal to have separate calls for every node property. If there are multiple inconsistencies, you would make multiple transactions to update the node object instead of one call.

Since the requested functionality is already available and used by Zos for the same purpose, introducing an alternative way is not reasonable.

@Omarabdul3ziz
Copy link

my main concern was calculating the used resources while it maybe * during the update * get updated by another event deploying a workload but double checking we don't actually send the used resources while updating.

my suggestion was to tweak the update_node method or add a new one to not require all the fields of the node but only update the fields that has value changed on the node object.

a workaround instead of restructuring the node object locally, we could call the chain to get the node, compare with the new location if changed, overwrite with the new location and send back to the chain with the current update_node method.

this will work fine for my case, so no work needed from the chain side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type_feature New feature or request
Projects
Status: Done
Development

No branches or pull requests

4 participants