-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
refactor/sync flag #3793
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
base: develop
Are you sure you want to change the base?
refactor/sync flag #3793
Changes from 3 commits
455b16e
58edfc7
bc0ccad
0425b60
d005d7c
c298199
d596292
cfcb5b5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| alter table `project__secret_storage` drop column `sync_enabled`; | ||
| alter table `project__secret_storage` drop column `sync_interval`; | ||
| alter table `project__secret_storage` drop column `last_synced_at`; | ||
| alter table `project__secret_storage` drop column `last_sync_failed_at`; | ||
|
|
||
| drop table `project__secret_storage__sync_path`; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| alter table `project__secret_storage` add `sync_enabled` boolean not null default false; | ||
| alter table `project__secret_storage` add `sync_interval` int not null default 0; | ||
| alter table `project__secret_storage` add `last_synced_at` datetime null; | ||
| alter table `project__secret_storage` add `last_sync_failed_at` datetime null; | ||
|
|
||
| create table `project__secret_storage__sync_path` ( | ||
| `id` integer primary key autoincrement, | ||
|
|
||
| storage_id int not null, | ||
| path varchar(1000) not null default '', | ||
| prefix varchar(1000) not null default '', | ||
| `separator` varchar(20) not null default '', | ||
|
|
||
| foreign key (`storage_id`) references `project__secret_storage`(`id`) on delete cascade | ||
| ); | ||
|
|
||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,10 @@ | ||
| package sql | ||
|
|
||
| import "github.com/semaphoreui/semaphore/db" | ||
| import ( | ||
| "time" | ||
|
|
||
| "github.com/semaphoreui/semaphore/db" | ||
| ) | ||
|
|
||
| func (d *SqlDb) GetSecretStorages(projectID int) (storages []db.SecretStorage, err error) { | ||
| storages = make([]db.SecretStorage, 0) | ||
|
|
@@ -19,18 +23,31 @@ func (d *SqlDb) GetSecretStorages(projectID int) (storages []db.SecretStorage, e | |
|
|
||
| _, err = d.selectAll(&storages, query, args...) | ||
|
|
||
| if err != nil { | ||
| return | ||
| } | ||
|
|
||
| for i := range storages { | ||
| storages[i].SyncPaths, err = d.GetSecretStorageSyncPaths(storages[i].ID) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
After this refactor, secret storages load Useful? React with 👍 / 👎. |
||
| if err != nil { | ||
| return | ||
| } | ||
| } | ||
|
|
||
| return | ||
| } | ||
|
|
||
| func (d *SqlDb) CreateSecretStorage(storage db.SecretStorage) (newStorage db.SecretStorage, err error) { | ||
| insertID, err := d.insert( | ||
| "id", | ||
| "insert into project__secret_storage (name, type, project_id, params, readonly) values (?, ?, ?, ?, ?)", | ||
| "insert into project__secret_storage (name, type, project_id, params, readonly, sync_enabled, sync_interval) values (?, ?, ?, ?, ?, ?, ?)", | ||
| storage.Name, | ||
| storage.Type, | ||
| storage.ProjectID, | ||
| storage.Params, | ||
| storage.ReadOnly, | ||
| storage.SyncEnabled, | ||
| storage.SyncInterval, | ||
| ) | ||
|
|
||
| if err != nil { | ||
|
|
@@ -39,13 +56,24 @@ func (d *SqlDb) CreateSecretStorage(storage db.SecretStorage) (newStorage db.Sec | |
|
|
||
| newStorage = storage | ||
| newStorage.ID = insertID | ||
|
|
||
| err = d.ReplaceSecretStorageSyncPaths(newStorage.ID, storage.SyncPaths) | ||
| if err != nil { | ||
| return | ||
| } | ||
|
|
||
| newStorage.SyncPaths, err = d.GetSecretStorageSyncPaths(newStorage.ID) | ||
| return | ||
| } | ||
|
|
||
| func (d *SqlDb) GetSecretStorage(projectID int, storageID int) (key db.SecretStorage, err error) { | ||
|
|
||
| err = d.getObject(projectID, db.SecretStorageProps, storageID, &key) | ||
| if err != nil { | ||
| return | ||
| } | ||
|
|
||
| key.SyncPaths, err = d.GetSecretStorageSyncPaths(key.ID) | ||
| return | ||
| } | ||
|
|
||
|
|
@@ -62,13 +90,84 @@ func (d *SqlDb) UpdateSecretStorage(storage db.SecretStorage) error { | |
| "name=?, "+ | ||
| "type=?, "+ | ||
| "params=?, "+ | ||
| "readonly=? "+ | ||
| "readonly=?, "+ | ||
| "sync_enabled=?, "+ | ||
| "sync_interval=? "+ | ||
| "where project_id=? and id=?", | ||
| storage.Name, | ||
| storage.Type, | ||
| storage.Params, | ||
| storage.ReadOnly, | ||
| storage.SyncEnabled, | ||
| storage.SyncInterval, | ||
| storage.ProjectID, | ||
| storage.ID) | ||
|
|
||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| return d.ReplaceSecretStorageSyncPaths(storage.ID, storage.SyncPaths) | ||
| } | ||
|
|
||
| func (d *SqlDb) GetSyncEnabledSecretStorages() (storages []db.SecretStorage, err error) { | ||
| storages = make([]db.SecretStorage, 0) | ||
| _, err = d.selectAll( | ||
| &storages, | ||
| "select * from project__secret_storage where sync_enabled=? and sync_interval>0", | ||
| true, | ||
| ) | ||
| if err != nil { | ||
| return | ||
| } | ||
| for i := range storages { | ||
| storages[i].SyncPaths, err = d.GetSecretStorageSyncPaths(storages[i].ID) | ||
| if err != nil { | ||
| return | ||
| } | ||
| } | ||
| return | ||
| } | ||
|
|
||
| func (d *SqlDb) MarkSecretStorageSynced(storageID int, success bool, at time.Time) error { | ||
| var query string | ||
| if success { | ||
| query = "update project__secret_storage set last_synced_at=? where id=?" | ||
| } else { | ||
| query = "update project__secret_storage set last_sync_failed_at=? where id=?" | ||
| } | ||
| _, err := d.exec(query, at, storageID) | ||
| return err | ||
| } | ||
|
|
||
| func (d *SqlDb) GetSecretStorageSyncPaths(storageID int) (paths []db.SecretStorageSyncPath, err error) { | ||
| paths = make([]db.SecretStorageSyncPath, 0) | ||
| _, err = d.selectAll( | ||
| &paths, | ||
| "select id, storage_id, path, prefix, `separator` "+ | ||
| "from project__secret_storage__sync_path where storage_id=? order by id", | ||
| storageID, | ||
| ) | ||
| return | ||
| } | ||
|
|
||
| func (d *SqlDb) ReplaceSecretStorageSyncPaths(storageID int, paths []db.SecretStorageSyncPath) error { | ||
| if _, err := d.exec("delete from project__secret_storage__sync_path where storage_id=?", storageID); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| for _, p := range paths { | ||
| if _, err := d.insert( | ||
| "id", | ||
| "insert into project__secret_storage__sync_path (storage_id, path, prefix, `separator`) values (?, ?, ?, ?)", | ||
| storageID, | ||
| p.Path, | ||
| p.Prefix, | ||
| p.Separator, | ||
| ); err != nil { | ||
| return err | ||
| } | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In active-active mode, this unconditional scheduler start runs on every node, while the rest of
runServiceexplicitly adds HA coordination only for schedules/websockets. BecauseGetSyncEnabledSecretStorages()reads from shared DB state, each node will callSyncSecretsfor the same storage on the same interval, leading to duplicated external sync operations and race-prone concurrent writes in multi-node deployments.Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@codex fix it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary
runService, so both schedule execution and secret-sync ticks can share the same cluster-wide lock behavior. cli/cmd/root.goL138-L141SecretStorageSyncSchedulerwith an optional tick deduplicator hook and added a lock gate at the start of each tick, preventing duplicate sync execution across active-active nodes when deduplication is available. services/server/secret_storage_sync_scheduler.goL16-L23 services/server/secret_storage_sync_scheduler.goL36-L40 services/server/secret_storage_sync_scheduler.goL68-L73b3b10b0) and created a follow-up PR message via themake_prtool.Testing
go test ./services/servergo test ./cli/cmd ./services/server(fails in this environment becauseapi/router.goexpects embeddedpublic/*assets that are not present during this run)View task →