feat(kurl-migration): persist migration state #3252
Conversation
…github.com:replicatedhq/embedded-cluster into diamonwiggins/130971/kurl-migration-api-foundation
…github.com:replicatedhq/embedded-cluster into diamonwiggins/130971/kurl-migration-api-foundation
…github.com:replicatedhq/embedded-cluster into diamonwiggins/130971/kurl-migration-api-foundation
|
This PR has been released (on staging) and is available for download with a embedded-cluster-smoke-test-staging-app license ID. Online Installer: Airgap Installer (may take a few minutes before the airgap bundle is built): Happy debugging! |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
JGAntunes
left a comment
JGAntunes
left a comment
There was a problem hiding this comment.
Some small comments/questions.
| // persistedState represents the structure saved to migration-state.json | ||
| type persistedState struct { | ||
| MigrationID string `json:"migrationId"` | ||
| TransferMode string `json:"transferMode"` | ||
| Config types.LinuxInstallationConfig `json:"config"` // resolved/merged config | ||
| UserConfig types.LinuxInstallationConfig `json:"userConfig"` // user-provided config | ||
| Status types.KURLMigrationStatusResponse `json:"status"` | ||
| } |
There was a problem hiding this comment.
We could probably re-use the same state between file and memory store:
embedded-cluster/api/internal/store/kurlmigration/store.go
Lines 52 to 61 in e875af5
| Config types.LinuxInstallationConfig `json:"config"` // resolved/merged config | ||
| UserConfig types.LinuxInstallationConfig `json:"userConfig"` // user-provided config |
There was a problem hiding this comment.
Still unclear to me how these 2 will defer, I guess that's logic that will land later?
Thoughts on renaming Config to something clearer (e.g. ResolvedConfig or MergedConfig like you have have in the comment)
| func TestFileStore_JSONFormatting(t *testing.T) { | ||
| tmpDir := t.TempDir() | ||
| store := NewFileStore(tmpDir) | ||
|
|
||
| config := types.LinuxInstallationConfig{ | ||
| DataDirectory: "/var/lib/embedded-cluster", | ||
| } | ||
| err := store.InitializeMigration("test-id", "copy", config) | ||
| require.NoError(t, err) | ||
|
|
||
| // Read file and verify it's valid, pretty-printed JSON | ||
| statePath := filepath.Join(tmpDir, "migration-state.json") | ||
| data, err := os.ReadFile(statePath) | ||
| require.NoError(t, err) | ||
|
|
||
| // Verify it's valid JSON | ||
| var jsonData map[string]interface{} | ||
| err = json.Unmarshal(data, &jsonData) | ||
| require.NoError(t, err) | ||
|
|
||
| // Verify pretty-printing (should contain newlines and indentation) | ||
| assert.Contains(t, string(data), "\n") | ||
| assert.Contains(t, string(data), " ") | ||
| } |
There was a problem hiding this comment.
This feels implementation specific, I wonder if it's worth testing out? Unless we deem the storage format as part of our API/document that. Even if we do the pretty printing validation seems unnecessary IMO.
What this PR does / why we need it:
Implements file-based state persistence for kURL to Embedded Cluster migration to enable resume capability after interruptions.
Key Changes:
/var/lib/embedded-cluster/migration-state.jsonwith atomic writes (temp file + os.Rename pattern)Technical Implementation:
fileStoreimplements the Store interface as drop-in replacement for memoryStoreThis foundation enables kURL migration to survive process restarts and provides reliable state tracking for the multi-phase migration orchestration.
Which issue(s) this PR fixes:
Fixes https://app.shortcut.com/replicated-prod/story/130972
Does this PR require a test?
Yes - Comprehensive testing added:
Unit Tests (
api/internal/store/kurlmigration/file_store_test.go):Dry Run Test (
tests/dryrun/upgrade_kurl_migration_test.go):/var/lib/embedded-cluster/migration-state.json