From 049e41cb3cba7bc3867d3cb7562c98d6a6bb6457 Mon Sep 17 00:00:00 2001 From: Doug Hellmann Date: Wed, 7 Jun 2023 18:20:28 -0400 Subject: [PATCH 1/3] standardize logging and error messages in prerun module --- pkg/admin/data/data_manager.go | 78 ++++++++++++--------- pkg/admin/prerun/prerun.go | 119 ++++++++++++++++++++++----------- 2 files changed, 128 insertions(+), 69 deletions(-) diff --git a/pkg/admin/data/data_manager.go b/pkg/admin/data/data_manager.go index f69a67c0c5..96ddbeecfc 100644 --- a/pkg/admin/data/data_manager.go +++ b/pkg/admin/data/data_manager.go @@ -10,6 +10,7 @@ import ( "github.com/openshift/microshift/pkg/config" "github.com/openshift/microshift/pkg/util" + "github.com/pkg/errors" "k8s.io/klog/v2" ) @@ -44,7 +45,16 @@ func (dm *manager) BackupExists(name BackupName) (bool, error) { } func (dm *manager) RemoveBackup(name BackupName) error { - return os.RemoveAll(dm.GetBackupPath(name)) + klog.InfoS("Removing backup", + "name", name, + ) + if err := os.RemoveAll(dm.GetBackupPath(name)); err != nil { + return errors.Wrap(err, fmt.Sprintf("Failed to delete backup %q", name)) + } + klog.InfoS("Removed backup", + "name", name, + ) + return nil } func (dm *manager) GetBackupList() ([]BackupName, error) { @@ -64,29 +74,31 @@ func (dm *manager) GetBackupList() ([]BackupName, error) { } func (dm *manager) Backup(name BackupName) error { - klog.InfoS("Backing up the data", - "storage", dm.storage, "name", name, "data", config.DataDir) + klog.InfoS("Starting backup", + "storage", dm.storage, + "name", name, + "data", config.DataDir, + ) if name == "" { return &EmptyArgErr{"name"} } if exists, err := dm.BackupExists(name); err != nil { - return fmt.Errorf("checking if backup %s exists failed: %w", name, err) + return errors.Wrap(err, fmt.Sprintf("Failed to determine if backup %s exists", name)) } else if exists { - klog.ErrorS(nil, "Backup already exists - name should be unique", "name", name) - return fmt.Errorf("backup %s already exists", name) + return fmt.Errorf("Failed to create backup destination %q because it already exists", name) } if found, err := pathExists(string(dm.storage)); err != nil { - return err + return errors.Wrap(err, fmt.Sprintf("Failed to determine if storage location %q exists", dm.storage)) } else if !found { if makeDirErr := util.MakeDir(string(dm.storage)); makeDirErr != nil { - return fmt.Errorf("making %s directory failed: %w", dm.storage, makeDirErr) + return errors.Wrap(makeDirErr, fmt.Sprintf("Failed to create backup storage directory %q", dm.storage)) } - klog.InfoS("Backup storage directory created", "path", dm.storage) + klog.InfoS("Created backup storage directory", "path", dm.storage) } else { - klog.InfoS("Backup storage directory already existed", "path", dm.storage) + klog.InfoS("Found existing backup storage directory", "path", dm.storage) } dest := dm.GetBackupPath(name) @@ -100,73 +112,79 @@ func (dm *manager) Backup(name BackupName) error { } func (dm *manager) Restore(name BackupName) error { - klog.InfoS("Restoring the data", "storage", dm.storage, "name", name, "data", config.DataDir) + klog.InfoS("Starting restore", + "storage", dm.storage, + "name", name, + "data", config.DataDir, + ) if name == "" { return &EmptyArgErr{"name"} } if exists, err := dm.BackupExists(name); err != nil { - return fmt.Errorf("checking if backup %s exists failed: %w", name, err) + return errors.Wrap(err, fmt.Sprintf("Failed to determine if backup %q exists failed", name)) } else if !exists { - return fmt.Errorf("failed to restore backup, %s does not exist", name) + return fmt.Errorf("Failed to restore backup, %q does not exist", name) } tmp := fmt.Sprintf("%s.saved", config.DataDir) - klog.InfoS("Temporarily renaming data dir", "data", config.DataDir, "renamedTo", tmp) + klog.InfoS("Renaming existing data dir", "data", config.DataDir, "renamedTo", tmp) if err := os.Rename(config.DataDir, tmp); err != nil { - return fmt.Errorf("renaming %s to %s failed: %w", config.DataDir, tmp, err) + return errors.Wrap(err, fmt.Sprintf("Failed to rename existing data directory %q to %q", config.DataDir, tmp)) } src := dm.GetBackupPath(name) if err := copyPath(src, config.DataDir); err != nil { - klog.ErrorS(err, "Copying backup failed - restoring previous data dir") + klog.ErrorS(err, "Failed to restore from backup, restoring current data dir") if err := os.RemoveAll(config.DataDir); err != nil { - return fmt.Errorf("removing %s failed: %w", config.DataDir, err) + return errors.Wrap(err, fmt.Sprintf("Failed to remove data directory %q", config.DataDir)) } if err := os.Rename(tmp, config.DataDir); err != nil { - return fmt.Errorf("renaming %s to %s failed: %w", tmp, config.DataDir, err) + return errors.Wrap(err, fmt.Sprintf("Failed to rename temporary directory %q to %q", tmp, config.DataDir)) } - return fmt.Errorf("restoring backup %s failed: %w", src, err) + return errors.Wrap(err, "Failed to restore backup") } - klog.InfoS("Removing temporary data dir", "path", tmp) + klog.InfoS("Removing temporary data directory", "path", tmp) if err := os.RemoveAll(tmp); err != nil { - klog.ErrorS(err, "Failed to remove path", "path", tmp) + klog.ErrorS(err, "Failed to remove temporary data directory, leaving in place", "path", tmp) } - klog.InfoS("Restore finished", "backup", src, "data", config.DataDir) + klog.InfoS("Finished restore", + "name", name, + "data", config.DataDir, + ) return nil } func copyPath(src, dest string) error { cmd := exec.Command("cp", append(cpArgs, src, dest)...) //nolint:gosec - klog.InfoS("Executing command", "cmd", cmd) + klog.InfoS("Starting copy", "cmd", cmd) var outb, errb bytes.Buffer cmd.Stdout = &outb cmd.Stderr = &errb err := cmd.Run() - klog.InfoS("Command finished running", "cmd", cmd, - "stdout", strings.ReplaceAll(outb.String(), "\n", `, `), - "stderr", errb.String()) - if err != nil { - return fmt.Errorf("command %s failed: %w", cmd, err) + klog.InfoS("Failed to copy", "cmd", cmd, + "stdout", strings.ReplaceAll(outb.String(), "\n", `, `), + "stderr", errb.String()) + return errors.Wrap(err, fmt.Sprintf("Failed to copy %q to %q", src, dest)) } - klog.InfoS("Command successful", "cmd", cmd) + klog.InfoS("Finished copy", "cmd", cmd) return nil } func pathExists(path string) (bool, error) { exists, err := util.PathExists(path) if err != nil { - return false, fmt.Errorf("checking if %s exists failed: %w", path, err) + return false, errors.Wrap(err, fmt.Sprintf("failed to check if %q exists", path)) } return exists, nil } diff --git a/pkg/admin/prerun/prerun.go b/pkg/admin/prerun/prerun.go index 0679a86e90..31e8158352 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" @@ -11,6 +10,7 @@ import ( "github.com/openshift/microshift/pkg/admin/data" "github.com/openshift/microshift/pkg/util" + "github.com/pkg/errors" "k8s.io/klog/v2" ) @@ -43,38 +43,54 @@ func New(dataManager data.Manager) *PreRun { } func (pr *PreRun) Perform() error { + klog.InfoS("Starting pre-run") + health, err := getHealthInfo() if err != nil { if errors.Is(err, errHealthFileDoesNotExist) { - klog.InfoS("Health file does not exist - skipping backup") + klog.InfoS("Skipping pre-run: Health status file does not exist") return nil } - return err + return errors.Wrap(err, "Failed to determine the current system health") } - if isCurr, err := containsCurrentBootID(health.BootID); err != nil { - return err - } else if isCurr { + currentBootID, err := getCurrentBootID() + if err != nil { + return errors.Wrap(err, "Failed to determine the current boot ID") + } + + klog.InfoS("Found boot details", + "health", health.Health, + "deploymentID", health.DeploymentID, + "previousBootID", health.BootID, + "currentBootID", currentBootID, + ) + + if currentBootID == health.BootID { // This might happen if microshift is (re)started after greenboot finishes running. // Green script will overwrite the health.json with // current boot's ID, deployment ID, and health. - klog.InfoS("Health file contains current boot - skipping pre-run") + klog.InfoS("Skipping pre-run: Health file refers to the current boot ID") return nil } - klog.InfoS("Previous boot", "health", health.Health, "deploymentID", health.DeploymentID, "bootID", health.BootID) - + // TODO: We may end up needing to split this if statement into + // functions, but for now let's just tell the linter not to apply + // the rule. + // + //nolint:nestif if health.IsHealthy() { + klog.Info("Previous boot was healthy") if err := pr.backup(health); err != nil { - return err + return errors.Wrap(err, "Failed to backup during pre-run") } migrationNeeded, err := pr.checkVersions() if err != nil { - return err + return errors.Wrap(err, "Failed version checks") } - klog.InfoS("Version checks successful", "is-migration-needed?", migrationNeeded) + klog.InfoS("Completed version checks", "is-migration-needed?", migrationNeeded) if migrationNeeded { _ = migrationNeeded @@ -82,17 +98,27 @@ func (pr *PreRun) Perform() error { } return nil + } else { + klog.Info("Previous boot was not healthy") + if err = pr.restore(); err != nil { + return errors.Wrap(err, "Failed to restore during pre-run") + } } - return pr.restore() + klog.InfoS("Finished pre-run") + return nil } func (pr *PreRun) backup(health *HealthInfo) error { - klog.InfoS("Backing up the data for deployment", "deployment", health.DeploymentID) + newBackupName := health.BackupName() + klog.InfoS("Preparing to backup", + "deploymentID", health.DeploymentID, + "name", newBackupName, + ) existingBackups, err := pr.dataManager.GetBackupList() if err != nil { - return err + return errors.Wrap(err, "Failed to determine the existing backups") } // get list of already existing backups for deployment ID persisted in health file @@ -100,39 +126,48 @@ func (pr *PreRun) backup(health *HealthInfo) error { // (so only the most recent one for specific deployment is kept) backupsForDeployment := getExistingBackupsForTheDeployment(existingBackups, health.DeploymentID) - newBackupName := health.BackupName() if backupAlreadyExists(backupsForDeployment, newBackupName) { - klog.InfoS("Backup already exists", "name", newBackupName) + klog.InfoS("Skipping backup: Backup already exists", + "deploymentID", health.DeploymentID, + "name", newBackupName, + ) return nil } if err := pr.dataManager.Backup(newBackupName); err != nil { - return err + return errors.Wrap(err, fmt.Sprintf("Failed to create new backup %q", newBackupName)) } pr.removeOldBackups(backupsForDeployment) + klog.InfoS("Finished backup", + "deploymentID", health.DeploymentID, + "destination", newBackupName, + ) return nil } func (pr *PreRun) restore() error { // TODO: Check if containers are already running (i.e. microshift.service was restarted)? + klog.Info("Preparing to restore") currentDeploymentID, err := getCurrentDeploymentID() if err != nil { return err } - klog.InfoS("Restoring data for current deployment", "deployID", currentDeploymentID) existingBackups, err := pr.dataManager.GetBackupList() if err != nil { - return err + return errors.Wrap(err, "Failed to determine the existing backups") } - klog.InfoS("List of existing backups", "backups", existingBackups) + klog.InfoS("Found existing backups", + "currentDeploymentID", currentDeploymentID, + "backups", existingBackups, + ) backupsForDeployment := getExistingBackupsForTheDeployment(existingBackups, currentDeploymentID) if len(backupsForDeployment) == 0 { - return fmt.Errorf("there is no backup to restore for current deployment (%s)", currentDeploymentID) + return fmt.Errorf("there is no backup to restore for current deployment %q", currentDeploymentID) } if len(backupsForDeployment) > 1 { @@ -140,7 +175,13 @@ func (pr *PreRun) restore() error { klog.InfoS("TODO: more than 1 backup, need to pick most recent one") } - return pr.dataManager.Restore(backupsForDeployment[0]) + err = pr.dataManager.Restore(backupsForDeployment[0]) + if err != nil { + return errors.Wrap(err, "Failed to restore backup") + } + + klog.Info("Finished restore") + return nil } // checkVersions compares version of data and executable @@ -148,9 +189,10 @@ func (pr *PreRun) restore() error { // 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, err + return false, errors.Wrap(err, "Failed to determine the active version of the MicroShift") } dataVer, err := getVersionOfData() @@ -160,10 +202,10 @@ func (pr *PreRun) checkVersions() (bool, error) { // TODO: 4.13 return true, nil } - return false, err + return false, errors.Wrap(err, "Failed to determine the version of the existing data") } - klog.InfoS("Checking version difference between data and executable", "data", dataVer, "exec", execVer) + klog.InfoS("Comparing versions", "data", dataVer, "active", execVer) return checkVersionDiff(execVer, dataVer) } @@ -175,45 +217,44 @@ func getCurrentDeploymentID() (string, error) { cmd.Stderr = &stderr if err := cmd.Run(); err != nil { - return "", fmt.Errorf("command %s failed: %s", strings.TrimSpace(cmd.String()), strings.TrimSpace(stderr.String())) + return "", fmt.Errorf("Failed to determine the rpm-ostree deployment id, command %q failed: %s", strings.TrimSpace(cmd.String()), strings.TrimSpace(stderr.String())) } ids := []string{} if err := json.Unmarshal(stdout.Bytes(), &ids); err != nil { - return "", fmt.Errorf("unmarshalling '%s' to json failed: %w", strings.TrimSpace(stdout.String()), err) + return "", errors.Wrap(err, fmt.Sprintf("Failed to determine the rpm-ostree deployment id from %q", strings.TrimSpace(stdout.String()))) } if len(ids) != 1 { // this shouldn't happen if running on ostree system, but just in case - klog.ErrorS(nil, "Unexpected amount of deployments in rpm-ostree output", + klog.ErrorS(nil, "Unexpected number of deployments in rpm-ostree output", "cmd", cmd.String(), "stdout", strings.TrimSpace(stdout.String()), "stderr", strings.TrimSpace(stderr.String()), "unmarshalledIDs", ids) - return "", fmt.Errorf("rpm-ostree returned unexpected amount of deployment IDs: %d", len(ids)) + return "", fmt.Errorf("Expected 1 deployment ID, rpm-ostree returned %d", len(ids)) } return ids[0], nil } func (pr *PreRun) removeOldBackups(backups []data.BackupName) { + klog.Info("Preparing to prune backups") for _, b := range backups { - klog.InfoS("Removing older backup", "name", b) if err := pr.dataManager.RemoveBackup(b); err != nil { - klog.ErrorS(err, "Failed to remove backup", "name", b) + klog.ErrorS(err, "Failed to remove backup, ignoring", "name", b) } } + klog.Info("Finished pruning backups") } -func containsCurrentBootID(id string) (bool, error) { +func getCurrentBootID() (string, error) { path := "/proc/sys/kernel/random/boot_id" content, err := os.ReadFile(path) if err != nil { - return false, fmt.Errorf("reading file %s failed: %w", path, err) + return "", errors.Wrap(err, fmt.Sprintf("Failed to determine boot ID from %s", path)) } - currentBootID := strings.ReplaceAll(strings.TrimSpace(string(content)), "-", "") - klog.InfoS("Comparing boot IDs", "current", currentBootID, "toCompare", id) - return id == currentBootID, nil + return strings.ReplaceAll(strings.TrimSpace(string(content)), "-", ""), nil } func getHealthInfo() (*HealthInfo, error) { @@ -226,12 +267,12 @@ func getHealthInfo() (*HealthInfo, error) { content, err := os.ReadFile(path) if err != nil { - return nil, fmt.Errorf("reading file %s failed: %w", path, err) + return nil, errors.Wrap(err, fmt.Sprintf("Failed to read health data from %q", path)) } health := &HealthInfo{} if err := json.Unmarshal(content, &health); err != nil { - return nil, fmt.Errorf("unmarshalling '%s' failed: %w", strings.TrimSpace(string(content)), err) + return nil, errors.Wrap(err, fmt.Sprintf("Failed to parse health data %q", strings.TrimSpace(string(content)))) } return health, nil } From 50a49aaf3481c0476eaa5cd2d9c9135bcfd90277 Mon Sep 17 00:00:00 2001 From: Doug Hellmann Date: Mon, 12 Jun 2023 06:52:35 -0400 Subject: [PATCH 2/3] remove use of deprecated package github.com/pkg/errors github.com/pkg/errors is no longer maintained. Switch to using %w verb in fmt.Errorf() instead. --- .../openshift/microshift/pkg/util/cert.go | 11 +++--- .../openshift/microshift/pkg/util/net.go | 7 ++-- pkg/admin/data/data_manager.go | 30 +++++++++------- pkg/admin/prerun/prerun.go | 34 +++++++++---------- pkg/loadbalancerservice/controller.go | 7 ++-- pkg/mdns/routes.go | 8 ++--- pkg/util/cert.go | 11 +++--- pkg/util/net.go | 7 ++-- 8 files changed, 61 insertions(+), 54 deletions(-) diff --git a/etcd/vendor/github.com/openshift/microshift/pkg/util/cert.go b/etcd/vendor/github.com/openshift/microshift/pkg/util/cert.go index 45bfab6d67..e3cd3122c6 100644 --- a/etcd/vendor/github.com/openshift/microshift/pkg/util/cert.go +++ b/etcd/vendor/github.com/openshift/microshift/pkg/util/cert.go @@ -23,7 +23,6 @@ import ( "fmt" "os" - "github.com/pkg/errors" "k8s.io/client-go/util/keyutil" ) @@ -41,12 +40,12 @@ func EnsureKeyPair(pubKeyPath, privKeyPath string) error { func GenKeys(pubPath, keyPath string) error { rsaKey, err := rsa.GenerateKey(rand.Reader, keySize) if err != nil { - return errors.Wrap(err, "error generating RSA private key") + return fmt.Errorf("failed to generate RSA private key: %w", err) } keyPEM, err := keyutil.MarshalPrivateKeyToPEM(rsaKey) if err != nil { - return fmt.Errorf("failed to encode private key to PEM: %v", err) + return fmt.Errorf("failed to encode private key to PEM: %w", err) } pubPEM, err := PublicKeyToPem(&rsaKey.PublicKey) @@ -55,11 +54,11 @@ func GenKeys(pubPath, keyPath string) error { } if err := keyutil.WriteKey(keyPath, keyPEM); err != nil { - return fmt.Errorf("failed to write the private key to %s: %v", keyPath, err) + return fmt.Errorf("failed to write the private key to %s: %w", keyPath, err) } if err := os.WriteFile(pubPath, pubPEM, 0400); err != nil { - return fmt.Errorf("failed to write public key to %s: %v", pubPath, err) + return fmt.Errorf("failed to write public key to %s: %w", pubPath, err) } return nil @@ -69,7 +68,7 @@ func GenKeys(pubPath, keyPath string) error { func PublicKeyToPem(key *rsa.PublicKey) ([]byte, error) { keyInBytes, err := x509.MarshalPKIXPublicKey(key) if err != nil { - return nil, errors.Wrap(err, "failed to MarshalPKIXPublicKey") + return nil, fmt.Errorf("failed to MarshalPKIXPublicKey: %w", err) } keyinPem := pem.EncodeToMemory( &pem.Block{ diff --git a/etcd/vendor/github.com/openshift/microshift/pkg/util/net.go b/etcd/vendor/github.com/openshift/microshift/pkg/util/net.go index 4985cd8c47..3c1ade909d 100644 --- a/etcd/vendor/github.com/openshift/microshift/pkg/util/net.go +++ b/etcd/vendor/github.com/openshift/microshift/pkg/util/net.go @@ -18,6 +18,7 @@ package util import ( "context" "crypto/tls" + "fmt" tcpnet "net" "net/http" "os" @@ -27,7 +28,6 @@ import ( "time" "github.com/openshift/microshift/pkg/config/ovn" - "github.com/pkg/errors" "k8s.io/apimachinery/pkg/util/net" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/klog/v2" @@ -123,7 +123,10 @@ func AddToNoProxyEnv(additionalEntries ...string) error { // unset the lower-case one, and keep only upper-case os.Unsetenv("no_proxy") - return errors.Wrap(os.Setenv("NO_PROXY", noProxyEnv), "error updating NO_PROXY") + if err := os.Setenv("NO_PROXY", noProxyEnv); err != nil { + return fmt.Errorf("failed to update NO_PROXY: %w", err) + } + return nil } func mapKeys(entries map[string]struct{}) []string { diff --git a/pkg/admin/data/data_manager.go b/pkg/admin/data/data_manager.go index 96ddbeecfc..7af7a6e43f 100644 --- a/pkg/admin/data/data_manager.go +++ b/pkg/admin/data/data_manager.go @@ -10,7 +10,6 @@ import ( "github.com/openshift/microshift/pkg/config" "github.com/openshift/microshift/pkg/util" - "github.com/pkg/errors" "k8s.io/klog/v2" ) @@ -49,7 +48,7 @@ func (dm *manager) RemoveBackup(name BackupName) error { "name", name, ) if err := os.RemoveAll(dm.GetBackupPath(name)); err != nil { - return errors.Wrap(err, fmt.Sprintf("Failed to delete backup %q", name)) + return fmt.Errorf("Failed to delete backup %q: %w", name, err) } klog.InfoS("Removed backup", "name", name, @@ -85,16 +84,19 @@ func (dm *manager) Backup(name BackupName) error { } if exists, err := dm.BackupExists(name); err != nil { - return errors.Wrap(err, fmt.Sprintf("Failed to determine if backup %s exists", name)) + return fmt.Errorf("Failed to determine if backup %q exists: %w", name, err) } else if exists { - return fmt.Errorf("Failed to create backup destination %q because it already exists", name) + return fmt.Errorf("Failed to create backup destination %q because it already exists", + name) } if found, err := pathExists(string(dm.storage)); err != nil { - return errors.Wrap(err, fmt.Sprintf("Failed to determine if storage location %q exists", dm.storage)) + return fmt.Errorf("Failed to determine if storage location %q for backup exists: %w", + dm.storage, err) } else if !found { if makeDirErr := util.MakeDir(string(dm.storage)); makeDirErr != nil { - return errors.Wrap(makeDirErr, fmt.Sprintf("Failed to create backup storage directory %q", dm.storage)) + return fmt.Errorf("Failed to create backup storage directory %q: %w", + dm.storage, makeDirErr) } klog.InfoS("Created backup storage directory", "path", dm.storage) } else { @@ -123,7 +125,7 @@ func (dm *manager) Restore(name BackupName) error { } if exists, err := dm.BackupExists(name); err != nil { - return errors.Wrap(err, fmt.Sprintf("Failed to determine if backup %q exists failed", name)) + return fmt.Errorf("Failed to determine if backup %q exists: %w", name, err) } else if !exists { return fmt.Errorf("Failed to restore backup, %q does not exist", name) } @@ -131,7 +133,8 @@ func (dm *manager) Restore(name BackupName) error { tmp := fmt.Sprintf("%s.saved", config.DataDir) klog.InfoS("Renaming existing data dir", "data", config.DataDir, "renamedTo", tmp) if err := os.Rename(config.DataDir, tmp); err != nil { - return errors.Wrap(err, fmt.Sprintf("Failed to rename existing data directory %q to %q", config.DataDir, tmp)) + return fmt.Errorf("Failed to rename existing data directory %q to %q: %w", + config.DataDir, tmp, err) } src := dm.GetBackupPath(name) @@ -139,14 +142,15 @@ func (dm *manager) Restore(name BackupName) error { klog.ErrorS(err, "Failed to restore from backup, restoring current data dir") if err := os.RemoveAll(config.DataDir); err != nil { - return errors.Wrap(err, fmt.Sprintf("Failed to remove data directory %q", config.DataDir)) + return fmt.Errorf("Failed to remove data directory %q: %w", config.DataDir, err) } if err := os.Rename(tmp, config.DataDir); err != nil { - return errors.Wrap(err, fmt.Sprintf("Failed to rename temporary directory %q to %q", tmp, config.DataDir)) + return fmt.Errorf("Failed to rename temporary directory %q to %q: %w", + tmp, config.DataDir, err) } - return errors.Wrap(err, "Failed to restore backup") + return fmt.Errorf("Failed to restore backup: %w", err) } klog.InfoS("Removing temporary data directory", "path", tmp) @@ -174,7 +178,7 @@ func copyPath(src, dest string) error { klog.InfoS("Failed to copy", "cmd", cmd, "stdout", strings.ReplaceAll(outb.String(), "\n", `, `), "stderr", errb.String()) - return errors.Wrap(err, fmt.Sprintf("Failed to copy %q to %q", src, dest)) + return fmt.Errorf("Failed to copy %q to %q: %w", src, dest, err) } klog.InfoS("Finished copy", "cmd", cmd) @@ -184,7 +188,7 @@ func copyPath(src, dest string) error { func pathExists(path string) (bool, error) { exists, err := util.PathExists(path) if err != nil { - return false, errors.Wrap(err, fmt.Sprintf("failed to check if %q exists", path)) + return false, fmt.Errorf("Failed to check if path %q exists: %w", path, err) } return exists, nil } diff --git a/pkg/admin/prerun/prerun.go b/pkg/admin/prerun/prerun.go index 31e8158352..0a36b9be2e 100644 --- a/pkg/admin/prerun/prerun.go +++ b/pkg/admin/prerun/prerun.go @@ -3,6 +3,7 @@ package prerun import ( "bytes" "encoding/json" + "errors" "fmt" "os" "os/exec" @@ -10,7 +11,6 @@ import ( "github.com/openshift/microshift/pkg/admin/data" "github.com/openshift/microshift/pkg/util" - "github.com/pkg/errors" "k8s.io/klog/v2" ) @@ -51,12 +51,12 @@ func (pr *PreRun) Perform() error { klog.InfoS("Skipping pre-run: Health status file does not exist") return nil } - return errors.Wrap(err, "Failed to determine the current system health") + return fmt.Errorf("Failed to determine the current system health: %w", err) } currentBootID, err := getCurrentBootID() if err != nil { - return errors.Wrap(err, "Failed to determine the current boot ID") + return fmt.Errorf("Failed to determine the current boot ID: %w", err) } klog.InfoS("Found boot details", @@ -82,12 +82,12 @@ func (pr *PreRun) Perform() error { if health.IsHealthy() { klog.Info("Previous boot was healthy") if err := pr.backup(health); err != nil { - return errors.Wrap(err, "Failed to backup during pre-run") + return fmt.Errorf("Failed to backup during pre-run: %w", err) } migrationNeeded, err := pr.checkVersions() if err != nil { - return errors.Wrap(err, "Failed version checks") + return fmt.Errorf("Failed version checks: %w", err) } klog.InfoS("Completed version checks", "is-migration-needed?", migrationNeeded) @@ -101,7 +101,7 @@ func (pr *PreRun) Perform() error { } else { klog.Info("Previous boot was not healthy") if err = pr.restore(); err != nil { - return errors.Wrap(err, "Failed to restore during pre-run") + return fmt.Errorf("Failed to restore during pre-run: %w", err) } } @@ -118,7 +118,7 @@ func (pr *PreRun) backup(health *HealthInfo) error { existingBackups, err := pr.dataManager.GetBackupList() if err != nil { - return errors.Wrap(err, "Failed to determine the existing backups") + return fmt.Errorf("Failed to determine the existing backups: %w", err) } // get list of already existing backups for deployment ID persisted in health file @@ -135,7 +135,7 @@ func (pr *PreRun) backup(health *HealthInfo) error { } if err := pr.dataManager.Backup(newBackupName); err != nil { - return errors.Wrap(err, fmt.Sprintf("Failed to create new backup %q", newBackupName)) + return fmt.Errorf("Failed to create new backup %q: %w", newBackupName, err) } pr.removeOldBackups(backupsForDeployment) @@ -158,7 +158,7 @@ func (pr *PreRun) restore() error { existingBackups, err := pr.dataManager.GetBackupList() if err != nil { - return errors.Wrap(err, "Failed to determine the existing backups") + return fmt.Errorf("Failed to determine the existing backups: %w", err) } klog.InfoS("Found existing backups", "currentDeploymentID", currentDeploymentID, @@ -177,7 +177,7 @@ func (pr *PreRun) restore() error { err = pr.dataManager.Restore(backupsForDeployment[0]) if err != nil { - return errors.Wrap(err, "Failed to restore backup") + return fmt.Errorf("Failed to restore backup: %w", err) } klog.Info("Finished restore") @@ -192,7 +192,7 @@ func (pr *PreRun) checkVersions() (bool, error) { klog.Info("Starting version checks") execVer, err := getVersionOfExecutable() if err != nil { - return false, errors.Wrap(err, "Failed to determine the active version of the MicroShift") + return false, fmt.Errorf("Failed to determine the active version of the MicroShift: %w", err) } dataVer, err := getVersionOfData() @@ -202,7 +202,7 @@ func (pr *PreRun) checkVersions() (bool, error) { // TODO: 4.13 return true, nil } - return false, errors.Wrap(err, "Failed to determine the version of the existing data") + return false, fmt.Errorf("Failed to determine the version of the existing data: %w", err) } klog.InfoS("Comparing versions", "data", dataVer, "active", execVer) @@ -217,12 +217,12 @@ func getCurrentDeploymentID() (string, error) { cmd.Stderr = &stderr if err := cmd.Run(); err != nil { - return "", fmt.Errorf("Failed to determine the rpm-ostree deployment id, command %q failed: %s", strings.TrimSpace(cmd.String()), strings.TrimSpace(stderr.String())) + return "", fmt.Errorf("Failed to determine the rpm-ostree deployment id, command %q failed: %s: %w", strings.TrimSpace(cmd.String()), strings.TrimSpace(stderr.String()), err) } ids := []string{} if err := json.Unmarshal(stdout.Bytes(), &ids); err != nil { - return "", errors.Wrap(err, fmt.Sprintf("Failed to determine the rpm-ostree deployment id from %q", strings.TrimSpace(stdout.String()))) + return "", fmt.Errorf("Failed to determine the rpm-ostree deployment id from %q: %w", strings.TrimSpace(stdout.String()), err) } if len(ids) != 1 { @@ -252,7 +252,7 @@ func getCurrentBootID() (string, error) { path := "/proc/sys/kernel/random/boot_id" content, err := os.ReadFile(path) if err != nil { - return "", errors.Wrap(err, fmt.Sprintf("Failed to determine boot ID from %s", path)) + return "", fmt.Errorf("Failed to determine boot ID from %q: %w", path, err) } return strings.ReplaceAll(strings.TrimSpace(string(content)), "-", ""), nil } @@ -267,12 +267,12 @@ func getHealthInfo() (*HealthInfo, error) { content, err := os.ReadFile(path) if err != nil { - return nil, errors.Wrap(err, fmt.Sprintf("Failed to read health data from %q", path)) + return nil, fmt.Errorf("Failed to read health data from %q: %w", path, err) } health := &HealthInfo{} if err := json.Unmarshal(content, &health); err != nil { - return nil, errors.Wrap(err, fmt.Sprintf("Failed to parse health data %q", strings.TrimSpace(string(content)))) + return nil, fmt.Errorf("Failed to parse health data %q: %w", strings.TrimSpace(string(content)), err) } return health, nil } diff --git a/pkg/loadbalancerservice/controller.go b/pkg/loadbalancerservice/controller.go index dda9504dc9..e969af86be 100644 --- a/pkg/loadbalancerservice/controller.go +++ b/pkg/loadbalancerservice/controller.go @@ -5,7 +5,6 @@ import ( "fmt" "time" - "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/informers" @@ -56,11 +55,11 @@ func (c *LoadbalancerServiceController) Run(ctx context.Context, ready chan<- st restCfg, err := c.restConfig() if err != nil { - return errors.Wrap(err, "error creating rest config for service controller") + return fmt.Errorf("failed to create rest config for service controller: %w", err) } c.client, err = kubernetes.NewForConfig(restCfg) if err != nil { - return errors.Wrap(err, "failed to create clientset for service controller") + return fmt.Errorf("failed to create clientset for service controller: %w", err) } klog.Infof("Starting service controller") @@ -91,7 +90,7 @@ func (c *LoadbalancerServiceController) Run(ctx context.Context, ready chan<- st }, }) if err != nil { - return errors.Wrap(err, "failed to initialize informer event handlers") + return fmt.Errorf("failed to initialize informer event handlers: %w", err) } factory.Start(stopCh) diff --git a/pkg/mdns/routes.go b/pkg/mdns/routes.go index 967a6e5999..525f7851dd 100644 --- a/pkg/mdns/routes.go +++ b/pkg/mdns/routes.go @@ -2,11 +2,11 @@ package mdns import ( "context" + "fmt" "strings" "time" "github.com/openshift/microshift/pkg/mdns/server" - "github.com/pkg/errors" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -30,12 +30,12 @@ func (c *MicroShiftmDNSController) startRouteInformer(stopCh chan struct{}) erro klog.Infof("Starting MicroShift mDNS route watcher") cfg, err := c.restConfig() if err != nil { - return errors.Wrap(err, "error creating rest config for route informer") + return fmt.Errorf("failed to create rest config for route informer: %w", err) } dc, err := dynamic.NewForConfig(cfg) if err != nil { - return errors.Wrap(err, "error creating dynamic informer") + return fmt.Errorf("failed to create dynamic informer for route controller: %w", err) } return c.run(stopCh, dc) @@ -55,7 +55,7 @@ func (c *MicroShiftmDNSController) run(stopCh chan struct{}, dc dynamic.Interfac } if _, err := informer.AddEventHandler(handlers); err != nil { - return errors.Wrap(err, "failed to initialize event handler: %w") + return fmt.Errorf("failed to initialize event handler for route controller: %w", err) } informer.Run(stopCh) diff --git a/pkg/util/cert.go b/pkg/util/cert.go index 45bfab6d67..e3cd3122c6 100644 --- a/pkg/util/cert.go +++ b/pkg/util/cert.go @@ -23,7 +23,6 @@ import ( "fmt" "os" - "github.com/pkg/errors" "k8s.io/client-go/util/keyutil" ) @@ -41,12 +40,12 @@ func EnsureKeyPair(pubKeyPath, privKeyPath string) error { func GenKeys(pubPath, keyPath string) error { rsaKey, err := rsa.GenerateKey(rand.Reader, keySize) if err != nil { - return errors.Wrap(err, "error generating RSA private key") + return fmt.Errorf("failed to generate RSA private key: %w", err) } keyPEM, err := keyutil.MarshalPrivateKeyToPEM(rsaKey) if err != nil { - return fmt.Errorf("failed to encode private key to PEM: %v", err) + return fmt.Errorf("failed to encode private key to PEM: %w", err) } pubPEM, err := PublicKeyToPem(&rsaKey.PublicKey) @@ -55,11 +54,11 @@ func GenKeys(pubPath, keyPath string) error { } if err := keyutil.WriteKey(keyPath, keyPEM); err != nil { - return fmt.Errorf("failed to write the private key to %s: %v", keyPath, err) + return fmt.Errorf("failed to write the private key to %s: %w", keyPath, err) } if err := os.WriteFile(pubPath, pubPEM, 0400); err != nil { - return fmt.Errorf("failed to write public key to %s: %v", pubPath, err) + return fmt.Errorf("failed to write public key to %s: %w", pubPath, err) } return nil @@ -69,7 +68,7 @@ func GenKeys(pubPath, keyPath string) error { func PublicKeyToPem(key *rsa.PublicKey) ([]byte, error) { keyInBytes, err := x509.MarshalPKIXPublicKey(key) if err != nil { - return nil, errors.Wrap(err, "failed to MarshalPKIXPublicKey") + return nil, fmt.Errorf("failed to MarshalPKIXPublicKey: %w", err) } keyinPem := pem.EncodeToMemory( &pem.Block{ diff --git a/pkg/util/net.go b/pkg/util/net.go index 4985cd8c47..3c1ade909d 100644 --- a/pkg/util/net.go +++ b/pkg/util/net.go @@ -18,6 +18,7 @@ package util import ( "context" "crypto/tls" + "fmt" tcpnet "net" "net/http" "os" @@ -27,7 +28,6 @@ import ( "time" "github.com/openshift/microshift/pkg/config/ovn" - "github.com/pkg/errors" "k8s.io/apimachinery/pkg/util/net" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/klog/v2" @@ -123,7 +123,10 @@ func AddToNoProxyEnv(additionalEntries ...string) error { // unset the lower-case one, and keep only upper-case os.Unsetenv("no_proxy") - return errors.Wrap(os.Setenv("NO_PROXY", noProxyEnv), "error updating NO_PROXY") + if err := os.Setenv("NO_PROXY", noProxyEnv); err != nil { + return fmt.Errorf("failed to update NO_PROXY: %w", err) + } + return nil } func mapKeys(entries map[string]struct{}) []string { From 67350d2e5d7c1492ad8042864e9f799464074804 Mon Sep 17 00:00:00 2001 From: Doug Hellmann Date: Mon, 12 Jun 2023 07:24:33 -0400 Subject: [PATCH 3/3] standardize on lower-case error message text Follow the standards at https://github.com/golang/go/wiki/CodeReviewComments#error-strings --- .../openshift/microshift/pkg/config/config.go | 6 ++-- .../openshift/microshift/pkg/config/files.go | 12 +++---- .../microshift/pkg/config/manifests.go | 2 +- pkg/admin/data/data_manager.go | 26 +++++++------- pkg/admin/prerun/prerun.go | 34 +++++++++---------- pkg/cmd/showConfig.go | 2 +- pkg/config/config.go | 6 ++-- pkg/config/files.go | 12 +++---- pkg/config/lvmd/lvmd.go | 2 +- pkg/config/manifests.go | 2 +- pkg/kustomize/apply.go | 2 +- 11 files changed, 53 insertions(+), 53 deletions(-) diff --git a/etcd/vendor/github.com/openshift/microshift/pkg/config/config.go b/etcd/vendor/github.com/openshift/microshift/pkg/config/config.go index 153dea18cc..199bb52abd 100644 --- a/etcd/vendor/github.com/openshift/microshift/pkg/config/config.go +++ b/etcd/vendor/github.com/openshift/microshift/pkg/config/config.go @@ -67,7 +67,7 @@ func (c *Config) fillDefaults() error { } hostname, err := os.Hostname() if err != nil { - return fmt.Errorf("Failed to get hostname %v", err) + return fmt.Errorf("failed to get hostname %v", err) } nodeIP, err := util.GetHostIP() if err != nil { @@ -250,7 +250,7 @@ func (c *Config) validate() error { return fmt.Errorf("subjectAltNames must not contain node IP") } if !stringSliceContains(c.ApiServer.SubjectAltNames, u.Host) || u.Host != c.Node.HostnameOverride { - return fmt.Errorf("Cluster URL host %v is not included in subjectAltNames or nodeName", u.String()) + return fmt.Errorf("cluster URL host %q must be included in subjectAltNames or nodeName", u.String()) } } if stringSliceContains( @@ -289,7 +289,7 @@ func getAllHostnames() ([]string, error) { cmd.Stdout = &out err := cmd.Run() if err != nil { - return nil, fmt.Errorf("Error when executing 'hostname -A': %v", err) + return nil, fmt.Errorf("error when executing 'hostname -A': %v", err) } outString := out.String() outString = strings.Trim(outString[:len(outString)-1], " ") diff --git a/etcd/vendor/github.com/openshift/microshift/pkg/config/files.go b/etcd/vendor/github.com/openshift/microshift/pkg/config/files.go index ff6fb3af27..6078040b90 100644 --- a/etcd/vendor/github.com/openshift/microshift/pkg/config/files.go +++ b/etcd/vendor/github.com/openshift/microshift/pkg/config/files.go @@ -16,7 +16,7 @@ const ( func parse(contents []byte) (*Config, error) { c := &Config{} if err := yaml.Unmarshal(contents, c); err != nil { - return nil, fmt.Errorf("Unable to decode configuration: %v", err) + return nil, fmt.Errorf("failed to decode configuration: %v", err) } return c, nil } @@ -24,21 +24,21 @@ func parse(contents []byte) (*Config, error) { func getActiveConfigFromYAML(contents []byte) (*Config, error) { userSettings, err := parse(contents) if err != nil { - return nil, fmt.Errorf("Error parsing config file %q: %v", ConfigFile, err) + return nil, fmt.Errorf("failed to parse config file %q: %v", ConfigFile, err) } // Start with the defaults, then apply the user settings and // recompute dynamic values. results := &Config{} if err := results.fillDefaults(); err != nil { - return nil, fmt.Errorf("Invalid configuration: %v", err) + return nil, fmt.Errorf("invalid configuration: %v", err) } results.incorporateUserSettings(userSettings) if err := results.updateComputedValues(); err != nil { - return nil, fmt.Errorf("Invalid configuration: %v", err) + return nil, fmt.Errorf("invalid configuration: %v", err) } if err := results.validate(); err != nil { - return nil, fmt.Errorf("Invalid configuration: %v", err) + return nil, fmt.Errorf("invalid configuration: %v", err) } return results, nil } @@ -58,7 +58,7 @@ func ActiveConfig() (*Config, error) { // Read the file and merge user-provided settings with the defaults contents, err := os.ReadFile(ConfigFile) if err != nil { - return nil, fmt.Errorf("Error reading config file %q: %v", ConfigFile, err) + return nil, fmt.Errorf("error reading config file %q: %v", ConfigFile, err) } return getActiveConfigFromYAML(contents) } diff --git a/etcd/vendor/github.com/openshift/microshift/pkg/config/manifests.go b/etcd/vendor/github.com/openshift/microshift/pkg/config/manifests.go index 7ad14c1907..8fae4cf717 100644 --- a/etcd/vendor/github.com/openshift/microshift/pkg/config/manifests.go +++ b/etcd/vendor/github.com/openshift/microshift/pkg/config/manifests.go @@ -34,7 +34,7 @@ func (m *Manifests) GetKustomizationPaths() ([]string, error) { pattern := filepath.Join(path, "kustomization.yaml") matches, err := filepath.Glob(pattern) if err != nil { - return nil, fmt.Errorf("Could not understand kustomizePath value %v: %w", path, err) + return nil, fmt.Errorf("failed to expand pattern in kustomizePath %v: %w", path, err) } if len(matches) == 0 { klog.Infof("No kustomize path matches %v", pattern) diff --git a/pkg/admin/data/data_manager.go b/pkg/admin/data/data_manager.go index 7af7a6e43f..658363665c 100644 --- a/pkg/admin/data/data_manager.go +++ b/pkg/admin/data/data_manager.go @@ -48,7 +48,7 @@ func (dm *manager) RemoveBackup(name BackupName) error { "name", name, ) if err := os.RemoveAll(dm.GetBackupPath(name)); err != nil { - return fmt.Errorf("Failed to delete backup %q: %w", name, err) + return fmt.Errorf("failed to delete backup %q: %w", name, err) } klog.InfoS("Removed backup", "name", name, @@ -84,18 +84,18 @@ func (dm *manager) Backup(name BackupName) error { } if exists, err := dm.BackupExists(name); err != nil { - return fmt.Errorf("Failed to determine if backup %q exists: %w", name, err) + return fmt.Errorf("failed to determine if backup %q exists: %w", name, err) } else if exists { - return fmt.Errorf("Failed to create backup destination %q because it already exists", + return fmt.Errorf("failed to create backup destination %q because it already exists", name) } if found, err := pathExists(string(dm.storage)); err != nil { - return fmt.Errorf("Failed to determine if storage location %q for backup exists: %w", + return fmt.Errorf("failed to determine if storage location %q for backup exists: %w", dm.storage, err) } else if !found { if makeDirErr := util.MakeDir(string(dm.storage)); makeDirErr != nil { - return fmt.Errorf("Failed to create backup storage directory %q: %w", + return fmt.Errorf("failed to create backup storage directory %q: %w", dm.storage, makeDirErr) } klog.InfoS("Created backup storage directory", "path", dm.storage) @@ -125,15 +125,15 @@ func (dm *manager) Restore(name BackupName) error { } if exists, err := dm.BackupExists(name); err != nil { - return fmt.Errorf("Failed to determine if backup %q exists: %w", name, err) + return fmt.Errorf("failed to determine if backup %q exists: %w", name, err) } else if !exists { - return fmt.Errorf("Failed to restore backup, %q does not exist", name) + return fmt.Errorf("failed to restore backup, %q does not exist", name) } tmp := fmt.Sprintf("%s.saved", config.DataDir) klog.InfoS("Renaming existing data dir", "data", config.DataDir, "renamedTo", tmp) if err := os.Rename(config.DataDir, tmp); err != nil { - return fmt.Errorf("Failed to rename existing data directory %q to %q: %w", + return fmt.Errorf("failed to rename existing data directory %q to %q: %w", config.DataDir, tmp, err) } @@ -142,15 +142,15 @@ func (dm *manager) Restore(name BackupName) error { klog.ErrorS(err, "Failed to restore from backup, restoring current data dir") if err := os.RemoveAll(config.DataDir); err != nil { - return fmt.Errorf("Failed to remove data directory %q: %w", config.DataDir, err) + return fmt.Errorf("failed to remove data directory %q: %w", config.DataDir, err) } if err := os.Rename(tmp, config.DataDir); err != nil { - return fmt.Errorf("Failed to rename temporary directory %q to %q: %w", + return fmt.Errorf("failed to rename temporary directory %q to %q: %w", tmp, config.DataDir, err) } - return fmt.Errorf("Failed to restore backup: %w", err) + return fmt.Errorf("failed to restore backup: %w", err) } klog.InfoS("Removing temporary data directory", "path", tmp) @@ -178,7 +178,7 @@ func copyPath(src, dest string) error { klog.InfoS("Failed to copy", "cmd", cmd, "stdout", strings.ReplaceAll(outb.String(), "\n", `, `), "stderr", errb.String()) - return fmt.Errorf("Failed to copy %q to %q: %w", src, dest, err) + return fmt.Errorf("failed to copy %q to %q: %w", src, dest, err) } klog.InfoS("Finished copy", "cmd", cmd) @@ -188,7 +188,7 @@ func copyPath(src, dest string) error { func pathExists(path string) (bool, error) { exists, err := util.PathExists(path) if err != nil { - return false, fmt.Errorf("Failed to check if path %q exists: %w", path, err) + return false, fmt.Errorf("failed to check if path %q exists: %w", path, err) } return exists, nil } diff --git a/pkg/admin/prerun/prerun.go b/pkg/admin/prerun/prerun.go index 0a36b9be2e..590c64a2ea 100644 --- a/pkg/admin/prerun/prerun.go +++ b/pkg/admin/prerun/prerun.go @@ -51,12 +51,12 @@ func (pr *PreRun) Perform() error { klog.InfoS("Skipping pre-run: Health status file does not exist") return nil } - return fmt.Errorf("Failed to determine the current system health: %w", err) + return fmt.Errorf("failed to determine the current system health: %w", err) } currentBootID, err := getCurrentBootID() if err != nil { - return fmt.Errorf("Failed to determine the current boot ID: %w", err) + return fmt.Errorf("failed to determine the current boot ID: %w", err) } klog.InfoS("Found boot details", @@ -82,12 +82,12 @@ func (pr *PreRun) Perform() error { if health.IsHealthy() { klog.Info("Previous boot was healthy") if err := pr.backup(health); err != nil { - return fmt.Errorf("Failed to backup during pre-run: %w", err) + 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) + return fmt.Errorf("failed version checks: %w", err) } klog.InfoS("Completed version checks", "is-migration-needed?", migrationNeeded) @@ -101,7 +101,7 @@ func (pr *PreRun) Perform() error { } else { klog.Info("Previous boot was not healthy") if err = pr.restore(); err != nil { - return fmt.Errorf("Failed to restore during pre-run: %w", err) + return fmt.Errorf("failed to restore during pre-run: %w", err) } } @@ -118,7 +118,7 @@ func (pr *PreRun) backup(health *HealthInfo) error { existingBackups, err := pr.dataManager.GetBackupList() if err != nil { - return fmt.Errorf("Failed to determine the existing backups: %w", err) + return fmt.Errorf("failed to determine the existing backups: %w", err) } // get list of already existing backups for deployment ID persisted in health file @@ -135,7 +135,7 @@ func (pr *PreRun) backup(health *HealthInfo) error { } if err := pr.dataManager.Backup(newBackupName); err != nil { - return fmt.Errorf("Failed to create new backup %q: %w", newBackupName, err) + return fmt.Errorf("failed to create new backup %q: %w", newBackupName, err) } pr.removeOldBackups(backupsForDeployment) @@ -158,7 +158,7 @@ func (pr *PreRun) restore() error { existingBackups, err := pr.dataManager.GetBackupList() if err != nil { - return fmt.Errorf("Failed to determine the existing backups: %w", err) + return fmt.Errorf("failed to determine the existing backups: %w", err) } klog.InfoS("Found existing backups", "currentDeploymentID", currentDeploymentID, @@ -177,7 +177,7 @@ func (pr *PreRun) restore() error { err = pr.dataManager.Restore(backupsForDeployment[0]) if err != nil { - return fmt.Errorf("Failed to restore backup: %w", err) + return fmt.Errorf("failed to restore backup: %w", err) } klog.Info("Finished restore") @@ -192,7 +192,7 @@ 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) + return false, fmt.Errorf("failed to determine the active version of the MicroShift: %w", err) } dataVer, err := getVersionOfData() @@ -202,7 +202,7 @@ func (pr *PreRun) checkVersions() (bool, error) { // TODO: 4.13 return true, nil } - return false, fmt.Errorf("Failed to determine the version of the existing data: %w", err) + return false, fmt.Errorf("failed to determine the version of the existing data: %w", err) } klog.InfoS("Comparing versions", "data", dataVer, "active", execVer) @@ -217,12 +217,12 @@ func getCurrentDeploymentID() (string, error) { cmd.Stderr = &stderr if err := cmd.Run(); err != nil { - return "", fmt.Errorf("Failed to determine the rpm-ostree deployment id, command %q failed: %s: %w", strings.TrimSpace(cmd.String()), strings.TrimSpace(stderr.String()), err) + return "", fmt.Errorf("failed to determine the rpm-ostree deployment id, command %q failed: %s: %w", strings.TrimSpace(cmd.String()), strings.TrimSpace(stderr.String()), err) } ids := []string{} if err := json.Unmarshal(stdout.Bytes(), &ids); err != nil { - return "", fmt.Errorf("Failed to determine the rpm-ostree deployment id from %q: %w", strings.TrimSpace(stdout.String()), err) + return "", fmt.Errorf("failed to determine the rpm-ostree deployment id from %q: %w", strings.TrimSpace(stdout.String()), err) } if len(ids) != 1 { @@ -232,7 +232,7 @@ func getCurrentDeploymentID() (string, error) { "stdout", strings.TrimSpace(stdout.String()), "stderr", strings.TrimSpace(stderr.String()), "unmarshalledIDs", ids) - return "", fmt.Errorf("Expected 1 deployment ID, rpm-ostree returned %d", len(ids)) + return "", fmt.Errorf("expected 1 deployment ID, rpm-ostree returned %d", len(ids)) } return ids[0], nil @@ -252,7 +252,7 @@ func getCurrentBootID() (string, error) { path := "/proc/sys/kernel/random/boot_id" content, err := os.ReadFile(path) if err != nil { - return "", fmt.Errorf("Failed to determine boot ID from %q: %w", path, err) + return "", fmt.Errorf("failed to determine boot ID from %q: %w", path, err) } return strings.ReplaceAll(strings.TrimSpace(string(content)), "-", ""), nil } @@ -267,12 +267,12 @@ func getHealthInfo() (*HealthInfo, error) { content, err := os.ReadFile(path) if err != nil { - return nil, fmt.Errorf("Failed to read health data from %q: %w", path, err) + return nil, fmt.Errorf("failed to read health data from %q: %w", path, 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 nil, fmt.Errorf("failed to parse health data %q: %w", strings.TrimSpace(string(content)), err) } return health, nil } diff --git a/pkg/cmd/showConfig.go b/pkg/cmd/showConfig.go index 0ea23cff11..e86fea78e8 100644 --- a/pkg/cmd/showConfig.go +++ b/pkg/cmd/showConfig.go @@ -36,7 +36,7 @@ func NewShowConfigCommand(ioStreams genericclioptions.IOStreams) *cobra.Command case "default": cfg = config.NewDefault() default: - cmdutil.CheckErr(fmt.Errorf("Unrecognized mode %q", opts.Mode)) + cmdutil.CheckErr(fmt.Errorf("unrecognized mode %q", opts.Mode)) } marshalled, err := yaml.Marshal(cfg) diff --git a/pkg/config/config.go b/pkg/config/config.go index 153dea18cc..199bb52abd 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -67,7 +67,7 @@ func (c *Config) fillDefaults() error { } hostname, err := os.Hostname() if err != nil { - return fmt.Errorf("Failed to get hostname %v", err) + return fmt.Errorf("failed to get hostname %v", err) } nodeIP, err := util.GetHostIP() if err != nil { @@ -250,7 +250,7 @@ func (c *Config) validate() error { return fmt.Errorf("subjectAltNames must not contain node IP") } if !stringSliceContains(c.ApiServer.SubjectAltNames, u.Host) || u.Host != c.Node.HostnameOverride { - return fmt.Errorf("Cluster URL host %v is not included in subjectAltNames or nodeName", u.String()) + return fmt.Errorf("cluster URL host %q must be included in subjectAltNames or nodeName", u.String()) } } if stringSliceContains( @@ -289,7 +289,7 @@ func getAllHostnames() ([]string, error) { cmd.Stdout = &out err := cmd.Run() if err != nil { - return nil, fmt.Errorf("Error when executing 'hostname -A': %v", err) + return nil, fmt.Errorf("error when executing 'hostname -A': %v", err) } outString := out.String() outString = strings.Trim(outString[:len(outString)-1], " ") diff --git a/pkg/config/files.go b/pkg/config/files.go index ff6fb3af27..6078040b90 100644 --- a/pkg/config/files.go +++ b/pkg/config/files.go @@ -16,7 +16,7 @@ const ( func parse(contents []byte) (*Config, error) { c := &Config{} if err := yaml.Unmarshal(contents, c); err != nil { - return nil, fmt.Errorf("Unable to decode configuration: %v", err) + return nil, fmt.Errorf("failed to decode configuration: %v", err) } return c, nil } @@ -24,21 +24,21 @@ func parse(contents []byte) (*Config, error) { func getActiveConfigFromYAML(contents []byte) (*Config, error) { userSettings, err := parse(contents) if err != nil { - return nil, fmt.Errorf("Error parsing config file %q: %v", ConfigFile, err) + return nil, fmt.Errorf("failed to parse config file %q: %v", ConfigFile, err) } // Start with the defaults, then apply the user settings and // recompute dynamic values. results := &Config{} if err := results.fillDefaults(); err != nil { - return nil, fmt.Errorf("Invalid configuration: %v", err) + return nil, fmt.Errorf("invalid configuration: %v", err) } results.incorporateUserSettings(userSettings) if err := results.updateComputedValues(); err != nil { - return nil, fmt.Errorf("Invalid configuration: %v", err) + return nil, fmt.Errorf("invalid configuration: %v", err) } if err := results.validate(); err != nil { - return nil, fmt.Errorf("Invalid configuration: %v", err) + return nil, fmt.Errorf("invalid configuration: %v", err) } return results, nil } @@ -58,7 +58,7 @@ func ActiveConfig() (*Config, error) { // Read the file and merge user-provided settings with the defaults contents, err := os.ReadFile(ConfigFile) if err != nil { - return nil, fmt.Errorf("Error reading config file %q: %v", ConfigFile, err) + return nil, fmt.Errorf("error reading config file %q: %v", ConfigFile, err) } return getActiveConfigFromYAML(contents) } diff --git a/pkg/config/lvmd/lvmd.go b/pkg/config/lvmd/lvmd.go index 93f94f5893..19ac344a07 100644 --- a/pkg/config/lvmd/lvmd.go +++ b/pkg/config/lvmd/lvmd.go @@ -93,7 +93,7 @@ func getLvmdConfigForVGs(vgNames []string) (*Lvmd, error) { func DefaultLvmdConfig() (*Lvmd, error) { vgNames, err := getVolumeGroups() if err != nil { - return nil, fmt.Errorf("Failed to discover local volume groups: %s", err) + return nil, fmt.Errorf("failed to discover local volume groups: %s", err) } return getLvmdConfigForVGs(vgNames) } diff --git a/pkg/config/manifests.go b/pkg/config/manifests.go index 7ad14c1907..8fae4cf717 100644 --- a/pkg/config/manifests.go +++ b/pkg/config/manifests.go @@ -34,7 +34,7 @@ func (m *Manifests) GetKustomizationPaths() ([]string, error) { pattern := filepath.Join(path, "kustomization.yaml") matches, err := filepath.Glob(pattern) if err != nil { - return nil, fmt.Errorf("Could not understand kustomizePath value %v: %w", path, err) + return nil, fmt.Errorf("failed to expand pattern in kustomizePath %v: %w", path, err) } if len(matches) == 0 { klog.Infof("No kustomize path matches %v", pattern) diff --git a/pkg/kustomize/apply.go b/pkg/kustomize/apply.go index 765b5cc307..6061e459e6 100644 --- a/pkg/kustomize/apply.go +++ b/pkg/kustomize/apply.go @@ -45,7 +45,7 @@ func (s *Kustomizer) Run(ctx context.Context, ready chan<- struct{}, stopped cha kustomizationPaths, err := s.cfg.Manifests.GetKustomizationPaths() if err != nil { - return fmt.Errorf("Could not find kustomization paths: %w", err) + return fmt.Errorf("failed to find any kustomization paths: %w", err) } for _, path := range kustomizationPaths {