Skip to content

Commit d284c1e

Browse files
author
Kaan Yalti
committed
enhancement(5235): removed unpack specific test from upgrade tests, added test cases in the upgrade error handling test
1 parent 183a6eb commit d284c1e

1 file changed

Lines changed: 71 additions & 149 deletions

File tree

‎internal/pkg/agent/application/upgrade/upgrade_test.go‎

Lines changed: 71 additions & 149 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,15 @@
55
package upgrade
66

77
import (
8-
"bytes"
98
"context"
109
"crypto/tls"
1110
"fmt"
1211
"io"
1312
"net/http"
14-
"net/http/httptest"
1513
"net/url"
1614
"os"
1715
"path/filepath"
1816
"runtime"
19-
"strings"
2017
"sync"
2118
"testing"
2219
"time"
@@ -31,9 +28,7 @@ import (
3128
"github.com/elastic/elastic-agent-libs/transport/tlscommon"
3229
"github.com/elastic/elastic-agent/internal/pkg/agent/application/paths"
3330
"github.com/elastic/elastic-agent/internal/pkg/agent/application/upgrade/artifact"
34-
downloadErrors "github.com/elastic/elastic-agent/internal/pkg/agent/application/upgrade/artifact/download/errors"
3531
upgradeErrors "github.com/elastic/elastic-agent/internal/pkg/agent/application/upgrade/artifact/download/errors"
36-
"github.com/elastic/elastic-agent/internal/pkg/agent/application/upgrade/common"
3732
"github.com/elastic/elastic-agent/internal/pkg/agent/application/upgrade/details"
3833
"github.com/elastic/elastic-agent/internal/pkg/agent/errors"
3934
"github.com/elastic/elastic-agent/internal/pkg/config"
@@ -1316,59 +1311,94 @@ func (m *mockArtifactDownloader) withFleetServerURI(fleetServerURI string) {
13161311
m.fleetServerURI = fleetServerURI
13171312
}
13181313

1319-
type mockSender struct{}
1320-
1321-
func (m *mockSender) Send(ctx context.Context, method, path string, params url.Values, headers http.Header, body io.Reader) (*http.Response, error) {
1322-
return nil, nil
1314+
type mockUnpacker struct {
1315+
returnPackageMetadata packageMetadata
1316+
returnPackageMetadataError error
1317+
returnUnpackResult UnpackResult
1318+
returnUnpackError error
13231319
}
13241320

1325-
func (m *mockSender) URI() string {
1326-
return "mockURI"
1321+
func (m *mockUnpacker) getPackageMetadata(archivePath string) (packageMetadata, error) {
1322+
return m.returnPackageMetadata, m.returnPackageMetadataError
13271323
}
13281324

1329-
func TestSetClient(t *testing.T) {
1330-
log, _ := loggertest.New("test")
1331-
upgrader := &Upgrader{
1332-
log: log,
1333-
artifactDownloader: &mockArtifactDownloader{},
1334-
}
1335-
1336-
upgrader.SetClient(&mockSender{})
1337-
require.Equal(t, "mockURI", upgrader.artifactDownloader.(*mockArtifactDownloader).fleetServerURI)
1325+
func (m *mockUnpacker) unpack(version, archivePath, dataDir string, flavor string) (UnpackResult, error) {
1326+
return m.returnUnpackResult, m.returnUnpackError
13381327
}
13391328

13401329
func TestUpgradeErrorHandling(t *testing.T) {
13411330
log, _ := loggertest.New("test")
13421331
testError := errors.New("test error")
1332+
type upgraderMocker func(upgrader *Upgrader)
13431333

13441334
type testCase struct {
13451335
isDiskSpaceErrorResult bool
13461336
expectedError error
1337+
upgraderMocker upgraderMocker
13471338
}
13481339

13491340
testCases := map[string]testCase{
13501341
"should return error if downloadArtifact fails": {
13511342
isDiskSpaceErrorResult: false,
13521343
expectedError: testError,
1344+
upgraderMocker: func(upgrader *Upgrader) {
1345+
upgrader.artifactDownloader = &mockArtifactDownloader{
1346+
returnError: testError,
1347+
}
1348+
},
1349+
},
1350+
"should return error if getPackageMetadata fails": {
1351+
isDiskSpaceErrorResult: false,
1352+
expectedError: testError,
1353+
upgraderMocker: func(upgrader *Upgrader) {
1354+
upgrader.artifactDownloader = &mockArtifactDownloader{}
1355+
upgrader.unpacker = &mockUnpacker{
1356+
returnPackageMetadataError: testError,
1357+
}
1358+
},
1359+
},
1360+
"should return error if unpack fails": {
1361+
isDiskSpaceErrorResult: false,
1362+
expectedError: testError,
1363+
upgraderMocker: func(upgrader *Upgrader) {
1364+
upgrader.artifactDownloader = &mockArtifactDownloader{}
1365+
upgrader.extractAgentVersion = func(metadata packageMetadata, upgradeVersion string) agentVersion {
1366+
return agentVersion{
1367+
version: upgradeVersion,
1368+
snapshot: false,
1369+
hash: metadata.hash,
1370+
}
1371+
}
1372+
upgrader.unpacker = &mockUnpacker{
1373+
returnPackageMetadata: packageMetadata{
1374+
manifest: &v1.PackageManifest{},
1375+
hash: "hash",
1376+
},
1377+
returnUnpackError: testError,
1378+
}
1379+
},
13531380
},
13541381
"should add disk space error to the error chain if downloadArtifact fails with disk space error": {
13551382
isDiskSpaceErrorResult: true,
13561383
expectedError: upgradeErrors.ErrInsufficientDiskSpace,
1384+
upgraderMocker: func(upgrader *Upgrader) {
1385+
upgrader.artifactDownloader = &mockArtifactDownloader{
1386+
returnError: upgradeErrors.ErrInsufficientDiskSpace,
1387+
}
1388+
},
13571389
},
13581390
}
13591391

13601392
mockAgentInfo := info.NewAgent(t)
13611393
mockAgentInfo.On("Version").Return("9.0.0")
13621394

1363-
upgrader, err := NewUpgrader(log, &artifact.Config{}, mockAgentInfo)
1364-
require.NoError(t, err)
1365-
1366-
upgrader.artifactDownloader = &mockArtifactDownloader{
1367-
returnError: testError,
1368-
}
1369-
13701395
for name, tc := range testCases {
13711396
t.Run(name, func(t *testing.T) {
1397+
upgrader, err := NewUpgrader(log, &artifact.Config{}, mockAgentInfo)
1398+
require.NoError(t, err)
1399+
1400+
tc.upgraderMocker(upgrader)
1401+
13721402
upgrader.isDiskSpaceErrorFunc = func(err error) bool {
13731403
return tc.isDiskSpaceErrorResult
13741404
}
@@ -1379,130 +1409,22 @@ func TestUpgradeErrorHandling(t *testing.T) {
13791409
}
13801410
}
13811411

1382-
func TestUpgradeUnpackErrors(t *testing.T) {
1383-
log, _ := loggertest.New("test")
1384-
1385-
targetVersion := agtversion.NewParsedSemVer(3, 4, 5, "SNAPSHOT", "")
1386-
targetArtifactName, targetArchiveFiles := buildArchiveFiles(t, archiveFilesWithMoreComponents, targetVersion, "ghijkl")
1387-
1388-
// Get the content of the component file to be used in copy function
1389-
// assertions. It will be used to make sure the error gets triggered in the
1390-
// copy call in the unpacker and not the downloader.
1391-
targetArchiveComponentsFileContent := ""
1392-
for _, file := range targetArchiveFiles {
1393-
if file.fType == REGULAR {
1394-
dir := filepath.Dir(file.path)
1395-
if strings.HasSuffix(dir, "components") {
1396-
targetArchiveComponentsFileContent = file.content
1397-
break
1398-
}
1399-
}
1400-
}
1412+
type mockSender struct{}
14011413

1402-
// dir name of the versioned home
1403-
// used to calculate component paths
1404-
newVersionedDirName := "elastic-agent-3.4.5-SNAPSHOT-ghijkl"
1414+
func (m *mockSender) Send(ctx context.Context, method, path string, params url.Values, headers http.Header, body io.Reader) (*http.Response, error) {
1415+
return nil, nil
1416+
}
14051417

1406-
mockAgentInfo := info.NewAgent(t)
1407-
mockAgentInfo.On("Version").Return(targetVersion.String())
1408-
1409-
upgradeDetails := details.NewDetails(targetVersion.String(), details.StateRequested, "test")
1410-
1411-
mockStdlibFuncs := []common.MockStdLibFuncName{common.CopyFuncName, common.OpenFileFuncName, common.MkdirAllFuncName}
1412-
1413-
testCases := map[string]struct {
1414-
mockStdlibFunc common.MockStdLibFuncName
1415-
mockReturnedError error
1416-
expectedError error
1417-
}{}
1418-
1419-
for _, mockStdlibFunc := range mockStdlibFuncs {
1420-
for _, te := range downloadErrors.OS_DiskSpaceErrors {
1421-
testCases[fmt.Sprintf("unpack_should_return_error_if_unpack_%s_fails: %v", mockStdlibFunc, te)] = struct {
1422-
mockStdlibFunc common.MockStdLibFuncName
1423-
mockReturnedError error
1424-
expectedError error
1425-
}{
1426-
mockStdlibFunc: mockStdlibFunc,
1427-
mockReturnedError: te,
1428-
expectedError: downloadErrors.ErrInsufficientDiskSpace,
1429-
}
1430-
}
1418+
func (m *mockSender) URI() string {
1419+
return "mockURI"
1420+
}
1421+
func TestSetClient(t *testing.T) {
1422+
log, _ := loggertest.New("test")
1423+
upgrader := &Upgrader{
1424+
log: log,
1425+
artifactDownloader: &mockArtifactDownloader{},
14311426
}
14321427

1433-
for name, tc := range testCases {
1434-
t.Run(name, func(t *testing.T) {
1435-
tempDir := t.TempDir()
1436-
paths.SetTop(tempDir)
1437-
paths.SetDownloads(filepath.Join(tempDir, "downloads"))
1438-
1439-
targetArchive, err := createArchive(t, targetArtifactName, targetArchiveFiles)
1440-
require.NoError(t, err)
1441-
1442-
// Used to assert that the mkdirAll error is triggered in the
1443-
// unpacker and not the downloader
1444-
newComponentsDir := filepath.Join(paths.Data(), newVersionedDirName, "components")
1445-
// Used to assert that the openFile error is triggered in the
1446-
// unpacker and not the downloader
1447-
newComponentsFile := filepath.Join(newComponentsDir, "comp1")
1448-
1449-
t.Logf("Created archive: %s", targetArchive)
1450-
1451-
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
1452-
http.ServeFile(w, r, targetArchive)
1453-
}))
1454-
t.Cleanup(server.Close)
1455-
1456-
config := artifact.Config{
1457-
TargetDirectory: paths.Downloads(),
1458-
SourceURI: server.URL,
1459-
RetrySleepInitDuration: 1 * time.Second,
1460-
HTTPTransportSettings: httpcommon.HTTPTransportSettings{
1461-
Timeout: 1 * time.Second,
1462-
},
1463-
}
1464-
1465-
stdlibMocker := common.PrepareStdLibMocks(common.StdLibMocks{
1466-
CopyMock: func(dst io.Writer, src io.Reader) (int64, error) {
1467-
// If the content is not the same as the target archive components file content,
1468-
// write the content to the destination. This is to make
1469-
// sure that the error is triggered in unpacker and not in downloader
1470-
buf, err := io.ReadAll(src)
1471-
require.NoError(t, err)
1472-
if !bytes.Equal(buf, []byte(targetArchiveComponentsFileContent)) {
1473-
return io.Copy(dst, bytes.NewReader(buf))
1474-
}
1475-
1476-
return 0, tc.mockReturnedError
1477-
},
1478-
OpenFileMock: func(name string, flag int, perm os.FileMode) (*os.File, error) {
1479-
// Make sure that the openFile error is triggered in the
1480-
// unpacker and not the downloader
1481-
if name != newComponentsFile {
1482-
return os.OpenFile(name, flag, perm)
1483-
}
1484-
1485-
return nil, tc.mockReturnedError
1486-
},
1487-
MkdirAllMock: func(path string, perm os.FileMode) error {
1488-
// Make sure that the mkdirAll error is triggered in the
1489-
// unpacker and not the downloader
1490-
if path != newComponentsDir {
1491-
return os.MkdirAll(path, perm)
1492-
}
1493-
1494-
return tc.mockReturnedError
1495-
},
1496-
})
1497-
1498-
stdlibMocker(t, tc.mockStdlibFunc)
1499-
1500-
upgrader, err := NewUpgrader(log, &config, mockAgentInfo)
1501-
require.NoError(t, err)
1502-
1503-
_, err = upgrader.Upgrade(context.Background(), targetVersion.String(), server.URL, nil, upgradeDetails, true, true)
1504-
require.ErrorIs(t, err, tc.expectedError, "expected error mismatch")
1505-
require.Equal(t, upgradeDetails.State, details.StateExtracting)
1506-
})
1507-
}
1428+
upgrader.SetClient(&mockSender{})
1429+
require.Equal(t, "mockURI", upgrader.artifactDownloader.(*mockArtifactDownloader).fleetServerURI)
15081430
}

0 commit comments

Comments
 (0)