USHIFT-1413: Update prerun after storage migration design changes

pkg/admin/prerun/health.go

@@ -0,0 +1,50 @@
+package prerun
+
+import (
+	"encoding/json"
+	"errors"
+	"fmt"
+	"os"
+	"strings"
+
+	"github.com/openshift/microshift/pkg/admin/data"
+	"github.com/openshift/microshift/pkg/util"
+)
+
+var (
+	healthFilepath            = "/var/lib/microshift-backups/health.json"
+	errHealthFileDoesNotExist = errors.New("health file does not exist")
+)
+
+type HealthInfo struct {
+	Health       string `json:"health"`
+	DeploymentID string `json:"deployment_id"`
+	BootID       string `json:"boot_id"`
+}
+
+func (hi *HealthInfo) BackupName() data.BackupName {
+	return data.BackupName(fmt.Sprintf("%s_%s", hi.DeploymentID, hi.BootID))
+}
+
+func (hi *HealthInfo) IsHealthy() bool {
+	return hi.Health == "healthy"
+}
+
+func getHealthInfo() (*HealthInfo, error) {
+	if exists, err := util.PathExistsAndIsNotEmpty(healthFilepath); err != nil {
+		return nil, err
+	} else if !exists {
+		return nil, errHealthFileDoesNotExist
+	}
+
+	content, err := os.ReadFile(healthFilepath)
+	if err != nil {
+		return nil, fmt.Errorf("failed to read health data from %q: %w", healthFilepath, err)
+	}
+
+	health := &HealthInfo{}
+	if err := json.Unmarshal(content, &health); err != nil {
+		return nil, fmt.Errorf("failed to parse health data %q: %w", strings.TrimSpace(string(content)), err)
+	}
+	return health, nil
+}

pkg/admin/prerun/prerun.go

