[PLAT-16179] YBA Installer to correctly update migration tracking

Summary:
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/D40644

managed/yba-installer/Makefile

@@ -109,8 +109,9 @@ clean-package:
 	rm -rf ${PACKAGE_NAME}
 
 .PHONY: test
-test:
+test: config
+	go test ./...
 
 .PHONY: config
 config:
-	cp ${CONFIG_INPUT_FILE_NAME} ${CONFIG_EMBEDED_NAME}
+	cp ${CONFIG_INPUT_FILE_NAME} ${CONFIG_EMBEDED_NAME}

managed/yba-installer/cmd/platform.go

@@ -260,7 +260,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 {
@@ -390,7 +390,7 @@ func (plat Platform) Uninstall(removeData bool) error {
 		}
 		// reload systemd daemon
 		if err := systemd.DaemonReload(); err != nil {
-			return fmt.Errorf("failed to uninstall platform: %w")
+			return fmt.Errorf("failed to uninstall platform: %w", err)
 		}
 	}
 

managed/yba-installer/cmd/replicated_migrate.go

@@ -102,7 +102,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
 

managed/yba-installer/cmd/upgrade.go

@@ -43,7 +43,7 @@ func rollbackUpgrade(backupDir string, state *ybactlstate.State) {
 	findArgs := []string{common.GetActiveSymlink() + "/", "-name", "yba-ctl", "-exec", "cp", "{}",
 		common.YbactlInstallDir(), ";"}
 	if out := shell.Run("find", findArgs...); !out.SucceededOrLog() {
-		log.Warn(fmt.Sprintf("failed to reinstall yba-ctl: %w", out.Error))
+		log.Warn(fmt.Sprintf("failed to reinstall yba-ctl: %s", out.Error.Error()))
 	}
 
 	// Remove newest install
@@ -57,7 +57,7 @@ func rollbackUpgrade(backupDir string, state *ybactlstate.State) {
 
 	// reconfigure with the old binary
 	if out := shell.Run(filepath.Join(common.YbactlInstallDir(), "yba-ctl"), "reconfigure"); !out.SucceededOrLog() {
-		log.Warn(fmt.Sprintf("failed to reconfigure with old yba version: %w", out.Error))
+		log.Warn(fmt.Sprintf("failed to reconfigure with old yba version: %s", out.Error.Error()))
 	}
 
 	// cleanup old backups

managed/yba-installer/pkg/preflight/checks/services_running.go

@@ -48,7 +48,7 @@ func (s *servicesRunningCheck) Execute() Result {
 			continue
 		}
 		if !common.IsHappyStatus(status) {
-			logging.Error(fmt.Sprintf("%s has bad status %s", service.Name(), status))
+			logging.Error(fmt.Sprintf("%s has bad status %s", service.Name(), status.Status))
 			failedServices = append(failedServices, service.Name())
 		}
 	}

managed/yba-installer/pkg/ybactlstate/migration.go

@@ -58,11 +58,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

managed/yba-installer/pkg/ybactlstate/migration_test.go

@@ -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