diff --git a/pkg/admin/prerun/health.go b/pkg/admin/prerun/health.go new file mode 100644 index 0000000000..4b83b823c8 --- /dev/null +++ b/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 +} diff --git a/pkg/admin/prerun/prerun.go b/pkg/admin/prerun/prerun.go index 117348cf8f..f9a29329de 100644 --- a/pkg/admin/prerun/prerun.go +++ b/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) diff --git a/pkg/admin/prerun/upgrade_blocking.go b/pkg/admin/prerun/upgrade_blocking.go index 696b4cee88..5c8bbd82c2 100644 --- a/pkg/admin/prerun/upgrade_blocking.go +++ b/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 } diff --git a/pkg/admin/prerun/upgrade_blocking_test.go b/pkg/admin/prerun/upgrade_blocking_test.go index e9a0ced5b9..fc3c6683e7 100644 --- a/pkg/admin/prerun/upgrade_blocking_test.go +++ b/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)) + } } } diff --git a/pkg/admin/prerun/version.go b/pkg/admin/prerun/version.go index 3219a139e3..29394ff5b9 100644 --- a/pkg/admin/prerun/version.go +++ b/pkg/admin/prerun/version.go @@ -20,44 +20,34 @@ var ( errDataVersionDoesNotExist = errors.New("version file for MicroShift data does not exist") ) -// CreateOrValidateDataVersion creates or compares data version against executable's version -// -// Function is intended to be invoked by main MicroShift run procedure, just before starting, -// to ensure that storage migration (which should update the version file) was performed. -func CreateOrValidateDataVersion() error { - dataVer, err := getVersionOfData() +// CheckAndUpdateDataVersion checks version skew between data and executable, +// and updates data version +func CheckAndUpdateDataVersion() error { + execVer, err := getVersionOfExecutable() if err != nil { - if errors.Is(err, errDataVersionDoesNotExist) { - // First run of MicroShift, create version file shortly after creating DataDir - return writeExecVersionToData() - } - return err + return fmt.Errorf("failed to get version of MicroShift executable: %w", err) } - execVer, err := getVersionOfExecutable() + dataVer, err := getVersionOfData() if err != nil { - return err - } - klog.InfoS("Comparing versions of MicroShift data on disk and executable", "data", dataVer, "exec", execVer) + if !errors.Is(err, errDataVersionDoesNotExist) { + return fmt.Errorf("failed to get version of existing MicroShift data: %w", err) + } + klog.InfoS("Version file does not exist yet") + } else { + if err := checkVersionCompatibility(execVer, dataVer); err != nil { + return fmt.Errorf("checking version skew failed: %w", err) + } - if execVer != dataVer { - return fmt.Errorf("data version (%s) does not match binary version (%s) - missing migration?", dataVer, execVer) + if err := isUpgradeBlocked(execVer, dataVer); err != nil { + return err + } } - return nil -} - -func writeExecVersionToData() error { - execVer, err := getVersionOfExecutable() - if err != nil { - return err + if err := writeDataVersion(execVer); err != nil { + return fmt.Errorf("failed to update data version: %w", err) } - version := execVer.String() - klog.InfoS("Writing MicroShift version to the file in data directory", "version", version) - if err := os.WriteFile(versionFilePath, []byte(version), 0600); err != nil { - return fmt.Errorf("writing '%s' to %s failed: %w", version, versionFilePath, err) - } return nil } @@ -118,28 +108,37 @@ func getVersionOfData() (versionMetadata, error) { return versionMetadataFromString(string(versionFileContents)) } -// checkVersionDiff compares versions of executable and existing data for purposes of data migration. -// It returns true if the migration should be performed. -func checkVersionDiff(execVer, dataVer versionMetadata) (bool, error) { +// checkVersionCompatibility compares versions of executable and existing data for purposes of data migration. +func checkVersionCompatibility(execVer, dataVer versionMetadata) error { if execVer == dataVer { - return false, nil + return nil } if execVer.Major != dataVer.Major { - return false, fmt.Errorf("major versions are different: %d and %d", dataVer.Major, execVer.Major) + return fmt.Errorf("major versions are different: %d and %d", dataVer.Major, execVer.Major) } if execVer.Minor < dataVer.Minor { - return false, fmt.Errorf("executable (%s) is older than existing data (%s): migrating data to older version is not supported", execVer.String(), dataVer.String()) + return fmt.Errorf("executable (%s) is older than existing data (%s): migrating data to older version is not supported", execVer.String(), dataVer.String()) } if execVer.Minor > dataVer.Minor { if execVer.Minor-1 == dataVer.Minor { - return true, nil + return nil } else { - return false, fmt.Errorf("executable (%s) is too recent compared to existing data (%s): maximum minor version difference is 1", execVer.String(), dataVer.String()) + return fmt.Errorf("executable (%s) is too recent compared to existing data (%s): maximum minor version difference is 1", execVer.String(), dataVer.String()) } } - return IsUpgradeBlocked(execVer, dataVer) + return nil +} + +func writeDataVersion(v versionMetadata) error { + s := v.String() + klog.InfoS("Writing MicroShift version to the file in data directory", "version", s) + + if err := os.WriteFile(versionFilePath, []byte(s), 0600); err != nil { + return fmt.Errorf("writing %q to %q failed: %w", s, versionFilePath, err) + } + return nil } diff --git a/pkg/admin/prerun/version_test.go b/pkg/admin/prerun/version_test.go index e8bb978729..1fa0eb2343 100644 --- a/pkg/admin/prerun/version_test.go +++ b/pkg/admin/prerun/version_test.go @@ -8,55 +8,48 @@ import ( func TestCheckVersionDiff(t *testing.T) { testData := []struct { - name string - execVer versionMetadata - dataVer versionMetadata - expectedMigrationRequired bool - errExpected bool + name string + execVer versionMetadata + dataVer versionMetadata + errExpected bool }{ { - name: "equal versions: no migration, no error", - execVer: versionMetadata{Major: 4, Minor: 14}, - dataVer: versionMetadata{Major: 4, Minor: 14}, - expectedMigrationRequired: false, - errExpected: false, + name: "equal versions: no migration, no error", + execVer: versionMetadata{Major: 4, Minor: 14}, + dataVer: versionMetadata{Major: 4, Minor: 14}, + errExpected: false, }, { - name: "X versions must be the same", - execVer: versionMetadata{Major: 4, Minor: 14}, - dataVer: versionMetadata{Major: 5, Minor: 14}, - expectedMigrationRequired: false, - errExpected: true, + name: "X versions must be the same", + execVer: versionMetadata{Major: 4, Minor: 14}, + dataVer: versionMetadata{Major: 5, Minor: 14}, + errExpected: true, }, { - name: "binary must not be older than data", - execVer: versionMetadata{Major: 4, Minor: 14}, - dataVer: versionMetadata{Major: 4, Minor: 15}, - expectedMigrationRequired: false, - errExpected: true, + name: "binary must not be older than data", + execVer: versionMetadata{Major: 4, Minor: 14}, + dataVer: versionMetadata{Major: 4, Minor: 15}, + errExpected: true, }, { - name: "binary must be newer only by one minor version", - execVer: versionMetadata{Major: 4, Minor: 15}, - dataVer: versionMetadata{Major: 4, Minor: 14}, - expectedMigrationRequired: true, - errExpected: false, + name: "binary must be newer only by one minor version", + execVer: versionMetadata{Major: 4, Minor: 15}, + dataVer: versionMetadata{Major: 4, Minor: 14}, + errExpected: false, }, { - name: "binary newer more than one minor version is not supported", - execVer: versionMetadata{Major: 4, Minor: 15}, - dataVer: versionMetadata{Major: 4, Minor: 13}, - expectedMigrationRequired: false, - errExpected: true, + name: "binary newer more than one minor version is not supported", + execVer: versionMetadata{Major: 4, Minor: 15}, + dataVer: versionMetadata{Major: 4, Minor: 13}, + errExpected: true, }, } for _, td := range testData { td := td t.Run(td.name, func(t *testing.T) { - migrationRequired, err := checkVersionDiff(td.execVer, td.dataVer) + err := checkVersionCompatibility(td.execVer, td.dataVer) - assert.Equal(t, td.expectedMigrationRequired, migrationRequired) if td.errExpected { assert.Error(t, err) } else { diff --git a/pkg/cmd/run.go b/pkg/cmd/run.go index 24493a1b2a..7f3aeda7f2 100644 --- a/pkg/cmd/run.go +++ b/pkg/cmd/run.go @@ -119,7 +119,7 @@ func RunMicroshift(cfg *config.Config) error { return fmt.Errorf("failed to create dir %q: %w", config.DataDir, err) } - if err := prerun.CreateOrValidateDataVersion(); err != nil { + if err := prerun.CheckAndUpdateDataVersion(); err != nil { return err }