Skip to content

Commit 66935ec

Browse files
author
Kaan Yalti
committed
enhancement(5235): removed separate test for cleanup, added cleanup checks in error handling test
1 parent 66ae14d commit 66935ec

File tree

1 file changed

+115
-35
lines changed

1 file changed

+115
-35
lines changed

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

Lines changed: 115 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ import (
2828

2929
"github.com/elastic/elastic-agent-libs/transport/httpcommon"
3030
"github.com/elastic/elastic-agent-libs/transport/tlscommon"
31-
"github.com/elastic/elastic-agent/internal/pkg/agent/application/info"
3231
"github.com/elastic/elastic-agent/internal/pkg/agent/application/paths"
3332
"github.com/elastic/elastic-agent/internal/pkg/agent/application/upgrade/artifact"
3433
upgradeErrors "github.com/elastic/elastic-agent/internal/pkg/agent/application/upgrade/artifact/download/errors"
@@ -1302,12 +1301,13 @@ func (f *fakeAcker) Commit(ctx context.Context) error {
13021301
}
13031302

13041303
type mockArtifactDownloader struct {
1305-
returnError error
1306-
fleetServerURI string
1304+
returnError error
1305+
returnArchivePath string
1306+
fleetServerURI string
13071307
}
13081308

13091309
func (m *mockArtifactDownloader) downloadArtifact(ctx context.Context, parsedVersion *agtversion.ParsedSemVer, sourceURI string, upgradeDetails *details.Details, skipVerifyOverride, skipDefaultPgp bool, pgpBytes ...string) (_ string, err error) {
1310-
return "", m.returnError
1310+
return m.returnArchivePath, m.returnError
13111311
}
13121312

13131313
func (m *mockArtifactDownloader) withFleetServerURI(fleetServerURI string) {
@@ -1332,39 +1332,49 @@ func (m *mockUnpacker) unpack(version, archivePath, dataDir string, flavor strin
13321332
func TestUpgradeErrorHandling(t *testing.T) {
13331333
log, _ := loggertest.New("test")
13341334
testError := errors.New("test error")
1335-
type upgraderMocker func(upgrader *Upgrader)
1335+
1336+
type upgraderMocker func(upgrader *Upgrader, archivePath string, versionedHome string)
13361337

13371338
type testCase struct {
1338-
isDiskSpaceErrorResult bool
1339-
expectedError error
1340-
upgraderMocker upgraderMocker
1339+
isDiskSpaceErrorResult bool
1340+
expectedError error
1341+
upgraderMocker upgraderMocker
1342+
checkArchiveCleanup bool
1343+
checkVersionedHomeCleanup bool
13411344
}
13421345

13431346
testCases := map[string]testCase{
1344-
"should return error if downloadArtifact fails": {
1347+
"should return error and cleanup downloaded archive if downloadArtifact fails after download is complete": {
13451348
isDiskSpaceErrorResult: false,
13461349
expectedError: testError,
1347-
upgraderMocker: func(upgrader *Upgrader) {
1350+
upgraderMocker: func(upgrader *Upgrader, archivePath string, versionedHome string) {
13481351
upgrader.artifactDownloader = &mockArtifactDownloader{
1349-
returnError: testError,
1352+
returnError: testError,
1353+
returnArchivePath: archivePath,
13501354
}
13511355
},
1356+
checkArchiveCleanup: true,
13521357
},
13531358
"should return error if getPackageMetadata fails": {
13541359
isDiskSpaceErrorResult: false,
13551360
expectedError: testError,
1356-
upgraderMocker: func(upgrader *Upgrader) {
1357-
upgrader.artifactDownloader = &mockArtifactDownloader{}
1361+
upgraderMocker: func(upgrader *Upgrader, archivePath string, versionedHome string) {
1362+
upgrader.artifactDownloader = &mockArtifactDownloader{
1363+
returnArchivePath: archivePath,
1364+
}
13581365
upgrader.unpacker = &mockUnpacker{
13591366
returnPackageMetadataError: testError,
13601367
}
13611368
},
1369+
checkArchiveCleanup: true,
13621370
},
1363-
"should return error if unpack fails": {
1371+
"should return error and cleanup downloaded archive if unpack fails before extracting": {
13641372
isDiskSpaceErrorResult: false,
13651373
expectedError: testError,
1366-
upgraderMocker: func(upgrader *Upgrader) {
1367-
upgrader.artifactDownloader = &mockArtifactDownloader{}
1374+
upgraderMocker: func(upgrader *Upgrader, archivePath string, versionedHome string) {
1375+
upgrader.artifactDownloader = &mockArtifactDownloader{
1376+
returnArchivePath: archivePath,
1377+
}
13681378
upgrader.extractAgentVersion = func(metadata packageMetadata, upgradeVersion string) agentVersion {
13691379
return agentVersion{
13701380
version: upgradeVersion,
@@ -1380,12 +1390,15 @@ func TestUpgradeErrorHandling(t *testing.T) {
13801390
returnUnpackError: testError,
13811391
}
13821392
},
1393+
checkArchiveCleanup: true,
13831394
},
1384-
"should return error if copyActionStore fails": {
1395+
"should return error and cleanup downloaded archive if unpack fails after extracting": {
13851396
isDiskSpaceErrorResult: false,
13861397
expectedError: testError,
1387-
upgraderMocker: func(upgrader *Upgrader) {
1388-
upgrader.artifactDownloader = &mockArtifactDownloader{}
1398+
upgraderMocker: func(upgrader *Upgrader, archivePath string, versionedHome string) {
1399+
upgrader.artifactDownloader = &mockArtifactDownloader{
1400+
returnArchivePath: archivePath,
1401+
}
13891402
upgrader.extractAgentVersion = func(metadata packageMetadata, upgradeVersion string) agentVersion {
13901403
return agentVersion{
13911404
version: upgradeVersion,
@@ -1398,22 +1411,55 @@ func TestUpgradeErrorHandling(t *testing.T) {
13981411
manifest: &v1.PackageManifest{},
13991412
hash: "hash",
14001413
},
1414+
returnUnpackError: testError,
14011415
returnUnpackResult: UnpackResult{
14021416
Hash: "hash",
1403-
VersionedHome: "versionedHome",
1417+
VersionedHome: versionedHome,
1418+
},
1419+
}
1420+
},
1421+
checkArchiveCleanup: true,
1422+
checkVersionedHomeCleanup: true,
1423+
},
1424+
"should return error and cleanup downloaded artifact and extracted archive if copyActionStore fails": {
1425+
isDiskSpaceErrorResult: false,
1426+
expectedError: testError,
1427+
upgraderMocker: func(upgrader *Upgrader, archivePath string, versionedHome string) {
1428+
upgrader.artifactDownloader = &mockArtifactDownloader{
1429+
returnArchivePath: archivePath,
1430+
}
1431+
upgrader.extractAgentVersion = func(metadata packageMetadata, upgradeVersion string) agentVersion {
1432+
return agentVersion{
1433+
version: upgradeVersion,
1434+
snapshot: false,
1435+
hash: metadata.hash,
1436+
}
1437+
}
1438+
upgrader.unpacker = &mockUnpacker{
1439+
returnPackageMetadata: packageMetadata{
1440+
manifest: &v1.PackageManifest{},
1441+
hash: "hash",
1442+
},
1443+
returnUnpackResult: UnpackResult{
1444+
Hash: "hash",
1445+
VersionedHome: versionedHome,
14041446
},
14051447
}
14061448
upgrader.copyActionStore = func(log *logger.Logger, newHome string) error {
14071449
return testError
14081450
}
14091451
},
1452+
checkArchiveCleanup: true,
1453+
checkVersionedHomeCleanup: true,
14101454
},
1411-
"should return error if copyRunDirectory fails": {
1455+
"should return error and cleanup downloaded artifact and extracted archive if copyRunDirectory fails": {
14121456
isDiskSpaceErrorResult: false,
14131457
expectedError: testError,
1414-
upgraderMocker: func(upgrader *Upgrader) {
1415-
upgrader.artifactDownloader = &mockArtifactDownloader{}
1458+
upgraderMocker: func(upgrader *Upgrader, archivePath string, versionedHome string) {
14161459
upgrader.artifactDownloader = &mockArtifactDownloader{}
1460+
upgrader.artifactDownloader = &mockArtifactDownloader{
1461+
returnArchivePath: archivePath,
1462+
}
14171463
upgrader.extractAgentVersion = func(metadata packageMetadata, upgradeVersion string) agentVersion {
14181464
return agentVersion{
14191465
version: upgradeVersion,
@@ -1428,7 +1474,7 @@ func TestUpgradeErrorHandling(t *testing.T) {
14281474
},
14291475
returnUnpackResult: UnpackResult{
14301476
Hash: "hash",
1431-
VersionedHome: "versionedHome",
1477+
VersionedHome: versionedHome,
14321478
},
14331479
}
14341480
upgrader.copyActionStore = func(log *logger.Logger, newHome string) error {
@@ -1438,12 +1484,16 @@ func TestUpgradeErrorHandling(t *testing.T) {
14381484
return testError
14391485
}
14401486
},
1487+
checkArchiveCleanup: true,
1488+
checkVersionedHomeCleanup: true,
14411489
},
1442-
"should return error if changeSymlink fails": {
1490+
"should return error and cleanup downloaded artifact and extracted archive if changeSymlink fails": {
14431491
isDiskSpaceErrorResult: false,
14441492
expectedError: testError,
1445-
upgraderMocker: func(upgrader *Upgrader) {
1446-
upgrader.artifactDownloader = &mockArtifactDownloader{}
1493+
upgraderMocker: func(upgrader *Upgrader, archivePath string, versionedHome string) {
1494+
upgrader.artifactDownloader = &mockArtifactDownloader{
1495+
returnArchivePath: archivePath,
1496+
}
14471497
upgrader.extractAgentVersion = func(metadata packageMetadata, upgradeVersion string) agentVersion {
14481498
return agentVersion{
14491499
version: upgradeVersion,
@@ -1458,7 +1508,7 @@ func TestUpgradeErrorHandling(t *testing.T) {
14581508
},
14591509
returnUnpackResult: UnpackResult{
14601510
Hash: "hash",
1461-
VersionedHome: "versionedHome",
1511+
VersionedHome: versionedHome,
14621512
},
14631513
}
14641514
upgrader.copyActionStore = func(log *logger.Logger, newHome string) error {
@@ -1474,12 +1524,16 @@ func TestUpgradeErrorHandling(t *testing.T) {
14741524
return testError
14751525
}
14761526
},
1527+
checkArchiveCleanup: true,
1528+
checkVersionedHomeCleanup: true,
14771529
},
1478-
"should return error if markUpgrade fails": {
1530+
"should return error and cleanup downloaded artifact and extracted archive if markUpgrade fails": {
14791531
isDiskSpaceErrorResult: false,
14801532
expectedError: testError,
1481-
upgraderMocker: func(upgrader *Upgrader) {
1482-
upgrader.artifactDownloader = &mockArtifactDownloader{}
1533+
upgraderMocker: func(upgrader *Upgrader, archivePath string, versionedHome string) {
1534+
upgrader.artifactDownloader = &mockArtifactDownloader{
1535+
returnArchivePath: archivePath,
1536+
}
14831537
upgrader.extractAgentVersion = func(metadata packageMetadata, upgradeVersion string) agentVersion {
14841538
return agentVersion{
14851539
version: upgradeVersion,
@@ -1494,7 +1548,7 @@ func TestUpgradeErrorHandling(t *testing.T) {
14941548
},
14951549
returnUnpackResult: UnpackResult{
14961550
Hash: "hash",
1497-
VersionedHome: "versionedHome",
1551+
VersionedHome: versionedHome,
14981552
},
14991553
}
15001554
upgrader.copyActionStore = func(log *logger.Logger, newHome string) error {
@@ -1513,13 +1567,15 @@ func TestUpgradeErrorHandling(t *testing.T) {
15131567
return testError
15141568
}
15151569
},
1570+
checkArchiveCleanup: true,
1571+
checkVersionedHomeCleanup: true,
15161572
},
15171573
"should add disk space error to the error chain if downloadArtifact fails with disk space error": {
15181574
isDiskSpaceErrorResult: true,
15191575
expectedError: upgradeErrors.ErrInsufficientDiskSpace,
1520-
upgraderMocker: func(upgrader *Upgrader) {
1576+
upgraderMocker: func(upgrader *Upgrader, archivePath string, versionedHome string) {
15211577
upgrader.artifactDownloader = &mockArtifactDownloader{
1522-
returnError: upgradeErrors.ErrInsufficientDiskSpace,
1578+
returnError: testError,
15231579
}
15241580
},
15251581
},
@@ -1530,17 +1586,41 @@ func TestUpgradeErrorHandling(t *testing.T) {
15301586

15311587
for name, tc := range testCases {
15321588
t.Run(name, func(t *testing.T) {
1589+
baseDir := t.TempDir()
1590+
paths.SetTop(baseDir)
1591+
15331592
upgrader, err := NewUpgrader(log, &artifact.Config{}, mockAgentInfo)
15341593
require.NoError(t, err)
15351594

1536-
tc.upgraderMocker(upgrader)
1595+
tc.upgraderMocker(upgrader, filepath.Join(baseDir, "mockArchive"), "versionedHome")
1596+
1597+
// Create the test files for all the cases
1598+
err = os.WriteFile(filepath.Join(baseDir, "mockArchive"), []byte("test"), 0o600)
1599+
require.NoError(t, err)
1600+
1601+
err = os.WriteFile(filepath.Join(baseDir, "versionedHome"), []byte("test"), 0o600)
1602+
require.NoError(t, err)
15371603

15381604
upgrader.isDiskSpaceErrorFunc = func(err error) bool {
15391605
return tc.isDiskSpaceErrorResult
15401606
}
15411607

15421608
_, err = upgrader.Upgrade(context.Background(), "9.0.0", "", nil, details.NewDetails("9.0.0", details.StateRequested, "test"), true, true)
15431609
require.ErrorIs(t, err, tc.expectedError)
1610+
1611+
// If the downloaded archive needs to be cleaned up assert that it is indeed cleaned up, if not assert that it still exists. The downloaded archive is a mock file that is created for all tests cases.
1612+
if tc.checkArchiveCleanup {
1613+
require.NoFileExists(t, filepath.Join(baseDir, "mockArchive"))
1614+
} else {
1615+
require.FileExists(t, filepath.Join(baseDir, "mockArchive"))
1616+
}
1617+
1618+
// If the extracted agent needs to be cleaned up assert that it is indeed cleaned up, if not assert that it still exists. Versioned home is a mock file that is created for all test cases.
1619+
if tc.checkVersionedHomeCleanup {
1620+
require.NoFileExists(t, filepath.Join(baseDir, "versionedHome"))
1621+
} else {
1622+
require.FileExists(t, filepath.Join(baseDir, "versionedHome"))
1623+
}
15441624
})
15451625
}
15461626
}

0 commit comments

Comments
 (0)