Skip to content

Commit

Permalink
[BACKPORT 2.20.8][PLAT-16179] YBA Installer to correctly update migra…
Browse files Browse the repository at this point in the history
…tion tracking

Summary:
Original commit: None / D40644
Moving from tracking migrations with a maximum "schemaVersion" to using a
list of run schemas "runSchemas" would add an extra run migration.

this change will ensure we do not count any schema as run if it is greater than the
stored "schemaVersion"

Wrote new unit test to cover this case, and ensure all tests are run during
build to help ensure we don't hit this going forward (involved some minor fixes
to have all tests pass)

Test Plan: manual upgrade testing and new unit test

Reviewers: muthu, sanketh

Reviewed By: muthu

Subscribers: yugaware

Differential Revision: https://phorge.dev.yugabyte.com/D40648
  • Loading branch information
shubin-yb committed Dec 13, 2024
1 parent d811166 commit ee89eef
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 4 deletions.
3 changes: 2 additions & 1 deletion managed/yba-installer/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,8 @@ clean-package:
rm -rf ${PACKAGE_NAME}

.PHONY: test
test:
test: config
go test ./...

.PHONY: config
config:
Expand Down
2 changes: 1 addition & 1 deletion managed/yba-installer/cmd/platform.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ func (plat Platform) copyYbcPackages() error {
matches, err := filepath.Glob(ybcPattern)
if err != nil {
return fmt.Errorf("Could not find ybc components in %s. Failed with err %w",
plat.PlatformPackages, err.Error())
plat.PlatformPackages, err)
}

for _, f := range matches {
Expand Down
2 changes: 1 addition & 1 deletion managed/yba-installer/cmd/replicated_migrate.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ NOTE: THIS FEATURE IS EARLY ACCESS
}
statInfo := info.Sys().(*syscall.Stat_t)
log.DebugLF(
fmt.Sprintf("Prometheus user:group ownership - '%s:%s'", statInfo.Uid, statInfo.Gid))
fmt.Sprintf("Prometheus user:group ownership - '%d:%d'", statInfo.Uid, statInfo.Gid))
state.Replicated.PrometheusFileUser = statInfo.Uid
state.Replicated.PrometheusFileGroup = statInfo.Gid

Expand Down
10 changes: 9 additions & 1 deletion managed/yba-installer/pkg/ybactlstate/migration.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,19 @@ func handleMigration(state *State) error {
// mark them as run.
func updateSchemaTracking(state *State) error {
if state._internalFields.RunSchemas != nil {
log.DebugLF("schema tracking already updated")
return nil
}
// allSchemaSlice returns the list for the current max schema, but we need specifically those
// that have already been run - tracked previously by the state SchemaVersion
state._internalFields.RunSchemas = allSchemaSlice()[:state._internalFields.SchemaVersion]
state._internalFields.RunSchemas = make([]int, 0)
for _, schemaVersion := range allSchemaSlice() {
if schemaVersion > state._internalFields.SchemaVersion {
log.DebugLF(fmt.Sprintf("skipping schema %d as it is not yet run", schemaVersion))
continue
}
state._internalFields.RunSchemas = append(state._internalFields.RunSchemas, schemaVersion)
}
log.DebugLF(fmt.Sprintf("updating schema tracking with run schemas of %v",
state._internalFields.RunSchemas))
return nil
Expand Down
23 changes: 23 additions & 0 deletions managed/yba-installer/pkg/ybactlstate/migration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,29 @@ func TestNoMigrationDefinedForIndex(t *testing.T) {
}
}

func TestUpdateTrackingWithSkippedMigration(t *testing.T) {
fs = mockFS{
WriteBuffer: &devNullWriter{}, // we don't care about storing the state
}
schemaVersionCache = 8
state := &State{}
state._internalFields = internalFields{}
state._internalFields.SchemaVersion = 6
state._internalFields.RunSchemas = nil
migrations = make(map[int]migrator)
for _, i := range []int{2, 3, 4, 5, 6, 8, 9} {
addToMigrationMap(i)
}
err := updateSchemaTracking(state)
if err != nil {
t.Errorf("running migrations failed: %s", err.Error())
}
expected := []int{2, 3, 4, 5, 6}
if slices.Compare(expected, state._internalFields.RunSchemas) != 0 {
t.Errorf("expected slice %v. Found slice %v", expected, state._internalFields.RunSchemas)
}
}

func expectedRunSchemas(v int) []int {
expected := make([]int, v)
start := 1
Expand Down

0 comments on commit ee89eef

Please sign in to comment.