Skip to content

Commit

Permalink
incorrect check when looking for installed package
Browse files Browse the repository at this point in the history
  • Loading branch information
@brownaaron
brownaaron committed Nov 4, 2024
1 parent 30cd884 commit 30f4726
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 30 deletions.
4 changes: 2 additions & 2 deletions internal/nodeprep/packagemanager/apt/apt.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ func (a *Apt) validatePackageInstall(ctx context.Context, packageName string) er
return errors.New(fmt.Sprintf("failed to validate %s package install: %s", packageName, output))
}

if strings.Contains(output, fmt.Sprintf("Unable to locate package %s", packageName)) {
return fmt.Errorf("failed to install package %s: %s", packageName, output)
if !strings.Contains(output, packageName) {
return errors.New(fmt.Sprintf("failed to install %s package : %s", packageName, output))
}

return nil
Expand Down
53 changes: 25 additions & 28 deletions internal/nodeprep/packagemanager/apt/apt_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ package apt_test

import (
"context"
"fmt"
"testing"
"time"

Expand Down Expand Up @@ -35,8 +34,7 @@ func TestApt_MultipathToolsInstalled(t *testing.T) {
}
const timeout = 1 * time.Second
const logCommandOutput = false
const alreadyInstalledOutput = "foo baz"
const notInstalledOutput = "Unable to locate package " + apt.PackageMultipathTools
const notInstalledOutput = "foo baz"

tests := map[string]parameters{
"error executing command": {
Expand All @@ -52,7 +50,7 @@ func TestApt_MultipathToolsInstalled(t *testing.T) {
getCommand: func(controller *gomock.Controller) *mockexec.MockCommand {
mockCommand := mockexec.NewMockCommand(controller)
mockCommand.EXPECT().ExecuteWithTimeout(gomock.Any(), "apt", timeout, logCommandOutput, "list",
"--installed", apt.PackageMultipathTools).Return([]byte(alreadyInstalledOutput), nil)
"--installed", apt.PackageMultipathTools).Return([]byte(apt.PackageMultipathTools), nil)
return mockCommand
},
assertResponse: assert.True,
Expand Down Expand Up @@ -91,8 +89,7 @@ func TestApt_InstallIscsiRequirements(t *testing.T) {

const timeout = 1 * time.Second
const logCommandOutput = false
const successfulpackageInstallOutput = "foo baz"
const failedPackageInstallOutput = "Unable to locate package %s"
const failedPackageInstallOutput = "foo baz"

tests := map[string]parameters{
"happy path: error installing lsscsi": {
Expand All @@ -103,11 +100,11 @@ func TestApt_InstallIscsiRequirements(t *testing.T) {
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "install",
"-y", apt.PackageOpenIscsi).Return(nil, nil)
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "list",
"--installed", apt.PackageOpenIscsi).Return([]byte(successfulpackageInstallOutput), nil)
"--installed", apt.PackageOpenIscsi).Return([]byte(apt.PackageOpenIscsi), nil)
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "install",
"-y", apt.PackageMultipathTools).Return(nil, nil)
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "list",
"--installed", apt.PackageMultipathTools).Return([]byte(successfulpackageInstallOutput), nil)
"--installed", apt.PackageMultipathTools).Return([]byte(apt.PackageMultipathTools), nil)
return mockCommand
},
assertError: assert.NoError,
Expand All @@ -122,11 +119,11 @@ func TestApt_InstallIscsiRequirements(t *testing.T) {
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "install",
"-y", apt.PackageOpenIscsi).Return(nil, nil)
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "list",
"--installed", apt.PackageOpenIscsi).Return([]byte(successfulpackageInstallOutput), nil)
"--installed", apt.PackageOpenIscsi).Return([]byte(apt.PackageOpenIscsi), nil)
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "install",
"-y", apt.PackageMultipathTools).Return(nil, nil)
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "list",
"--installed", apt.PackageMultipathTools).Return([]byte(successfulpackageInstallOutput), nil)
"--installed", apt.PackageMultipathTools).Return([]byte(apt.PackageMultipathTools), nil)
return mockCommand
},
assertError: assert.NoError,
Expand All @@ -137,15 +134,15 @@ func TestApt_InstallIscsiRequirements(t *testing.T) {
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "install",
"-y", apt.PackageLsscsi).Return(nil, nil)
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "list",
"--installed", apt.PackageLsscsi).Return([]byte(fmt.Sprintf(failedPackageInstallOutput, apt.PackageLsscsi)), nil)
"--installed", apt.PackageLsscsi).Return([]byte(failedPackageInstallOutput), nil)
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "install",
"-y", apt.PackageOpenIscsi).Return(nil, nil)
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "list",
"--installed", apt.PackageOpenIscsi).Return([]byte(successfulpackageInstallOutput), nil)
"--installed", apt.PackageOpenIscsi).Return([]byte(apt.PackageOpenIscsi), nil)
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "install",
"-y", apt.PackageMultipathTools).Return(nil, nil)
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "list",
"--installed", apt.PackageMultipathTools).Return([]byte(successfulpackageInstallOutput), nil)
"--installed", apt.PackageMultipathTools).Return([]byte(apt.PackageMultipathTools), nil)
return mockCommand
},
assertError: assert.NoError,
Expand All @@ -154,17 +151,17 @@ func TestApt_InstallIscsiRequirements(t *testing.T) {
getCommand: func(controller *gomock.Controller) *mockexec.MockCommand {
mockCommand := mockexec.NewMockCommand(controller)
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "install",
"-y", apt.PackageLsscsi).Return([]byte(successfulpackageInstallOutput), nil)
"-y", apt.PackageLsscsi).Return([]byte(apt.PackageLsscsi), nil)
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "list",
"--installed", apt.PackageLsscsi).Return([]byte(successfulpackageInstallOutput), nil)
"--installed", apt.PackageLsscsi).Return([]byte(apt.PackageLsscsi), nil)
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "install",
"-y", apt.PackageOpenIscsi).Return(nil, nil)
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "list",
"--installed", apt.PackageOpenIscsi).Return([]byte(successfulpackageInstallOutput), nil)
"--installed", apt.PackageOpenIscsi).Return([]byte(apt.PackageOpenIscsi), nil)
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "install",
"-y", apt.PackageMultipathTools).Return(nil, nil)
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "list",
"--installed", apt.PackageMultipathTools).Return([]byte(successfulpackageInstallOutput), nil)
"--installed", apt.PackageMultipathTools).Return([]byte(apt.PackageMultipathTools), nil)
return mockCommand
},
assertError: assert.NoError,
Expand All @@ -175,7 +172,7 @@ func TestApt_InstallIscsiRequirements(t *testing.T) {
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "install",
"-y", apt.PackageLsscsi).Return(nil, nil)
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "list",
"--installed", apt.PackageLsscsi).Return([]byte(successfulpackageInstallOutput), nil)
"--installed", apt.PackageLsscsi).Return([]byte(apt.PackageLsscsi), nil)
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "install",
"-y", apt.PackageOpenIscsi).Return(nil, errors.New("some error"))
return mockCommand
Expand All @@ -188,7 +185,7 @@ func TestApt_InstallIscsiRequirements(t *testing.T) {
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "install",
"-y", apt.PackageLsscsi).Return(nil, nil)
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "list",
"--installed", apt.PackageLsscsi).Return([]byte(successfulpackageInstallOutput), nil)
"--installed", apt.PackageLsscsi).Return([]byte(apt.PackageLsscsi), nil)
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "install",
"-y", apt.PackageOpenIscsi).Return(nil, nil)
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "list",
Expand All @@ -203,11 +200,11 @@ func TestApt_InstallIscsiRequirements(t *testing.T) {
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "install",
"-y", apt.PackageLsscsi).Return(nil, nil)
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "list",
"--installed", apt.PackageLsscsi).Return([]byte(successfulpackageInstallOutput), nil)
"--installed", apt.PackageLsscsi).Return([]byte(apt.PackageLsscsi), nil)
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "install",
"-y", apt.PackageOpenIscsi).Return(nil, nil)
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "list",
"--installed", apt.PackageOpenIscsi).Return([]byte(fmt.Sprintf(failedPackageInstallOutput, apt.PackageOpenIscsi)), nil)
"--installed", apt.PackageOpenIscsi).Return([]byte(failedPackageInstallOutput), nil)
return mockCommand
},
assertError: assert.Error,
Expand All @@ -218,11 +215,11 @@ func TestApt_InstallIscsiRequirements(t *testing.T) {
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "install",
"-y", apt.PackageLsscsi).Return(nil, nil)
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "list",
"--installed", apt.PackageLsscsi).Return([]byte(successfulpackageInstallOutput), nil)
"--installed", apt.PackageLsscsi).Return([]byte(apt.PackageLsscsi), nil)
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "install",
"-y", apt.PackageOpenIscsi).Return(nil, nil)
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "list",
"--installed", apt.PackageOpenIscsi).Return([]byte(successfulpackageInstallOutput), nil)
"--installed", apt.PackageOpenIscsi).Return([]byte(apt.PackageOpenIscsi), nil)
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "install",
"-y", apt.PackageMultipathTools).Return(nil, errors.New("some error"))
return mockCommand
Expand All @@ -235,11 +232,11 @@ func TestApt_InstallIscsiRequirements(t *testing.T) {
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "install",
"-y", apt.PackageLsscsi).Return(nil, nil)
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "list",
"--installed", apt.PackageLsscsi).Return([]byte(successfulpackageInstallOutput), nil)
"--installed", apt.PackageLsscsi).Return([]byte(apt.PackageLsscsi), nil)
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "install",
"-y", apt.PackageOpenIscsi).Return(nil, nil)
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "list",
"--installed", apt.PackageOpenIscsi).Return([]byte(successfulpackageInstallOutput), nil)
"--installed", apt.PackageOpenIscsi).Return([]byte(apt.PackageOpenIscsi), nil)
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "install",
"-y", apt.PackageMultipathTools).Return(nil, nil)
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "list",
Expand All @@ -254,15 +251,15 @@ func TestApt_InstallIscsiRequirements(t *testing.T) {
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "install",
"-y", apt.PackageLsscsi).Return(nil, nil)
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "list",
"--installed", apt.PackageLsscsi).Return([]byte(successfulpackageInstallOutput), nil)
"--installed", apt.PackageLsscsi).Return([]byte(apt.PackageLsscsi), nil)
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "install",
"-y", apt.PackageOpenIscsi).Return(nil, nil)
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "list",
"--installed", apt.PackageOpenIscsi).Return([]byte(successfulpackageInstallOutput), nil)
"--installed", apt.PackageOpenIscsi).Return([]byte(apt.PackageOpenIscsi), nil)
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "install",
"-y", apt.PackageMultipathTools).Return(nil, nil)
mockCommand.EXPECT().ExecuteWithTimeout(context.TODO(), "apt", timeout, logCommandOutput, "list",
"--installed", apt.PackageMultipathTools).Return([]byte(fmt.Sprintf(failedPackageInstallOutput, apt.PackageMultipathTools)), nil)
"--installed", apt.PackageMultipathTools).Return([]byte(failedPackageInstallOutput), nil)
return mockCommand
},
assertError: assert.Error,
Expand Down

0 comments on commit 30f4726

Please sign in to comment.