Inline Delegating- and InternalBuildService

8 files changed, 23 insertions, 88 deletions

diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/BuildService.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/BuildService.java
index e91a5909f80..56c2ee8da6b 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/BuildService.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/BuildService.java
@@ -20,11 +20,6 @@ public interface BuildService {
      */
     JobState stateOf(BuildJob buildJob);
 
-    /**
-     * Returns whether the given build job should be performed by this build service.
-     */
-    default boolean builds(BuildJob buildJob) { return true; }
-
     enum JobState {
 
         /** Job is not running, and may be triggered. */
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java
index 790d6d00035..f8be9f55b84 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java
@@ -26,8 +26,6 @@ import com.yahoo.vespa.hosted.controller.api.integration.routing.GlobalRoutingSe
 import com.yahoo.vespa.hosted.controller.api.integration.routing.RotationStatus;
 import com.yahoo.vespa.hosted.controller.api.integration.routing.RoutingGenerator;
 import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneRegistry;
-import com.yahoo.vespa.hosted.controller.deployment.DelegatingBuildService;
-import com.yahoo.vespa.hosted.controller.deployment.InternalBuildService;
 import com.yahoo.vespa.hosted.controller.deployment.JobController;
 import com.yahoo.vespa.hosted.controller.persistence.CuratorDb;
 import com.yahoo.vespa.hosted.controller.rotation.Rotation;
@@ -126,8 +124,7 @@ public class Controller extends AbstractComponent {
                                                           configServer,
                                                           Objects.requireNonNull(artifactRepository, "ArtifactRepository cannot be null"),
                                                           Objects.requireNonNull(routingGenerator, "RoutingGenerator cannot be null"),
-                                                          new DelegatingBuildService(Objects.requireNonNull(buildService, "BuildService cannot be null"),
-                                                                                     new InternalBuildService(jobController)),
+                                                          Objects.requireNonNull(buildService, "BuildService cannot be null"),
                                                           clock);
         tenantController = new TenantController(this, curator, athenzClientFactory);
 
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DelegatingBuildService.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DelegatingBuildService.java
deleted file mode 100644
index d2159841c9d..00000000000
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DelegatingBuildService.java
+++ /dev/null
@@ -1,30 +0,0 @@
-package com.yahoo.vespa.hosted.controller.deployment;
-
-import com.yahoo.vespa.hosted.controller.api.integration.BuildService;
-
-/**
- * Sends build jobs to an internal build system whenever it accepts them, or to an external one otherwise.
- *
- * @author jonmv
- */
-public class DelegatingBuildService implements BuildService {
-
-    private final BuildService external;
-    private final BuildService internal;
-
-    public DelegatingBuildService(BuildService external, BuildService internal) {
-        this.external = external;
-        this.internal = internal;
-    }
-
-    @Override
-    public void trigger(BuildJob buildJob) {
-        (internal.builds(buildJob) ? internal : external).trigger(buildJob);
-    }
-
-    @Override
-    public JobState stateOf(BuildJob buildJob) {
-        return (internal.builds(buildJob) ? internal : external).stateOf(buildJob);
-    }
-
-}
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java
index e3b4b4cef8c..5a0cef62c85 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java
@@ -69,11 +69,13 @@ public class DeploymentTrigger {
     private final Controller controller;
     private final Clock clock;
     private final BuildService buildService;
+    private final JobController jobs;
 
     public DeploymentTrigger(Controller controller, BuildService buildService, Clock clock) {
         this.controller = Objects.requireNonNull(controller, "controller cannot be null");
-        this.buildService = Objects.requireNonNull(buildService, "buildService cannot be null");
         this.clock = Objects.requireNonNull(clock, "clock cannot be null");
+        this.buildService = Objects.requireNonNull(buildService, "buildService cannot be null");
+        this.jobs = controller.jobController();
     }
 
     public DeploymentSteps steps(DeploymentSpec spec) {
@@ -161,14 +163,19 @@ public class DeploymentTrigger {
      * Attempts to trigger the given job for the given application and returns the outcome.
      *
      * If the build service can not find the given job, or claims it is illegal to trigger it,
-     * the project id is removed from the application owning the job, to prevent further trigger attemps.
+     * the project id is removed from the application owning the job, to prevent further trigger attempts.
      */
     public boolean trigger(Job job) {
         log.log(LogLevel.INFO, String.format("Triggering %s: %s", job, job.triggering));
         try {
-            buildService.trigger(job);
-            applications().lockOrThrow(job.applicationId(), application ->
-                    applications().store(application.withJobTriggering(job.jobType, job.triggering)));
+            applications().lockOrThrow(job.applicationId(), application -> {
+                if (application.get().deploymentJobs().builtInternally())
+                    jobs.start(job.applicationId(), job.jobType);
+                else
+                    buildService.trigger(job);
+
+                applications().store(application.withJobTriggering(job.jobType, job.triggering));
+            });
             return true;
         }
         catch (RuntimeException e) {
@@ -184,6 +191,9 @@ public class DeploymentTrigger {
     public List<JobType> forceTrigger(ApplicationId applicationId, JobType jobType, String user) {
         Application application = applications().require(applicationId);
         if (jobType == component) {
+            if (application.deploymentJobs().builtInternally())
+                throw new IllegalArgumentException(applicationId + " has no component job we can trigger.");
+
             buildService.trigger(BuildJob.of(applicationId, application.deploymentJobs().projectId().getAsLong(), jobType.jobName()));
             return singletonList(component);
         }
@@ -376,6 +386,10 @@ public class DeploymentTrigger {
     }
 
     private JobState jobStateOf(Application application, JobType jobType) {
+        if (application.deploymentJobs().builtInternally()) {
+            Optional<RunStatus> run = controller.jobController().last(application.id(), jobType);
+            return run.isPresent() && ! run.get().hasEnded() ? JobState.running : JobState.idle;
+        }
         return buildService.stateOf(BuildJob.of(application.id(),
                                                 application.deploymentJobs().projectId().getAsLong(),
                                                 jobType.jobName()));
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalBuildService.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalBuildService.java
deleted file mode 100644
index 18b62f5ea0f..00000000000
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalBuildService.java
+++ /dev/null
@@ -1,39 +0,0 @@
-package com.yahoo.vespa.hosted.controller.deployment;
-
-import com.yahoo.vespa.hosted.controller.api.integration.BuildService;
-import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType;
-
-import java.util.Optional;
-
-/**
- * Wraps a JobController as a BuildService.
- *
- * Shall be inlined when the {@link DelegatingBuildService} delegates all jobs to it.
- *
- * @author jonmv
- */
-public class InternalBuildService implements BuildService {
-
-    private final JobController jobs;
-
-    public InternalBuildService(JobController jobs) {
-        this.jobs = jobs;
-    }
-
-    @Override
-    public void trigger(BuildJob buildJob) {
-        jobs.start(buildJob.applicationId(), JobType.fromJobName(buildJob.jobName()));
-    }
-
-    @Override
-    public JobState stateOf(BuildJob buildJob) {
-        Optional<RunStatus> run = jobs.last(buildJob.applicationId(), JobType.fromJobName(buildJob.jobName()));
-        return run.isPresent() && ! run.get().hasEnded() ? JobState.running : JobState.idle;
-    }
-
-    @Override
-    public boolean builds(BuildJob buildJob) {
-        return jobs.builds(buildJob.applicationId());
-    }
-
-}
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RunResult.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RunResult.java
index aaf43097908..c4aac48503f 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RunResult.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RunResult.java
@@ -1,7 +1,7 @@
 package com.yahoo.vespa.hosted.controller.deployment;
 
 /**
- * Outcomes of jobs run by an {@link InternalBuildService}.
+ * Outcomes of jobs run by a {@link JobController}.
  *
  * @author jonmv
  */
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RunStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RunStatus.java
index 0f605c162aa..c6724255a58 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RunStatus.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RunStatus.java
@@ -16,7 +16,7 @@ import static com.yahoo.vespa.hosted.controller.deployment.Step.Status.unfinishe
 import static java.util.Objects.requireNonNull;
 
 /**
- * Immutable class containing status information for a deployment job run by an {@link InternalBuildService}.
+ * Immutable class containing status information for a deployment job run by a {@link JobController}.
  *
  * @author jonmv
  */
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunner.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunner.java
index 7dbf1a2c05e..9fa67cb5f89 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunner.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunner.java
@@ -2,7 +2,6 @@ package com.yahoo.vespa.hosted.controller.maintenance;
 
 import com.yahoo.log.LogLevel;
 import com.yahoo.vespa.hosted.controller.Controller;
-import com.yahoo.vespa.hosted.controller.deployment.InternalBuildService;
 import com.yahoo.vespa.hosted.controller.deployment.JobController;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId;
 import com.yahoo.vespa.hosted.controller.deployment.RunStatus;
@@ -20,9 +19,8 @@ import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.logging.Logger;
 
 /**
- * Advances the set of {@link RunStatus}es for an {@link InternalBuildService}.
+ * Advances the set of {@link RunStatus}es for a {@link JobController}.
  *
- * @see JobController
  * @author jonmv
  */
 public class JobRunner extends Maintainer {