@@ -3,7 +3,6 @@ package prerun
 import (
 	"bytes"
 	"encoding/json"
-	"errors"
 	"fmt"
 	"os"
 	"os/exec"
@@ -15,25 +14,6 @@ import (
 	"k8s.io/klog/v2"
 )
 
-var (
-	healthFilepath            = "/var/lib/microshift-backups/health.json"
-	errHealthFileDoesNotExist = errors.New("health file does not exist")
-)
-
-type HealthInfo struct {
-	Health       string `json:"health"`
-	DeploymentID string `json:"deployment_id"`
-	BootID       string `json:"boot_id"`
-}
-
-func (hi *HealthInfo) BackupName() data.BackupName {
-	return data.BackupName(fmt.Sprintf("%s_%s", hi.DeploymentID, hi.BootID))
-}
-
-func (hi *HealthInfo) IsHealthy() bool {
-	return hi.Health == "healthy"
-}
-
 type PreRun struct {
 	dataManager data.Manager
 }
@@ -120,7 +100,7 @@ func (pr *PreRun) Perform() error {
 		// (i.e. backup was manually restored but health.json not deleted).
 
 		klog.InfoS("Data exists, but version file is missing - assuming upgrade from 4.13")
-		return pr.upgradeFrom413()
+		return pr.backup413()
 	}
 
 	// 7
@@ -184,22 +164,6 @@ func (pr *PreRun) regularPrerun() error {
 		if err := pr.backup(health); err != nil {
 			return fmt.Errorf("failed to backup during pre-run: %w", err)
 		}
-
-		migrationNeeded, err := pr.checkVersions()
-		if err != nil {
-			return fmt.Errorf("failed version checks: %w", err)
-		}
-
-		klog.InfoS("Completed version checks", "is-migration-needed?", migrationNeeded)
-
-		if migrationNeeded {
-			_ = migrationNeeded
-			// TODO: data migration
-
-			if err := writeExecVersionToData(); err != nil {
-				return fmt.Errorf("failed to write MicroShift version to data directory: %w", err)
-			}
-		}
 	} else {
 		klog.Info("Previous boot was not healthy")
 		if err = pr.restore(); err != nil {
@@ -210,19 +174,13 @@ func (pr *PreRun) regularPrerun() error {
 	return nil
 }
 
-func (pr *PreRun) upgradeFrom413() error {
+func (pr *PreRun) backup413() error {
 	backupName := data.BackupName("4.13")
 
 	if err := pr.dataManager.Backup(backupName); err != nil {
 		return fmt.Errorf("failed to create new backup %q: %w", backupName, err)
 	}
 
-	// TODO: data migration
-
-	if err := writeExecVersionToData(); err != nil {
-		return fmt.Errorf("failed to write MicroShift version to data directory: %w", err)
-	}
-
 	return nil
 }
 
@@ -301,27 +259,6 @@ func (pr *PreRun) restore() error {
 	return nil
 }
 
-// checkVersions compares version of data and executable
-//
-// It returns true if migration should be performed.
-// It returns non-nil error if difference between versions is unsupported.
-func (pr *PreRun) checkVersions() (bool, error) {
-	klog.Info("Starting version checks")
-	execVer, err := getVersionOfExecutable()
-	if err != nil {
-		return false, fmt.Errorf("failed to determine the active version of the MicroShift: %w", err)
-	}
-
-	dataVer, err := getVersionOfData()
-	if err != nil {
-		return false, fmt.Errorf("failed to determine the version of the existing data: %w", err)
-	}
-
-	klog.InfoS("Comparing versions", "data", dataVer, "active", execVer)
-
-	return checkVersionDiff(execVer, dataVer)
-}
-
 func getCurrentDeploymentID() (string, error) {
 	cmd := exec.Command("rpm-ostree", "status", "--jsonpath=$.deployments[0].id", "--booted")
 	var stdout, stderr bytes.Buffer
@@ -369,25 +306,6 @@ func getCurrentBootID() (string, error) {
 	return strings.ReplaceAll(strings.TrimSpace(string(content)), "-", ""), nil
 }
 
-func getHealthInfo() (*HealthInfo, error) {
-	if exists, err := util.PathExistsAndIsNotEmpty(healthFilepath); err != nil {
-		return nil, err
-	} else if !exists {
-		return nil, errHealthFileDoesNotExist
-	}
-
-	content, err := os.ReadFile(healthFilepath)
-	if err != nil {
-		return nil, fmt.Errorf("failed to read health data from %q: %w", healthFilepath, err)
-	}
-
-	health := &HealthInfo{}
-	if err := json.Unmarshal(content, &health); err != nil {
-		return nil, fmt.Errorf("failed to parse health data %q: %w", strings.TrimSpace(string(content)), err)
-	}
-	return health, nil
-}
-
 func getExistingBackupsForTheDeployment(existingBackups []data.BackupName, deployID string) []data.BackupName {
 	existingDeploymentBackups := make([]data.BackupName, 0)
 

pkg/admin/prerun/upgrade_blocking.go

@@ -10,21 +10,21 @@ import (
 	"k8s.io/klog/v2"
 )
 
-func IsUpgradeBlocked(execVersion versionMetadata, dataVersion versionMetadata) (bool, error) {
+func isUpgradeBlocked(execVersion versionMetadata, dataVersion versionMetadata) error {
 	buf, err := getBlockedUpgradesAsset()
 	if err != nil {
 		if errors.Is(err, fs.ErrNotExist) {
-			return false, nil
+			return nil
 		}
-		return false, fmt.Errorf("failed to load embedded blocked upgrades asset: %w", err)
+		return fmt.Errorf("failed to load embedded blocked upgrades asset: %w", err)
 	}
 
 	m, err := unmarshalBlockedUpgrades(buf)
 	if err != nil {
-		return false, err
+		return err
 	}
 
-	return isBlocked(m, execVersion.String(), dataVersion.String()), nil
+	return isBlocked(m, execVersion.String(), dataVersion.String())
 }
 
 func getBlockedUpgradesAsset() ([]byte, error) {
@@ -40,20 +40,21 @@ func unmarshalBlockedUpgrades(data []byte) (map[string][]string, error) {
 	return blockedEdges, nil
 }
 
-func isBlocked(blockedUpgrades map[string][]string, execVersion, dataVersion string) bool {
-	klog.InfoS("Checking if upgrade is allowed", "existing-data-version", dataVersion, "new-binary-version", execVersion, "blocked-upgrades", blockedUpgrades)
+func isBlocked(blockedUpgrades map[string][]string, execVersion, dataVersion string) error {
+	klog.InfoS("Checking if upgrade is allowed",
+		"existing-data-version", dataVersion,
+		"new-binary-version", execVersion,
+		"blocked-upgrades", blockedUpgrades)
 
 	for targetVersion, fromVersions := range blockedUpgrades {
 		if targetVersion == execVersion {
 			for _, from := range fromVersions {
 				if from == dataVersion {
-					klog.ErrorS(nil, "Detected an attempt of unsupported upgrade", "existing-data-version", dataVersion, "new-binary-version", execVersion)
-					return true
+					return fmt.Errorf("upgrade from %q to %q is blocked", dataVersion, execVersion)
 				}
 			}
 		}
 	}
 
-	klog.InfoS("Upgrade is allowed", "existing-data-version", dataVersion, "new-binary-version", execVersion)
-	return false
+	return nil
 }

pkg/admin/prerun/upgrade_blocking_test.go

@@ -35,43 +35,47 @@ func Test_IsBlocked(t *testing.T) {
 	}
 
 	testData := []struct {
-		dataVersion    string
-		execVersion    string
-		expectedResult bool
+		dataVersion string
+		execVersion string
+		errExpected bool
 	}{
 		{
-			dataVersion:    "4.14.4",
-			execVersion:    "4.14.10",
-			expectedResult: false,
+			dataVersion: "4.14.4",
+			execVersion: "4.14.10",
+			errExpected: false,
 		},
 		{
-			dataVersion:    "4.14.5",
-			execVersion:    "4.14.10",
-			expectedResult: true,
+			dataVersion: "4.14.5",
+			execVersion: "4.14.10",
+			errExpected: true,
 		},
 		{
-			dataVersion:    "4.14.6",
-			execVersion:    "4.14.10",
-			expectedResult: true,
+			dataVersion: "4.14.6",
+			execVersion: "4.14.10",
+			errExpected: true,
 		},
 		{
-			dataVersion:    "4.14.7",
-			execVersion:    "4.14.10",
-			expectedResult: false,
+			dataVersion: "4.14.7",
+			execVersion: "4.14.10",
+			errExpected: false,
 		},
 		{
-			dataVersion:    "4.14.7",
-			execVersion:    "4.15.0",
-			expectedResult: false,
+			dataVersion: "4.14.7",
+			execVersion: "4.15.0",
+			errExpected: false,
 		},
 		{
-			dataVersion:    "4.15.2",
-			execVersion:    "4.15.5",
-			expectedResult: true,
+			dataVersion: "4.15.2",
+			execVersion: "4.15.5",
+			errExpected: true,
 		},
 	}
 
 	for _, td := range testData {
-		assert.Equal(t, td.expectedResult, isBlocked(edges, td.execVersion, td.dataVersion))
+		if td.errExpected {
+			assert.Error(t, isBlocked(edges, td.execVersion, td.dataVersion))
+		} else {
+			assert.NoError(t, isBlocked(edges, td.execVersion, td.dataVersion))
+		}
 	}
 }