diff options
author | Valerij Fredriksen <freva@users.noreply.github.com> | 2018-10-08 22:51:31 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-10-08 22:51:31 +0200 |
commit | 50ae5ed3ce3bb3c95e39d01a6db0edb49d702bcd (patch) | |
tree | 1f9e4f666da0c91ba9e621ad5740358bd54ef69b /node-admin/src/test/java/com/yahoo/vespa | |
parent | e6c831abd5454e1ac50b34c6e164e7daa88451fb (diff) |
Revert "NodeAdmin: new file helper"
Diffstat (limited to 'node-admin/src/test/java/com/yahoo/vespa')
7 files changed, 423 insertions, 208 deletions
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java index 49a03c454c1..15bb2825738 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java @@ -87,7 +87,7 @@ public class DockerTester implements AutoCloseable { Clock clock = Clock.systemUTC(); DockerOperations dockerOperations = new DockerOperationsImpl(dockerMock, environment, processExecuter); - StorageMaintainerMock storageMaintainer = new StorageMaintainerMock(dockerOperations, null, environment, callOrderVerifier); + StorageMaintainerMock storageMaintainer = new StorageMaintainerMock(dockerOperations, null, environment, callOrderVerifier, clock); AclMaintainer aclMaintainer = mock(AclMaintainer.class); AthenzCredentialsMaintainer athenzCredentialsMaintainer = mock(AthenzCredentialsMaintainer.class); @@ -95,7 +95,7 @@ public class DockerTester implements AutoCloseable { MetricReceiverWrapper mr = new MetricReceiverWrapper(MetricReceiver.nullImplementation); Function<String, NodeAgent> nodeAgentFactory = (hostName) -> new NodeAgentImpl(hostName, nodeRepositoryMock, orchestratorMock, dockerOperations, storageMaintainer, aclMaintainer, environment, clock, NODE_AGENT_SCAN_INTERVAL, athenzCredentialsMaintainer); - nodeAdmin = new NodeAdminImpl(dockerOperations, nodeAgentFactory, aclMaintainer, mr, Clock.systemUTC()); + nodeAdmin = new NodeAdminImpl(dockerOperations, nodeAgentFactory, storageMaintainer, aclMaintainer, mr, Clock.systemUTC()); nodeAdminStateUpdater = new NodeAdminStateUpdaterImpl(nodeRepositoryMock, orchestratorMock, storageMaintainer, nodeAdmin, DOCKER_HOST_HOSTNAME, clock, NODE_ADMIN_CONVERGE_STATE_INTERVAL, Optional.of(new ClassLocking())); diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/RunInContainerTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/RunInContainerTest.java index 2a3171762bc..a46defc991b 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/RunInContainerTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/RunInContainerTest.java @@ -245,7 +245,7 @@ public class RunInContainerTest { private final Function<String, NodeAgent> nodeAgentFactory = (hostName) -> new NodeAgentImpl(hostName, nodeRepositoryMock, orchestratorMock, dockerOperationsMock, storageMaintainer, aclMaintainer, environment, Clock.systemUTC(), NODE_AGENT_SCAN_INTERVAL, athenzCredentialsMaintainer); - private final NodeAdmin nodeAdmin = new NodeAdminImpl(dockerOperationsMock, nodeAgentFactory, aclMaintainer, mr, Clock.systemUTC()); + private final NodeAdmin nodeAdmin = new NodeAdminImpl(dockerOperationsMock, nodeAgentFactory, storageMaintainer, aclMaintainer, mr, Clock.systemUTC()); private final NodeAdminStateUpdaterImpl nodeAdminStateUpdater = new NodeAdminStateUpdaterImpl(nodeRepositoryMock, orchestratorMock, storageMaintainer, nodeAdmin, "localhost.test.yahoo.com", Clock.systemUTC(), NODE_ADMIN_CONVERGE_STATE_INTERVAL, Optional.of(new ClassLocking())); diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/StorageMaintainerMock.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/StorageMaintainerMock.java index 62f1a59ecf2..6b7d545c286 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/StorageMaintainerMock.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/StorageMaintainerMock.java @@ -1,13 +1,16 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.node.admin.integrationTests; +import com.yahoo.metrics.simple.MetricReceiver; import com.yahoo.system.ProcessExecuter; import com.yahoo.vespa.hosted.dockerapi.ContainerName; +import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec; import com.yahoo.vespa.hosted.node.admin.docker.DockerOperations; import com.yahoo.vespa.hosted.node.admin.maintenance.StorageMaintainer; import com.yahoo.vespa.hosted.node.admin.component.Environment; +import java.time.Clock; import java.util.Optional; /** @@ -16,8 +19,8 @@ import java.util.Optional; public class StorageMaintainerMock extends StorageMaintainer { private final CallOrderVerifier callOrderVerifier; - public StorageMaintainerMock(DockerOperations dockerOperations, ProcessExecuter processExecuter, Environment environment, CallOrderVerifier callOrderVerifier) { - super(dockerOperations, processExecuter, environment, null); + public StorageMaintainerMock(DockerOperations dockerOperations, ProcessExecuter processExecuter, Environment environment, CallOrderVerifier callOrderVerifier, Clock clock) { + super(dockerOperations, processExecuter, new MetricReceiverWrapper(MetricReceiver.nullImplementation), environment, clock); this.callOrderVerifier = callOrderVerifier; } @@ -35,6 +38,10 @@ public class StorageMaintainerMock extends StorageMaintainer { } @Override + public void cleanNodeAdmin() { + } + + @Override public void cleanupNodeStorage(ContainerName containerName, NodeSpec node) { callOrderVerifier.add("DeleteContainerStorage with " + containerName); } diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/FileHelperTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/FileHelperTest.java new file mode 100644 index 00000000000..6b53bc217c4 --- /dev/null +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/FileHelperTest.java @@ -0,0 +1,324 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.node.admin.maintenance; + +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.time.Duration; +import java.util.Arrays; +import java.util.Collections; +import java.util.Optional; + +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +/** + * @author freva + */ +public class FileHelperTest { + @Rule + public TemporaryFolder folder = new TemporaryFolder(); + + @Before + public void initFiles() throws IOException { + for (int i=0; i<10; i++) { + File temp = folder.newFile("test_" + i + ".json"); + temp.setLastModified(System.currentTimeMillis() - i*Duration.ofSeconds(130).toMillis()); + } + + for (int i=0; i<7; i++) { + File temp = folder.newFile("test_" + i + "_file.test"); + temp.setLastModified(System.currentTimeMillis() - i*Duration.ofSeconds(250).toMillis()); + } + + for (int i=0; i<5; i++) { + File temp = folder.newFile(i + "-abc" + ".json"); + temp.setLastModified(System.currentTimeMillis() - i*Duration.ofSeconds(80).toMillis()); + } + + File temp = folder.newFile("week_old_file.json"); + temp.setLastModified(System.currentTimeMillis() - Duration.ofDays(8).toMillis()); + } + + @Test + public void testDeleteAll() throws IOException { + FileHelper.deleteFiles(folder.getRoot().toPath(), Duration.ZERO, Optional.empty(), false); + + assertEquals(0, getContentsOfDirectory(folder.getRoot()).length); + } + + @Test + public void testDeletePrefix() throws IOException { + FileHelper.deleteFiles(folder.getRoot().toPath(), Duration.ZERO, Optional.of("^test_"), false); + + assertEquals(6, getContentsOfDirectory(folder.getRoot()).length); // 5 abc files + 1 week_old_file + } + + @Test + public void testDeleteSuffix() throws IOException { + FileHelper.deleteFiles(folder.getRoot().toPath(), Duration.ZERO, Optional.of(".json$"), false); + + assertEquals(7, getContentsOfDirectory(folder.getRoot()).length); + } + + @Test + public void testDeletePrefixAndSuffix() throws IOException { + FileHelper.deleteFiles(folder.getRoot().toPath(), Duration.ZERO, Optional.of("^test_.*\\.json$"), false); + + assertEquals(13, getContentsOfDirectory(folder.getRoot()).length); // 5 abc files + 7 test_*_file.test files + week_old_file + } + + @Test + public void testDeleteOld() throws IOException { + FileHelper.deleteFiles(folder.getRoot().toPath(), Duration.ofSeconds(600), Optional.empty(), false); + + assertEquals(13, getContentsOfDirectory(folder.getRoot()).length); // All 23 - 6 (from test_*_.json) - 3 (from test_*_file.test) - 1 week old file + } + + @Test + public void testDeleteWithAllParameters() throws IOException { + FileHelper.deleteFiles(folder.getRoot().toPath(), Duration.ofSeconds(200), Optional.of("^test_.*\\.json$"), false); + + assertEquals(15, getContentsOfDirectory(folder.getRoot()).length); // All 23 - 8 (from test_*_.json) + } + + @Test + public void testDeleteWithSubDirectoriesNoRecursive() throws IOException { + initSubDirectories(); + FileHelper.deleteFiles(folder.getRoot().toPath(), Duration.ZERO, Optional.of("^test_.*\\.json$"), false); + + // 6 test_*.json from test_folder1/ + // + 9 test_*.json and 4 abc_*.json from test_folder2/ + // + 13 test_*.json from test_folder2/subSubFolder2/ + // + 7 test_*_file.test and 5 *-abc.json and 1 week_old_file from root + // + test_folder1/ and test_folder2/ and test_folder2/subSubFolder2/ themselves + assertEquals(48, getNumberOfFilesAndDirectoriesIn(folder.getRoot())); + } + + @Test + public void testDeleteWithSubDirectoriesRecursive() throws IOException { + initSubDirectories(); + FileHelper.deleteFiles(folder.getRoot().toPath(), Duration.ZERO, Optional.of("^test_.*\\.json$"), true); + + // 4 abc_*.json from test_folder2/ + // + 7 test_*_file.test and 5 *-abc.json and 1 week_old_file from root + // + test_folder2/ itself + assertEquals(18, getNumberOfFilesAndDirectoriesIn(folder.getRoot())); + } + + @Test + public void testDeleteFilesWhereFilenameRegexAlsoMatchesDirectories() throws IOException { + initSubDirectories(); + + FileHelper.deleteFiles(folder.getRoot().toPath(), Duration.ZERO, Optional.of("^test_"), false); + + assertEquals(8, getContentsOfDirectory(folder.getRoot()).length); // 5 abc files + 1 week_old_file + 2 directories + } + + @Test + public void testGetContentsOfNonExistingDirectory() { + Path fakePath = Paths.get("/some/made/up/dir/"); + assertEquals(Collections.emptyList(), FileHelper.listContentsOfDirectory(fakePath)); + } + + @Test(expected=IllegalArgumentException.class) + public void testDeleteFilesExceptNMostRecentWithNegativeN() throws IOException { + FileHelper.deleteFilesExceptNMostRecent(folder.getRoot().toPath(), -5); + } + + @Test + public void testDeleteFilesExceptFiveMostRecent() throws IOException { + FileHelper.deleteFilesExceptNMostRecent(folder.getRoot().toPath(), 5); + + assertEquals(5, getContentsOfDirectory(folder.getRoot()).length); + + String[] oldestFiles = {"test_5_file.test", "test_6_file.test", "test_8.json", "test_9.json", "week_old_file.json"}; + String[] remainingFiles = Arrays.stream(getContentsOfDirectory(folder.getRoot())) + .map(File::getName) + .sorted() + .toArray(String[]::new); + + assertArrayEquals(oldestFiles, remainingFiles); + } + + @Test + public void testDeleteFilesExceptNMostRecentWithLargeN() throws IOException { + String[] filesPreDelete = folder.getRoot().list(); + + FileHelper.deleteFilesExceptNMostRecent(folder.getRoot().toPath(), 50); + + assertArrayEquals(filesPreDelete, folder.getRoot().list()); + } + + @Test + public void testDeleteFilesLargerThan10B() throws IOException { + initSubDirectories(); + + File temp1 = new File(folder.getRoot(), "small_file"); + writeNBytesToFile(temp1, 50); + + File temp2 = new File(folder.getRoot(), "some_file"); + writeNBytesToFile(temp2, 20); + + File temp3 = new File(folder.getRoot(), "test_folder1/some_other_file"); + writeNBytesToFile(temp3, 75); + + FileHelper.deleteFilesLargerThan(folder.getRoot().toPath(), 10); + + assertEquals(58, getNumberOfFilesAndDirectoriesIn(folder.getRoot())); + assertFalse(temp1.exists() || temp2.exists() || temp3.exists()); + } + + @Test + public void testDeleteDirectories() throws IOException { + initSubDirectories(); + + FileHelper.deleteDirectories(folder.getRoot().toPath(), Duration.ZERO, Optional.of(".*folder2")); + + //23 files in root + // + 6 in test_folder1 + test_folder1 itself + assertEquals(30, getNumberOfFilesAndDirectoriesIn(folder.getRoot())); + } + + @Test + public void testDeleteDirectoriesBasedOnAge() throws IOException { + initSubDirectories(); + // Create folder3 which is older than maxAge, inside have a single directory, subSubFolder3, inside it which is + // also older than maxAge inside the sub directory, create some files which are newer than maxAge. + // deleteDirectories() should NOT delete folder3 + File subFolder3 = folder.newFolder("test_folder3"); + File subSubFolder3 = folder.newFolder("test_folder3", "subSubFolder3"); + + for (int j=0; j<11; j++) { + File.createTempFile("test_", ".json", subSubFolder3); + } + + subFolder3.setLastModified(System.currentTimeMillis() - Duration.ofHours(1).toMillis()); + subSubFolder3.setLastModified(System.currentTimeMillis() - Duration.ofHours(3).toMillis()); + + FileHelper.deleteDirectories(folder.getRoot().toPath(), Duration.ofSeconds(50), Optional.of(".*folder.*")); + + //23 files in root + // + 13 in test_folder2 + // + 13 in subSubFolder2 + // + 11 in subSubFolder3 + // + test_folder2 + subSubFolder2 + folder3 + subSubFolder3 itself + assertEquals(64, getNumberOfFilesAndDirectoriesIn(folder.getRoot())); + } + + @Test + public void testRecursivelyDeleteDirectory() throws IOException { + initSubDirectories(); + FileHelper.recursiveDelete(folder.getRoot().toPath()); + assertFalse(folder.getRoot().exists()); + } + + @Test + public void testRecursivelyDeleteRegularFile() throws IOException { + File file = folder.newFile(); + assertTrue(file.exists()); + assertTrue(file.isFile()); + FileHelper.recursiveDelete(file.toPath()); + assertFalse(file.exists()); + } + + @Test + public void testRecursivelyDeleteNonExistingFile() throws IOException { + File file = folder.getRoot().toPath().resolve("non-existing-file.json").toFile(); + assertFalse(file.exists()); + FileHelper.recursiveDelete(file.toPath()); + assertFalse(file.exists()); + } + + @Test + public void testInitSubDirectories() throws IOException { + initSubDirectories(); + assertTrue(folder.getRoot().exists()); + assertTrue(folder.getRoot().isDirectory()); + + Path test_folder1 = folder.getRoot().toPath().resolve("test_folder1"); + assertTrue(test_folder1.toFile().exists()); + assertTrue(test_folder1.toFile().isDirectory()); + + Path test_folder2 = folder.getRoot().toPath().resolve("test_folder2"); + assertTrue(test_folder2.toFile().exists()); + assertTrue(test_folder2.toFile().isDirectory()); + + Path subSubFolder2 = test_folder2.resolve("subSubFolder2"); + assertTrue(subSubFolder2.toFile().exists()); + assertTrue(subSubFolder2.toFile().isDirectory()); + } + + @Test + public void testDoesNotFailOnLastModifiedOnSymLink() throws IOException { + Path symPath = folder.getRoot().toPath().resolve("symlink"); + Path fakePath = Paths.get("/some/not/existant/file"); + + Files.createSymbolicLink(symPath, fakePath); + assertTrue(Files.isSymbolicLink(symPath)); + assertFalse(Files.exists(fakePath)); + + // Not possible to set modified time on symlink in java, so just check that it doesn't crash + FileHelper.getLastModifiedTime(symPath).toInstant(); + } + + private void initSubDirectories() throws IOException { + File subFolder1 = folder.newFolder("test_folder1"); + File subFolder2 = folder.newFolder("test_folder2"); + File subSubFolder2 = folder.newFolder("test_folder2", "subSubFolder2"); + + for (int j=0; j<6; j++) { + File temp = File.createTempFile("test_", ".json", subFolder1); + temp.setLastModified(System.currentTimeMillis() - (j+1)*Duration.ofSeconds(60).toMillis()); + } + + for (int j=0; j<9; j++) { + File.createTempFile("test_", ".json", subFolder2); + } + + for (int j=0; j<4; j++) { + File.createTempFile("abc_", ".txt", subFolder2); + } + + for (int j=0; j<13; j++) { + File temp = File.createTempFile("test_", ".json", subSubFolder2); + temp.setLastModified(System.currentTimeMillis() - (j+1)*Duration.ofSeconds(40).toMillis()); + } + + //Must be after all the files have been created + subFolder1.setLastModified(System.currentTimeMillis() - Duration.ofHours(2).toMillis()); + subFolder2.setLastModified(System.currentTimeMillis() - Duration.ofHours(1).toMillis()); + subSubFolder2.setLastModified(System.currentTimeMillis() - Duration.ofHours(3).toMillis()); + } + + private static int getNumberOfFilesAndDirectoriesIn(File folder) { + int total = 0; + for (File file : getContentsOfDirectory(folder)) { + if (file.isDirectory()) { + total += getNumberOfFilesAndDirectoriesIn(file); + } + total++; + } + + return total; + } + + private static void writeNBytesToFile(File file, int nBytes) throws IOException { + Files.write(file.toPath(), new byte[nBytes]); + } + + private static File[] getContentsOfDirectory(File directory) { + File[] directoryContents = directory.listFiles(); + + return directoryContents == null ? new File[0] : directoryContents; + } +} diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java index 7722354a633..d9cce7f80a0 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java @@ -1,12 +1,20 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.node.admin.maintenance; +import com.yahoo.collections.Pair; +import com.yahoo.config.provision.NodeType; +import com.yahoo.metrics.simple.MetricReceiver; import com.yahoo.system.ProcessExecuter; +import com.yahoo.test.ManualClock; +import com.yahoo.vespa.hosted.dockerapi.ContainerName; +import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper; +import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec; import com.yahoo.vespa.hosted.node.admin.config.ConfigServerConfig; import com.yahoo.vespa.hosted.node.admin.docker.DockerNetworking; import com.yahoo.vespa.hosted.node.admin.docker.DockerOperations; import com.yahoo.vespa.hosted.node.admin.component.Environment; import com.yahoo.vespa.hosted.node.admin.component.PathResolver; +import com.yahoo.vespa.hosted.provision.Node; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; @@ -14,15 +22,21 @@ import org.junit.rules.TemporaryFolder; import java.io.File; import java.io.IOException; import java.nio.file.Files; +import java.time.Duration; import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; +import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; /** * @author dybis */ public class StorageMaintainerTest { + private final ManualClock clock = new ManualClock(); private final Environment environment = new Environment.Builder() .configServerConfig(new ConfigServerConfig(new ConfigServerConfig.Builder())) .region("us-east-1") @@ -36,7 +50,7 @@ public class StorageMaintainerTest { private final DockerOperations docker = mock(DockerOperations.class); private final ProcessExecuter processExecuter = mock(ProcessExecuter.class); private final StorageMaintainer storageMaintainer = new StorageMaintainer(docker, processExecuter, - environment, null); + new MetricReceiverWrapper(MetricReceiver.nullImplementation), environment, clock); @Rule public TemporaryFolder folder = new TemporaryFolder(); @@ -57,7 +71,76 @@ public class StorageMaintainerTest { assertEquals(0L, usedBytes); } + @Test + public void testMaintenanceThrottlingAfterSuccessfulMaintenance() { + String hostname = "node-123.us-north-3.test.yahoo.com"; + ContainerName containerName = ContainerName.fromHostname(hostname); + NodeSpec node = new NodeSpec.Builder() + .hostname(hostname) + .state(Node.State.ready) + .nodeType(NodeType.tenant) + .flavor("docker") + .minCpuCores(1) + .minMainMemoryAvailableGb(1) + .minDiskAvailableGb(1) + .build(); + + try { + when(processExecuter.exec(any(String[].class))).thenReturn(new Pair<>(0, "")); + } catch (IOException ignored) { } + storageMaintainer.removeOldFilesFromNode(containerName); + verifyProcessExecuterCalled(1); + // Will not actually run maintenance job until an hour passes + storageMaintainer.removeOldFilesFromNode(containerName); + verifyProcessExecuterCalled(1); + + clock.advance(Duration.ofMinutes(61)); + storageMaintainer.removeOldFilesFromNode(containerName); + verifyProcessExecuterCalled(2); + + // Coredump handling is unthrottled + storageMaintainer.handleCoreDumpsForContainer(containerName, node); + verifyProcessExecuterCalled(3); + + storageMaintainer.handleCoreDumpsForContainer(containerName, node); + verifyProcessExecuterCalled(4); + + // cleanupNodeStorage is unthrottled and it should reset previous times + storageMaintainer.cleanupNodeStorage(containerName, node); + verifyProcessExecuterCalled(5); + storageMaintainer.cleanupNodeStorage(containerName, node); + verifyProcessExecuterCalled(6); + } + + @Test + public void testMaintenanceThrottlingAfterFailedMaintenance() { + String hostname = "node-123.us-north-3.test.yahoo.com"; + ContainerName containerName = ContainerName.fromHostname(hostname); + + try { + when(processExecuter.exec(any(String[].class))) + .thenThrow(new RuntimeException("Something went wrong")) + .thenReturn(new Pair<>(0, "")); + } catch (IOException ignored) { } + + try { + storageMaintainer.removeOldFilesFromNode(containerName); + fail("Maintenance job should've failed!"); + } catch (RuntimeException ignored) { } + verifyProcessExecuterCalled(1); + + // Maintenance job failed, we should be able to immediately re-run it + storageMaintainer.removeOldFilesFromNode(containerName); + verifyProcessExecuterCalled(2); + } + private static void writeNBytesToFile(File file, int nBytes) throws IOException { Files.write(file.toPath(), new byte[nBytes]); } + + private void verifyProcessExecuterCalled(int times) { + try { + verify(processExecuter, times(times)).exec(any(String[].class)); + } catch (IOException ignored) { } + } } diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImplTest.java index 065b039c7fd..efd35cce00e 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImplTest.java @@ -5,6 +5,7 @@ import com.yahoo.metrics.simple.MetricReceiver; import com.yahoo.test.ManualClock; import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper; import com.yahoo.vespa.hosted.node.admin.docker.DockerOperations; +import com.yahoo.vespa.hosted.node.admin.maintenance.StorageMaintainer; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgent; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentImpl; import org.junit.Test; @@ -39,10 +40,11 @@ public class NodeAdminImplTest { private interface NodeAgentFactory extends Function<String, NodeAgent> {} private final DockerOperations dockerOperations = mock(DockerOperations.class); private final Function<String, NodeAgent> nodeAgentFactory = mock(NodeAgentFactory.class); + private final StorageMaintainer storageMaintainer = mock(StorageMaintainer.class); private final Runnable aclMaintainer = mock(Runnable.class); private final ManualClock clock = new ManualClock(); - private final NodeAdminImpl nodeAdmin = new NodeAdminImpl(dockerOperations, nodeAgentFactory, aclMaintainer, + private final NodeAdminImpl nodeAdmin = new NodeAdminImpl(dockerOperations, nodeAgentFactory, storageMaintainer, aclMaintainer, new MetricReceiverWrapper(MetricReceiver.nullImplementation), clock); @Test diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileHelperTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileHelperTest.java deleted file mode 100644 index a3569853122..00000000000 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileHelperTest.java +++ /dev/null @@ -1,201 +0,0 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.node.admin.task.util.file; - -import org.junit.Before; -import org.junit.Rule; -import org.junit.Test; -import org.junit.experimental.runners.Enclosed; -import org.junit.rules.TemporaryFolder; -import org.junit.runner.RunWith; - -import java.io.IOException; -import java.io.UncheckedIOException; -import java.nio.file.Files; -import java.nio.file.Path; -import java.nio.file.Paths; -import java.nio.file.attribute.BasicFileAttributes; -import java.nio.file.attribute.FileTime; -import java.time.Duration; -import java.time.Instant; -import java.util.Arrays; -import java.util.HashSet; -import java.util.LinkedList; -import java.util.List; -import java.util.Set; -import java.util.regex.Pattern; -import java.util.stream.Collectors; -import java.util.stream.Stream; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - -/** - * @author freva - */ -@RunWith(Enclosed.class) -public class FileHelperTest { - - public static class GeneralLogicTests { - @Rule - public TemporaryFolder folder = new TemporaryFolder(); - - @Test - public void delete_all_files_non_recursive() { - int numDeleted = FileHelper.streamFiles(testRoot()) - .delete(); - - assertEquals(3, numDeleted); - assertRecursiveContents("test", "test/file.txt", "test/data.json", "test/subdir-1", "test/subdir-1/file", "test/subdir-2"); - } - - @Test - public void delete_all_files_recursive() { - int numDeleted = FileHelper.streamFiles(testRoot()) - .recursive(true) - .delete(); - - assertEquals(6, numDeleted); - assertRecursiveContents("test", "test/subdir-1", "test/subdir-2"); - } - - @Test - public void delete_with_filter_recursive() { - int numDeleted = FileHelper.streamFiles(testRoot()) - .filterFile(FileHelper.nameEndsWith(".json")) - .recursive(true) - .delete(); - - assertEquals(3, numDeleted); - assertRecursiveContents("test.txt", "test", "test/file.txt", "test/subdir-1", "test/subdir-1/file", "test/subdir-2"); - } - - @Test - public void delete_directory_with_filter() { - int numDeleted = FileHelper.streamDirectories(testRoot()) - .filterDirectory(FileHelper.nameStartsWith("subdir")) - .recursive(true) - .delete(); - - assertEquals(3, numDeleted); - assertRecursiveContents("file-1.json", "test.json", "test.txt", "test", "test/file.txt", "test/data.json"); - } - - @Test - public void delete_all_contents() { - int numDeleted = FileHelper.streamContents(testRoot()) - .recursive(true) - .delete(); - - assertEquals(9, numDeleted); - assertTrue(Files.exists(testRoot())); - assertRecursiveContents(); - } - - @Test - public void delete_everything() { - int numDeleted = FileHelper.streamContents(testRoot()) - .includeBase(true) - .recursive(true) - .delete(); - - assertEquals(10, numDeleted); - assertFalse(Files.exists(testRoot())); - } - - @Before - public void setup() throws IOException { - Path root = testRoot(); - - Files.createFile(root.resolve("file-1.json")); - Files.createFile(root.resolve("test.json")); - Files.createFile(root.resolve("test.txt")); - - Files.createDirectories(root.resolve("test")); - Files.createFile(root.resolve("test/file.txt")); - Files.createFile(root.resolve("test/data.json")); - - Files.createDirectories(root.resolve("test/subdir-1")); - Files.createFile(root.resolve("test/subdir-1/file")); - - Files.createDirectories(root.resolve("test/subdir-2")); - } - - private Path testRoot() { - return folder.getRoot().toPath(); - } - - private void assertRecursiveContents(String... relativePaths) { - Set<String> expectedPaths = new HashSet<>(Arrays.asList(relativePaths)); - Set<String> actualPaths = recursivelyListContents(testRoot()).stream() - .map(testRoot()::relativize) - .map(Path::toString) - .collect(Collectors.toSet()); - - assertEquals(expectedPaths, actualPaths); - } - - private List<Path> recursivelyListContents(Path basePath) { - try (Stream<Path> pathStream = Files.list(basePath)) { - List<Path> paths = new LinkedList<>(); - pathStream.forEach(path -> { - paths.add(path); - if (Files.isDirectory(path)) - paths.addAll(recursivelyListContents(path)); - }); - return paths; - } catch (IOException e) { - throw new UncheckedIOException(e); - } - } - } - - public static class FilterUnitTests { - - private final BasicFileAttributes attributes = mock(BasicFileAttributes.class); - - @Test - public void age_filter_test() { - Path path = Paths.get("/my/fake/path"); - when(attributes.lastModifiedTime()).thenReturn(FileTime.from(Instant.now().minus(Duration.ofHours(1)))); - FileHelper.FileAttributes fileAttributes = new FileHelper.FileAttributes(path, attributes); - - assertFalse(FileHelper.olderThan(Duration.ofMinutes(61)).test(fileAttributes)); - assertTrue(FileHelper.olderThan(Duration.ofMinutes(59)).test(fileAttributes)); - - assertTrue(FileHelper.youngerThan(Duration.ofMinutes(61)).test(fileAttributes)); - assertFalse(FileHelper.youngerThan(Duration.ofMinutes(59)).test(fileAttributes)); - } - - @Test - public void size_filters() { - Path path = Paths.get("/my/fake/path"); - when(attributes.size()).thenReturn(100L); - FileHelper.FileAttributes fileAttributes = new FileHelper.FileAttributes(path, attributes); - - assertFalse(FileHelper.largerThan(101).test(fileAttributes)); - assertTrue(FileHelper.largerThan(99).test(fileAttributes)); - - assertTrue(FileHelper.smallerThan(101).test(fileAttributes)); - assertFalse(FileHelper.smallerThan(99).test(fileAttributes)); - } - - @Test - public void filename_filters() { - Path path = Paths.get("/my/fake/path/some-12352-file.json"); - FileHelper.FileAttributes fileAttributes = new FileHelper.FileAttributes(path, attributes); - - assertTrue(FileHelper.nameStartsWith("some-").test(fileAttributes)); - assertFalse(FileHelper.nameStartsWith("som-").test(fileAttributes)); - - assertTrue(FileHelper.nameEndsWith(".json").test(fileAttributes)); - assertFalse(FileHelper.nameEndsWith("file").test(fileAttributes)); - - assertTrue(FileHelper.nameMatches(Pattern.compile("some-[0-9]+-file.json")).test(fileAttributes)); - assertTrue(FileHelper.nameMatches(Pattern.compile("^some-[0-9]+-file.json$")).test(fileAttributes)); - assertFalse(FileHelper.nameMatches(Pattern.compile("some-[0-9]-file.json")).test(fileAttributes)); - } - } -} |