diff options
6 files changed, 32 insertions, 49 deletions
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java index 43c8e7c9469..c526b335c90 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java @@ -23,7 +23,6 @@ import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RevisionId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; import com.yahoo.vespa.hosted.controller.application.Change; -import com.yahoo.vespa.hosted.controller.application.Deployment; import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; import com.yahoo.vespa.hosted.controller.deployment.ConvergenceSummary; import com.yahoo.vespa.hosted.controller.deployment.DeploymentStatus; @@ -53,7 +52,6 @@ import java.util.Locale; import java.util.Map; import java.util.NavigableMap; import java.util.Optional; -import java.util.SortedMap; import java.util.stream.Stream; import static com.yahoo.config.application.api.DeploymentSpec.UpgradePolicy.canary; @@ -109,10 +107,10 @@ class JobControllerApiHandlerHelper { int limit = limitStr.map(Integer::parseInt).orElse(Integer.MAX_VALUE); toSlime(cursor.setArray("runs"), runs.values(), application, limit, baseUriForJobType); - controller.applications().decideCloudAccountOf(new DeploymentId(id.application(), - runs.lastEntry().getValue().id().job().type().zone()), // Urgh, must use a job with actual zone. - application.deploymentSpec()) - .ifPresent(cloudAccount -> cursor.setObject("enclave").setString("cloudAccount", cloudAccount.value())); + Optional.ofNullable(runs.lastEntry()) + .map(entry -> new DeploymentId(id.application(), entry.getValue().id().job().type().zone())) // Urgh, must use a job with actual zone. + .flatMap(deployment -> controller.applications().decideCloudAccountOf(deployment, application.deploymentSpec())) + .ifPresent(cloudAccount -> cursor.setObject("enclave").setString("cloudAccount", cloudAccount.value())); return new SlimeJsonResponse(slime); } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/ContainerOperations.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/ContainerOperations.java index 264035b86a1..fa933e9622a 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/ContainerOperations.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/ContainerOperations.java @@ -36,7 +36,7 @@ public class ContainerOperations { public ContainerOperations(ContainerEngine containerEngine, Cgroup cgroup, FileSystem fileSystem, Timer timer) { this.containerEngine = Objects.requireNonNull(containerEngine); - this.imageDownloader = new ContainerImageDownloader(containerEngine); + this.imageDownloader = new ContainerImageDownloader(containerEngine, timer); this.imagePruner = new ContainerImagePruner(containerEngine, timer); this.containerStatsCollector = new ContainerStatsCollector(containerEngine, cgroup, fileSystem); } @@ -62,8 +62,8 @@ public class ContainerOperations { } /** Pull image asynchronously. Returns true if image is still downloading and false if download is complete */ - public boolean pullImageAsyncIfNeeded(TaskContext context, DockerImage dockerImage, RegistryCredentials registryCredentials) { - return !imageDownloader.get(context, dockerImage, registryCredentials); + public boolean pullImageAsyncIfNeeded(TaskContext context, DockerImage dockerImage, RegistryCredentialsProvider credentialsProvider) { + return !imageDownloader.get(context, dockerImage, credentialsProvider); } /** Executes a command inside container identified by given context. Does NOT throw on non-zero exit code */ diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/image/ContainerImageDownloader.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/image/ContainerImageDownloader.java index 1e37e080528..d3327bf5148 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/image/ContainerImageDownloader.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/image/ContainerImageDownloader.java @@ -3,10 +3,13 @@ package com.yahoo.vespa.hosted.node.admin.container.image; import com.yahoo.concurrent.DaemonThreadFactory; import com.yahoo.config.provision.DockerImage; +import com.yahoo.jdisc.Timer; import com.yahoo.vespa.hosted.node.admin.component.TaskContext; import com.yahoo.vespa.hosted.node.admin.container.ContainerEngine; -import com.yahoo.vespa.hosted.node.admin.container.RegistryCredentials; +import com.yahoo.vespa.hosted.node.admin.container.RegistryCredentialsProvider; +import java.time.Duration; +import java.time.Instant; import java.util.Collections; import java.util.HashSet; import java.util.Objects; @@ -26,13 +29,15 @@ public class ContainerImageDownloader { private static final Logger LOG = Logger.getLogger(ContainerImageDownloader.class.getName()); private final ContainerEngine containerEngine; + private final Timer timer; private final ExecutorService executorService = Executors.newSingleThreadExecutor( new DaemonThreadFactory("container-image-downloader")); // Download one image at a time private final Set<DockerImage> pendingDownloads = Collections.synchronizedSet(new HashSet<>()); - public ContainerImageDownloader(ContainerEngine containerEngine) { + public ContainerImageDownloader(ContainerEngine containerEngine, Timer timer) { this.containerEngine = Objects.requireNonNull(containerEngine); + this.timer = Objects.requireNonNull(timer); } /** @@ -40,12 +45,14 @@ public class ContainerImageDownloader { * * @return true if the image download has completed. */ - public boolean get(TaskContext context, DockerImage image, RegistryCredentials registryCredentials) { + public boolean get(TaskContext context, DockerImage image, RegistryCredentialsProvider credentialsProvider) { if (pendingDownloads.contains(image)) return false; if (containerEngine.hasImage(context, image)) return true; executorService.submit(() -> { try { - containerEngine.pullImage(context, image, registryCredentials); + Instant start = timer.currentTime(); + containerEngine.pullImage(context, image, credentialsProvider.get()); + LOG.log(Level.INFO, "Downloaded container image " + image + " in " + Duration.between(start, timer.currentTime())); } catch (RuntimeException e) { LOG.log(Level.SEVERE, "Failed to download container image " + image, e); } finally { diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java index 4c17bfbe039..7fc248024c3 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java @@ -21,7 +21,6 @@ import com.yahoo.vespa.hosted.node.admin.configserver.orchestrator.OrchestratorE import com.yahoo.vespa.hosted.node.admin.container.Container; import com.yahoo.vespa.hosted.node.admin.container.ContainerOperations; import com.yahoo.vespa.hosted.node.admin.container.ContainerResources; -import com.yahoo.vespa.hosted.node.admin.container.RegistryCredentials; import com.yahoo.vespa.hosted.node.admin.container.RegistryCredentialsProvider; import com.yahoo.vespa.hosted.node.admin.maintenance.ContainerWireguardTask; import com.yahoo.vespa.hosted.node.admin.maintenance.StorageMaintainer; @@ -431,9 +430,8 @@ public class NodeAgentImpl implements NodeAgent { NodeSpec node = context.node(); if (node.wantedDockerImage().equals(container.map(c -> c.image()))) return false; - RegistryCredentials credentials = registryCredentialsProvider.get(); return node.wantedDockerImage() - .map(image -> containerOperations.pullImageAsyncIfNeeded(context, image, credentials)) + .map(image -> containerOperations.pullImageAsyncIfNeeded(context, image, registryCredentialsProvider)) .orElse(false); } @@ -487,17 +485,14 @@ public class NodeAgentImpl implements NodeAgent { } switch (node.state()) { - case ready: - case reserved: - case failed: - case inactive: - case parked: + case ready, reserved, failed, inactive, parked -> { storageMaintainer.syncLogs(context, true); + if (node.state() == NodeState.reserved) downloadImageIfNeeded(context, container); removeContainerIfNeededUpdateContainerState(context, container); updateNodeRepoWithCurrentAttributes(context, Optional.empty()); stopServicesIfNeeded(context); - break; - case active: + } + case active -> { storageMaintainer.syncLogs(context, true); storageMaintainer.cleanDiskIfFull(context); storageMaintainer.handleCoreDumpsForContainer(context, container, false); @@ -550,11 +545,8 @@ public class NodeAgentImpl implements NodeAgent { orchestrator.resume(context.hostname().value()); suspendedInOrchestrator = false; } - break; - case provisioned: - nodeRepository.setNodeState(context.hostname().value(), NodeState.ready); - break; - case dirty: + } + case dirty -> { removeContainerIfNeededUpdateContainerState(context, container); context.log(logger, "State is " + node.state() + ", will delete application storage and mark node as ready"); credentialsMaintainers.forEach(maintainer -> maintainer.clearCredentials(context)); @@ -562,9 +554,8 @@ public class NodeAgentImpl implements NodeAgent { storageMaintainer.archiveNodeStorage(context); updateNodeRepoWithCurrentAttributes(context, Optional.empty()); nodeRepository.setNodeState(context.hostname().value(), NodeState.ready); - break; - default: - throw ConvergenceException.ofError("UNKNOWN STATE " + node.state().name()); + } + default -> throw ConvergenceException.ofError("Unexpected state " + node.state().name()); } } diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/image/ContainerImageDownloaderTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/image/ContainerImageDownloaderTest.java index 9fd14e7e665..7f002eee315 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/image/ContainerImageDownloaderTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/image/ContainerImageDownloaderTest.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.node.admin.container.image; import com.yahoo.config.provision.DockerImage; +import com.yahoo.jdisc.test.TestTimer; import com.yahoo.vespa.hosted.node.admin.component.TaskContext; import com.yahoo.vespa.hosted.node.admin.component.TestTaskContext; import com.yahoo.vespa.hosted.node.admin.container.ContainerEngineMock; @@ -21,15 +22,15 @@ public class ContainerImageDownloaderTest { @Timeout(5_000) void test_download() { ContainerEngineMock podman = new ContainerEngineMock().asyncImageDownload(true); - ContainerImageDownloader downloader = new ContainerImageDownloader(podman); + ContainerImageDownloader downloader = new ContainerImageDownloader(podman, new TestTimer()); TaskContext context = new TestTaskContext(); DockerImage image = DockerImage.fromString("registry.example.com/repo/vespa:7.42"); - assertFalse(downloader.get(context, image, RegistryCredentials.none), "Download started"); - assertFalse(downloader.get(context, image, RegistryCredentials.none), "Download pending"); + assertFalse(downloader.get(context, image, () -> RegistryCredentials.none), "Download started"); + assertFalse(downloader.get(context, image, () -> RegistryCredentials.none), "Download pending"); podman.completeDownloadOf(image); boolean downloadCompleted; - while (!(downloadCompleted = downloader.get(context, image, RegistryCredentials.none))) ; + while (!(downloadCompleted = downloader.get(context, image, () -> RegistryCredentials.none))) ; assertTrue(downloadCompleted, "Download completed"); } diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java index 0913e1d040a..ef4d6d849f6 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java @@ -487,20 +487,6 @@ public class NodeAgentImplTest { } @Test - void provisionedNodeIsMarkedAsReady() { - final NodeSpec node = nodeBuilder(NodeState.provisioned) - .wantedDockerImage(dockerImage) - .build(); - - NodeAgentContext context = createContext(node); - NodeAgentImpl nodeAgent = makeNodeAgent(null, false); - when(nodeRepository.getOptionalNode(hostName)).thenReturn(Optional.of(node)); - - nodeAgent.doConverge(context); - verify(nodeRepository, times(1)).setNodeState(eq(hostName), eq(NodeState.ready)); - } - - @Test void testRestartDeadContainerAfterNodeAdminRestart() { final NodeSpec node = nodeBuilder(NodeState.active) .currentDockerImage(dockerImage).wantedDockerImage(dockerImage) |