Skip to content

Conversation

@mohamed-essam
Copy link
Collaborator

@mohamed-essam mohamed-essam commented Jul 30, 2025

Describe your changes

Issue ticket number and link

Stack

Checklist

  • Is it a bug fix
  • Is a typo/documentation fix
  • Is a feature enhancement
  • It is a refactor
  • Created tests that fail without the change (if possible)
  • Extended the README / documentation, if necessary
  • Documentation is not needed

By submitting this pull request, you confirm that you have read and agree to the terms of the Contributor License Agreement.

@mohamed-essam mohamed-essam force-pushed the feat/auto-upgrade branch 4 times, most recently from 76b9397 to 9d44077 Compare July 30, 2025 18:08
@robertgro
Copy link
Contributor

Related

#1793

#4019

@mohamed-essam
Copy link
Collaborator Author

@lixmal All comments should be resolved, will need to conduct another round of testing however, so will keep the PR as Draft until testing is complete

@mohamed-essam mohamed-essam force-pushed the feat/auto-upgrade branch 5 times, most recently from 03eb50e to d15b3e5 Compare August 4, 2025 14:50
@mohamed-essam mohamed-essam marked this pull request as ready for review August 4, 2025 14:51
@mlsmaycon mlsmaycon requested a review from Copilot August 5, 2025 12:20
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces an auto-update client feature that allows the management server to control client version updates. The feature enables administrators to specify a target version for clients, which can be "latest", "disabled", or a specific semantic version.

  • Added AutoUpdateVersion field to account settings with validation for "latest", "disabled", or semantic version format
  • Implemented platform-specific update mechanisms for Windows, macOS, Linux, and FreeBSD
  • Added update manager that monitors version changes and triggers updates when client version is older than target

Reviewed Changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
version/update.go Made LatestAvailable field public for external access
management/server/types/settings.go Added AutoUpdateVersion field to Settings struct
management/server/token_mgr.go Added AutoUpdateVersion to TURN/relay token updates
management/server/peer.go Added AutoUpdateVersion to peer deletion updates
management/server/http/handlers/accounts/ Added API handlers and validation for AutoUpdateVersion
management/server/http/api/ Added AutoUpdateVersion to API types and OpenAPI spec
management/server/grpcserver.go Added AutoUpdateVersion to sync response
management/server/activity/codes.go Added activity tracking for AutoUpdateVersion updates
management/server/account.go Added handling for AutoUpdateVersion setting changes
management/proto/management.proto Added autoUpdateVersion field to SyncResponse
client/internal/updatemanager/ Implemented platform-specific update mechanisms
client/internal/engine.go Integrated update manager with sync response handling

pascal-fischer
pascal-fischer previously approved these changes Aug 8, 2025
Copy link
Collaborator

@lixmal lixmal left a comment

Choose a reason for hiding this comment

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

Have you put any thoughts into how we could do a rollback if the new version breaks connectivity?

@mohamed-essam
Copy link
Collaborator Author

mohamed-essam commented Aug 13, 2025

@lixmal

Have you put any thoughts into how we could do a rollback if the new version breaks connectivity?

Based on my discussions with @mlsmaycon , the upgrade mechanism is forward-moving only

@mohamed-essam
Copy link
Collaborator Author

@lixmal If it's alright with you, I will mark the PR as no documentation needed for the time being as the dashboard changes are not yet done, as soon as dashboard can modify the auto upgrade version I will open a PR in docs

mlsmaycon and others added 2 commits September 20, 2025 21:56
…rsion-networkmap

Move autoUpdateVersion inside NetworkMap
@Tristan-Theroux
Copy link

Hey,

Just wondering when is this expected to be released in production ?

Best,

@yshah-aromatech
Copy link

Hi Team,

Do we have an expected date for this release?

Thanks

@wgetnz
Copy link

wgetnz commented Oct 2, 2025

Do we have an expected date for this release?

@BashBandito
Copy link

Very much looking forward to this feature.

Thanks for the good work.

u.cancel()
if u.update != nil {
u.update.StopWatch()
u.update = nil
Copy link
Contributor

Choose a reason for hiding this comment

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

You set u.update = nil in Stop() while updateLoop or handleUpdate could still try to access it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is only referenced inside handleUpdate once inside a mutex, so I used this to prevent a race and also added a check so that if u.update is nil it should return

return false
}

if time.Since(u.lastTrigger) < 5*time.Minute {
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure but we can miss a potential upgrade if we just ignore. What is the goal with this condition?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is so it doesn't spam retrying to upgrade with any trigger since each failure will show an error to the user, so this is just a way to backoff failures

@nazarewk
Copy link
Contributor

fixes #4019 #1793 #832

@sonarqubecloud
Copy link

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.