Skip to content

Commit

Permalink
fix(apt): pass environment variables as argument to RunCommandWithRetry
Browse files Browse the repository at this point in the history
  • Loading branch information
cderici committed Jun 5, 2024
1 parent 0e2ca19 commit b78a77b
Show file tree
Hide file tree
Showing 8 changed files with 34 additions and 33 deletions.
16 changes: 8 additions & 8 deletions commands/apt.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ const (

// Environment variable for disabling interactive prompts in frontends of
// commands like apt-get
frontendNoninteractive = "DEBIAN_FRONTEND=noninteractive"
EnvFrontendNoninteractive = "DEBIAN_FRONTEND=noninteractive"

// the basic format for specifying a proxy option for apt:
aptProxySettingFormat = "Acquire::%s::Proxy %q;"
Expand All @@ -50,20 +50,20 @@ const (

// aptCmder is the packageCommander instantiation for apt-based systems.
var aptCmder = packageCommander{
prereq: buildCommand(frontendNoninteractive, aptget, "install python-software-properties"),
update: buildCommand(frontendNoninteractive, aptget, "update"),
upgrade: buildCommand(frontendNoninteractive, aptget, "upgrade"),
install: buildCommand(frontendNoninteractive, aptget, "install"),
remove: buildCommand(frontendNoninteractive, aptget, "remove"),
purge: buildCommand(frontendNoninteractive, aptget, "purge"),
prereq: buildCommand(aptget, "install python-software-properties"),
update: buildCommand(aptget, "update"),
upgrade: buildCommand(aptget, "upgrade"),
install: buildCommand(aptget, "install"),
remove: buildCommand(aptget, "remove"),
purge: buildCommand(aptget, "purge"),
search: buildCommand(aptcache, "search --names-only ^%s$"),
isInstalled: buildCommand(dpkgquery, "-s %s"),
listAvailable: buildCommand(aptcache, "pkgnames"),
listInstalled: buildCommand(dpkg, "--get-selections"),
addRepository: buildCommand(addaptrepo, "%q"),
listRepositories: buildCommand(`sed -r -n "s|^deb(-src)? (.*)|\2|p"`, "/etc/apt/sources.list"),
removeRepository: buildCommand(addaptrepo, "--remove ppa:%s"),
cleanup: buildCommand(frontendNoninteractive, aptget, "autoremove"),
cleanup: buildCommand(aptget, "autoremove"),
getProxy: buildCommand(aptconfig, "Acquire::http::Proxy Acquire::https::Proxy Acquire::ftp::Proxy"),
proxySettingsFormat: aptProxySettingFormat,
setProxy: buildCommand("echo %s >> ", AptConfFilePath),
Expand Down
4 changes: 2 additions & 2 deletions manager/apt.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func NewAptPackageManager() PackageManager {

// Search is defined on the PackageManager interface.
func (apt *apt) Search(pack string) (bool, error) {
out, _, err := RunCommandWithRetry(apt.cmder.SearchCmd(pack), apt, apt.retryPolicy)
out, _, err := RunCommandWithRetry(apt.cmder.SearchCmd(pack), apt, apt.retryPolicy, nil)
if err != nil {
return false, err
}
Expand All @@ -63,7 +63,7 @@ func (apt *apt) Search(pack string) (bool, error) {

// Install is defined on the PackageManager interface.
func (apt *apt) Install(packs ...string) error {
_, _, err := RunCommandWithRetry(apt.cmder.InstallCmd(packs...), apt.installRetryable, apt.retryPolicy)
_, _, err := RunCommandWithRetry(apt.cmder.InstallCmd(packs...), apt.installRetryable, apt.retryPolicy, []string{commands.EnvFrontendNoninteractive})
return err
}

Expand Down
18 changes: 9 additions & 9 deletions manager/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,37 +23,37 @@ type basePackageManager struct {

// InstallPrerequisite is defined on the PackageManager interface.
func (pm *basePackageManager) InstallPrerequisite() error {
_, _, err := RunCommandWithRetry(pm.cmder.InstallPrerequisiteCmd(), pm, pm.retryPolicy)
_, _, err := RunCommandWithRetry(pm.cmder.InstallPrerequisiteCmd(), pm, pm.retryPolicy, nil)
return err
}

// Update is defined on the PackageManager interface.
func (pm *basePackageManager) Update() error {
_, _, err := RunCommandWithRetry(pm.cmder.UpdateCmd(), pm, pm.retryPolicy)
_, _, err := RunCommandWithRetry(pm.cmder.UpdateCmd(), pm, pm.retryPolicy, nil)
return err
}

// Upgrade is defined on the PackageManager interface.
func (pm *basePackageManager) Upgrade() error {
_, _, err := RunCommandWithRetry(pm.cmder.UpgradeCmd(), pm, pm.retryPolicy)
_, _, err := RunCommandWithRetry(pm.cmder.UpgradeCmd(), pm, pm.retryPolicy, nil)
return err
}

// Install is defined on the PackageManager interface.
func (pm *basePackageManager) Install(packs ...string) error {
_, _, err := RunCommandWithRetry(pm.cmder.InstallCmd(packs...), pm, pm.retryPolicy)
_, _, err := RunCommandWithRetry(pm.cmder.InstallCmd(packs...), pm, pm.retryPolicy, nil)
return err
}

// Remove is defined on the PackageManager interface.
func (pm *basePackageManager) Remove(packs ...string) error {
_, _, err := RunCommandWithRetry(pm.cmder.RemoveCmd(packs...), pm, pm.retryPolicy)
_, _, err := RunCommandWithRetry(pm.cmder.RemoveCmd(packs...), pm, pm.retryPolicy, nil)
return err
}

// Purge is defined on the PackageManager interface.
func (pm *basePackageManager) Purge(packs ...string) error {
_, _, err := RunCommandWithRetry(pm.cmder.PurgeCmd(packs...), pm, pm.retryPolicy)
_, _, err := RunCommandWithRetry(pm.cmder.PurgeCmd(packs...), pm, pm.retryPolicy, nil)
return err
}

Expand All @@ -67,19 +67,19 @@ func (pm *basePackageManager) IsInstalled(pack string) bool {

// AddRepository is defined on the PackageManager interface.
func (pm *basePackageManager) AddRepository(repo string) error {
_, _, err := RunCommandWithRetry(pm.cmder.AddRepositoryCmd(repo), pm, pm.retryPolicy)
_, _, err := RunCommandWithRetry(pm.cmder.AddRepositoryCmd(repo), pm, pm.retryPolicy, nil)
return err
}

// RemoveRepository is defined on the PackageManager interface.
func (pm *basePackageManager) RemoveRepository(repo string) error {
_, _, err := RunCommandWithRetry(pm.cmder.RemoveRepositoryCmd(repo), pm, pm.retryPolicy)
_, _, err := RunCommandWithRetry(pm.cmder.RemoveRepositoryCmd(repo), pm, pm.retryPolicy, nil)
return err
}

// Cleanup is defined on the PackageManager interface.
func (pm *basePackageManager) Cleanup() error {
_, _, err := RunCommandWithRetry(pm.cmder.CleanupCmd(), pm, pm.retryPolicy)
_, _, err := RunCommandWithRetry(pm.cmder.CleanupCmd(), pm, pm.retryPolicy, nil)
return err
}

Expand Down
4 changes: 2 additions & 2 deletions manager/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,8 @@ var (
// getMockRunCommandWithRetry returns a function with the same signature as
// RunCommandWithRetry which saves the command it receives in the provided
// string whilst always returning no output, 0 error code and nil error.
func getMockRunCommandWithRetry(stor *string) func(string, manager.Retryable, manager.RetryPolicy) (string, int, error) {
return func(cmd string, _ manager.Retryable, _ manager.RetryPolicy) (string, int, error) {
func getMockRunCommandWithRetry(stor *string) func(string, manager.Retryable, manager.RetryPolicy, []string) (string, int, error) {
return func(cmd string, _ manager.Retryable, _ manager.RetryPolicy, _ []string) (string, int, error) {
*stor = cmd
return "", 0, nil
}
Expand Down
3 changes: 2 additions & 1 deletion manager/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func DefaultRetryPolicy() RetryPolicy {
// It returns the output of the command, the exit code, and an error, if one occurs,
// logging along the way.
// It was aliased for testing purposes.
var RunCommandWithRetry = func(cmd string, retryable Retryable, policy RetryPolicy) (output string, code int, _ error) {
var RunCommandWithRetry = func(cmd string, retryable Retryable, policy RetryPolicy, envVariables []string) (output string, code int, _ error) {
// split the command for use with exec
args := strings.Fields(cmd)
if len(args) <= 1 {
Expand All @@ -105,6 +105,7 @@ var RunCommandWithRetry = func(cmd string, retryable Retryable, policy RetryPoli
// Create the command for each attempt, because we need to
// call cmd.CombinedOutput only once. See http://pad.lv/1394524.
command := exec.Command(args[0], args[1:]...)
command.Env = append(os.Environ(), envVariables...)

var err error
out, err = CommandOutput(command)
Expand Down
18 changes: 9 additions & 9 deletions manager/snap.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func NewSnapPackageManager() *Snap {

// Search is defined on the PackageManager interface.
func (snap *Snap) Search(pack string) (bool, error) {
out, _, err := RunCommandWithRetry(snap.cmder.SearchCmd(pack), snap, snap.retryPolicy)
out, _, err := RunCommandWithRetry(snap.cmder.SearchCmd(pack), snap, snap.retryPolicy, nil)
if strings.Contains(combinedOutput(out, err), "error: no snap found") {
return false, nil
} else if err != nil {
Expand All @@ -77,7 +77,7 @@ func (snap *Snap) Search(pack string) (bool, error) {

// IsInstalled is defined on the PackageManager interface.
func (snap *Snap) IsInstalled(pack string) bool {
out, _, err := RunCommandWithRetry(snap.cmder.IsInstalledCmd(pack), snap, snap.retryPolicy)
out, _, err := RunCommandWithRetry(snap.cmder.IsInstalledCmd(pack), snap, snap.retryPolicy, nil)
if strings.Contains(combinedOutput(out, err), "error: no matching snaps installed") || err != nil {
return false
}
Expand All @@ -86,7 +86,7 @@ func (snap *Snap) IsInstalled(pack string) bool {

// InstalledChannel returns the snap channel for an installed package.
func (snap *Snap) InstalledChannel(pack string) string {
out, _, err := RunCommandWithRetry(fmt.Sprintf("snap info %s", pack), snap, snap.retryPolicy)
out, _, err := RunCommandWithRetry(fmt.Sprintf("snap info %s", pack), snap, snap.retryPolicy, nil)
combined := combinedOutput(out, err)
matches := trackingRE.FindAllStringSubmatch(combined, 1)
if len(matches) == 0 {
Expand All @@ -99,7 +99,7 @@ func (snap *Snap) InstalledChannel(pack string) string {
// ChangeChannel updates the tracked channel for an installed snap.
func (snap *Snap) ChangeChannel(pack, channel string) error {
cmd := fmt.Sprintf("snap refresh --channel %s %s", channel, pack)
out, _, err := RunCommandWithRetry(cmd, snap, snap.retryPolicy)
out, _, err := RunCommandWithRetry(cmd, snap, snap.retryPolicy, nil)
if err != nil {
return err
} else if strings.Contains(combinedOutput(out, err), "not installed") {
Expand All @@ -111,7 +111,7 @@ func (snap *Snap) ChangeChannel(pack, channel string) error {

// Install is defined on the PackageManager interface.
func (snap *Snap) Install(packs ...string) error {
out, _, err := RunCommandWithRetry(snap.cmder.InstallCmd(packs...), snap.installRetryable, snap.retryPolicy)
out, _, err := RunCommandWithRetry(snap.cmder.InstallCmd(packs...), snap.installRetryable, snap.retryPolicy, nil)
if snapNotFoundRE.MatchString(combinedOutput(out, err)) {
return errors.New("unable to locate package")
}
Expand All @@ -122,7 +122,7 @@ func (snap *Snap) Install(packs ...string) error {
func (snap *Snap) GetProxySettings() (proxy.Settings, error) {
var res proxy.Settings

out, _, err := RunCommandWithRetry(snap.cmder.GetProxyCmd(), snap, snap.retryPolicy)
out, _, err := RunCommandWithRetry(snap.cmder.GetProxyCmd(), snap, snap.retryPolicy, nil)
if strings.Contains(combinedOutput(out, err), `no "proxy" configuration option`) {
return res, nil
} else if err != nil {
Expand Down Expand Up @@ -170,12 +170,12 @@ func (snap *Snap) ConfigureStoreProxy(assertions, storeID string) error {
_ = assertFile.Close()

ackCmd := fmt.Sprintf("snap ack %s", assertFile.Name())
if _, _, err = RunCommandWithRetry(ackCmd, snap, snap.retryPolicy); err != nil {
if _, _, err = RunCommandWithRetry(ackCmd, snap, snap.retryPolicy, nil); err != nil {
return errors.Annotate(err, "failed to execute 'snap ack'")
}

setCmd := fmt.Sprintf("snap set system proxy.store=%s", storeID)
if _, _, err = RunCommandWithRetry(setCmd, snap, snap.retryPolicy); err != nil {
if _, _, err = RunCommandWithRetry(setCmd, snap, snap.retryPolicy, nil); err != nil {
return errors.Annotatef(err, "failed to configure snap to use store ID %q", storeID)
}

Expand All @@ -189,7 +189,7 @@ func (snap *Snap) ConfigureStoreProxy(assertions, storeID string) error {
// call to SetProxy.
func (snap *Snap) DisableStoreProxy() error {
setCmd := "snap set system proxy.store="
if _, _, err := RunCommandWithRetry(setCmd, snap, snap.retryPolicy); err != nil {
if _, _, err := RunCommandWithRetry(setCmd, snap, snap.retryPolicy, nil); err != nil {
return errors.Annotate(err, "failed to configure snap to not use a store proxy")
}

Expand Down
2 changes: 1 addition & 1 deletion manager/yum.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func NewYumPackageManager() PackageManager {

// Search is defined on the PackageManager interface.
func (yum *yum) Search(pack string) (bool, error) {
_, code, err := RunCommandWithRetry(yum.cmder.SearchCmd(pack), yum, yum.retryPolicy)
_, code, err := RunCommandWithRetry(yum.cmder.SearchCmd(pack), yum, yum.retryPolicy, nil)

// yum list package returns 1 when it cannot find the package.
if code == 1 {
Expand Down
2 changes: 1 addition & 1 deletion manager/zypper.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func NewZypperPackageManager() PackageManager {

// Search is defined on the PackageManager interface.
func (zypper *zypper) Search(pack string) (bool, error) {
_, code, err := RunCommandWithRetry(zypper.cmder.SearchCmd(pack), zypper, zypper.retryPolicy)
_, code, err := RunCommandWithRetry(zypper.cmder.SearchCmd(pack), zypper, zypper.retryPolicy, nil)

// zypper search returns 104 when it cannot find the package.
if code == 104 {
Expand Down

0 comments on commit b78a77b

Please sign in to comment.