refactor(oortservice): Move to using CloudDriverService instead of Oo…

…rtService to eliminate direct reference to retrofit classes (spinnaker#4114)

Co-authored-by: Adam Jordens <[email protected]>

orca-applications/src/main/groovy/com/netflix/spinnaker/orca/applications/tasks/VerifyApplicationHasNoDependenciesTask.groovy

@@ -21,8 +21,8 @@ import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus
 import com.netflix.spinnaker.orca.api.pipeline.Task
 import com.netflix.spinnaker.orca.api.pipeline.models.StageExecution
 import com.netflix.spinnaker.orca.api.pipeline.TaskResult
+import com.netflix.spinnaker.orca.clouddriver.CloudDriverService
 import com.netflix.spinnaker.orca.clouddriver.MortService
-import com.netflix.spinnaker.orca.clouddriver.OortService
 import com.netflix.spinnaker.orca.front50.model.Application
 import groovy.util.logging.Slf4j
 import org.springframework.beans.factory.annotation.Autowired
@@ -35,7 +35,7 @@ import javax.annotation.Nonnull
 @Slf4j
 class VerifyApplicationHasNoDependenciesTask implements Task {
   @Autowired
-  OortService oortService
+  CloudDriverService cloudDriverService
 
   @Autowired
   MortService mortService
@@ -50,7 +50,7 @@ class VerifyApplicationHasNoDependenciesTask implements Task {
 
     def existingDependencyTypes = []
     try {
-      def oortResult = getOortResult(application.name as String)
+      def oortResult = cloudDriverService.getApplication(application.name as String)
       if (oortResult && oortResult.clusters) {
         existingDependencyTypes << "clusters"
       }
@@ -91,11 +91,6 @@ class VerifyApplicationHasNoDependenciesTask implements Task {
     ]]).build()
   }
 
-  protected Map getOortResult(String applicationName) {
-    def oortResponse = oortService.getApplication(applicationName)
-    return objectMapper.readValue(oortResponse.body.in().text, Map)
-  }
-
   protected List<Map> getMortResults(String applicationName, String type) {
     def mortResults = mortService.getSearchResults(applicationName, type)
     return mortResults ? mortResults[0].results : []

orca-applications/src/test/groovy/com/netflix/spinnaker/orca/applications/tasks/VerifyApplicationHasNoDependenciesTaskSpec.groovy

@@ -18,6 +18,7 @@ package com.netflix.spinnaker.orca.applications.tasks
 
 import com.fasterxml.jackson.databind.ObjectMapper
 import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus
+import com.netflix.spinnaker.orca.clouddriver.CloudDriverService
 import com.netflix.spinnaker.orca.pipeline.model.TaskExecutionImpl
 import spock.lang.Shared
 import spock.lang.Specification
@@ -35,25 +36,26 @@ class VerifyApplicationHasNoDependenciesTaskSpec extends Specification {
     }
   }
 
+  CloudDriverService cloudDriverService = Mock()
+
   @Unroll
   void "should be TERMINAL when application has clusters or security groups"() {
     given:
     def fixedSecurityGroups = securityGroups
     def fixedClusters = clusters
     def task = new VerifyApplicationHasNoDependenciesTask() {
-      @Override
-      protected Map getOortResult(String applicationName) {
-        return [clusters: fixedClusters]
-      }
 
       @Override
       protected List<Map> getMortResults(String applicationName, String type) {
         return fixedSecurityGroups
       }
     }
     task.objectMapper = new ObjectMapper()
+    task.cloudDriverService = cloudDriverService
+    cloudDriverService.getApplication(_) >> [clusters: fixedClusters]
 
     and:
+
     def stage = pipeline.stages.first()
     stage.tasks = [new TaskExecutionImpl(name: "T1"), new TaskExecutionImpl(name: "T2")]
 

orca-clouddriver-provider-titus/src/main/java/com/netflix/spinnaker/orca/clouddriver/tasks/servergroup/TitusInterestingHealthProviderNamesSupplier.java

@@ -16,9 +16,8 @@
 
 package com.netflix.spinnaker.orca.clouddriver.tasks.servergroup;
 
-import com.fasterxml.jackson.databind.ObjectMapper;
 import com.netflix.spinnaker.orca.api.pipeline.models.StageExecution;
-import com.netflix.spinnaker.orca.clouddriver.OortService;
+import com.netflix.spinnaker.orca.clouddriver.CloudDriverService;
 import com.netflix.spinnaker.orca.kato.pipeline.support.SourceResolver;
 import com.netflix.spinnaker.orca.kato.pipeline.support.StageData;
 import java.util.Arrays;
@@ -29,7 +28,6 @@
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Component;
-import retrofit.client.Response;
 
 @Component
 public class TitusInterestingHealthProviderNamesSupplier
@@ -42,16 +40,14 @@ public class TitusInterestingHealthProviderNamesSupplier
   private static final List<String> SUPPORTED_STAGES =
       Arrays.asList("cloneservergroup", "enableservergroup");
 
-  private final OortService oortService;
-  private final ObjectMapper objectMapper;
+  private final CloudDriverService cloudDriverService;
   private final SourceResolver sourceResolver;
 
   @Autowired
   public TitusInterestingHealthProviderNamesSupplier(
-      OortService oortService, SourceResolver sourceResolver, ObjectMapper objectMapper) {
-    this.oortService = oortService;
+      CloudDriverService cloudDriverService, SourceResolver sourceResolver) {
+    this.cloudDriverService = cloudDriverService;
     this.sourceResolver = sourceResolver;
-    this.objectMapper = objectMapper;
   }
 
   @Override
@@ -73,16 +69,15 @@ public List<String> process(String cloudProvider, StageExecution stage) {
         String serverGroupName =
             source.getServerGroupName() != null ? source.getServerGroupName() : source.getAsgName();
 
-        Response response =
-            oortService.getServerGroupFromCluster(
+        Map<String, Object> serverGroup =
+            cloudDriverService.getServerGroupFromCluster(
                 stageData.getApplication(),
                 source.getAccount(),
                 stageData.getCluster(),
                 serverGroupName,
                 source.getRegion(),
                 cloudProvider);
 
-        Map serverGroup = objectMapper.readValue(response.getBody().in(), Map.class);
         Map titusServerGroupLabels = (Map) serverGroup.get("labels");
 
         if (titusServerGroupLabels != null

orca-clouddriver-provider-titus/src/test/groovy/com/netflix/spinnaker/orca/clouddriver/tasks/servergroup/TitusInterestingHealthProviderNamesSupplierSpec.groovy

@@ -16,23 +16,19 @@
 
 package com.netflix.spinnaker.orca.clouddriver.tasks.servergroup
 
-import com.netflix.spinnaker.orca.clouddriver.OortService
-import com.netflix.spinnaker.orca.jackson.OrcaObjectMapper
+import com.netflix.spinnaker.orca.clouddriver.CloudDriverService
 import com.netflix.spinnaker.orca.kato.pipeline.support.SourceResolver
 import com.netflix.spinnaker.orca.pipeline.model.PipelineExecutionImpl
 import com.netflix.spinnaker.orca.pipeline.model.StageExecutionImpl
-import retrofit.client.Response
-import retrofit.mime.TypedString
 import spock.lang.Specification
 import spock.lang.Subject
 import spock.lang.Unroll
 
 class TitusInterestingHealthProviderNamesSupplierSpec extends Specification {
-  def oortService = Mock(OortService)
-  def mapper = OrcaObjectMapper.newInstance()
+  CloudDriverService cloudDriverService = Mock()
 
   @Subject
-  def titusInterestingHealthProviderNamesSupplier = new TitusInterestingHealthProviderNamesSupplier(oortService, new SourceResolver(), mapper)
+  def titusInterestingHealthProviderNamesSupplier = new TitusInterestingHealthProviderNamesSupplier(cloudDriverService, new SourceResolver())
 
   @Unroll
   def "should only support if cloudProvider is titus and stage type in [enableServerGroup, cloneServerGroup]"() {
@@ -56,15 +52,15 @@ class TitusInterestingHealthProviderNamesSupplierSpec extends Specification {
   def "should process interestingHealthNames by inspecting labels on titus serverGroup"() {
     given:
     def stage = new StageExecutionImpl(PipelineExecutionImpl.newPipeline("orca"), "createServerGroup", stageContext)
-    def response = mapper.writeValueAsString([
+    def response = [
       application: "app",
       region: "region",
       account: "test",
       labels: labels
-    ])
+    ]
 
     and:
-    1 * oortService.getServerGroupFromCluster(*_) >> new Response('oort', 200, 'ok', [], new TypedString(response))
+    1 * cloudDriverService.getServerGroupFromCluster(*_) >> response
 
     when:
     def interestingHealthProviderNames = titusInterestingHealthProviderNamesSupplier.process("titus", stage)

orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/OortService.groovy

@@ -125,7 +125,7 @@ interface OortService {
                                    @Path("name") String name)
 
   @GET("/{type}/images/{account}/{region}/{imageId}")
-  List<Map> getByAmiId(@Path("type") String type,
+  List<Map<String, Object>> getByAmiId(@Path("type") String type,
                        @Path("account") String account,
                        @Path("region") String region,
                        @Path("imageId") imageId)
@@ -138,7 +138,7 @@ interface OortService {
                       @QueryMap Map additionalFilters)
 
   @GET("/tags")
-  List<Map> getEntityTags(@Query("cloudProvider") String cloudProvider,
+  List<Map<String, Object>> getEntityTags(@Query("cloudProvider") String cloudProvider,
                           @Query("entityType") String entityType,
                           @Query("entityId") String entityId,
                           @Query("account") String account,

orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/tasks/instance/AbstractInstancesCheckTask.groovy

@@ -17,17 +17,16 @@
 package com.netflix.spinnaker.orca.clouddriver.tasks.instance
 
 import com.netflix.spinnaker.orca.api.pipeline.models.StageExecution
+import com.netflix.spinnaker.orca.clouddriver.CloudDriverService
 import com.netflix.spinnaker.orca.clouddriver.utils.MonikerHelper
 
 import java.time.Duration
 import java.util.concurrent.TimeUnit
-import com.fasterxml.jackson.databind.ObjectMapper
 import com.netflix.frigga.Names
 import com.netflix.spinnaker.moniker.Moniker
 import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus
 import com.netflix.spinnaker.orca.api.pipeline.OverridableTimeoutRetryableTask
 import com.netflix.spinnaker.orca.api.pipeline.TaskResult
-import com.netflix.spinnaker.orca.clouddriver.OortService
 import com.netflix.spinnaker.orca.clouddriver.tasks.AbstractCloudProviderAwareTask
 import com.netflix.spinnaker.orca.clouddriver.tasks.servergroup.ServerGroupCacheForceRefreshTask
 import com.netflix.spinnaker.orca.clouddriver.utils.OortHelper
@@ -43,10 +42,7 @@ abstract class AbstractInstancesCheckTask extends AbstractCloudProviderAwareTask
   long serverGroupWaitTime = TimeUnit.MINUTES.toMillis(10)
 
   @Autowired
-  OortService oortService
-
-  @Autowired
-  ObjectMapper objectMapper
+  CloudDriverService cloudDriverService
 
   @Autowired
   ServerGroupCacheForceRefreshTask serverGroupCacheForceRefreshTask
@@ -213,16 +209,15 @@ abstract class AbstractInstancesCheckTask extends AbstractCloudProviderAwareTask
       Names names = Names.parseName(serverGroupsByRegion.values().flatten()[0])
       def appName = moniker?.app ?: names.app
       def clusterName = moniker?.cluster ?: names.cluster
-      def response = oortService.getCluster(appName, account, clusterName, cloudProvider)
-      def cluster = objectMapper.readValue(response.body.in().text, Map)
+      def cluster = cloudDriverService.getCluster(appName, account, clusterName, cloudProvider)
       return cluster.serverGroups ?: []
     } else {
       def region = serverGroupsByRegion.keySet()[0]
       def serverGroupName = serverGroupsByRegion[region][0]
 
       try {
-        def response = oortService.getServerGroup(account, region, serverGroupName)
-        return [objectMapper.readValue(response.body.in().text, Map)]
+        def response = cloudDriverService.getServerGroup(account, region, serverGroupName)
+        return [response]
       } catch (RetrofitError e) {
         if (e.response?.status != 404 || waitForUpServerGroup()) {
           throw e

orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/utils/OortHelper.groovy

@@ -31,18 +31,25 @@ import retrofit.converter.JacksonConverter
  */
 @Component
 class OortHelper {
-  @Autowired
-  OortService oortService
+
+  private final OortService oortService
+
+  private final ObjectMapper objectMapper
+
+  private final JacksonConverter converter
 
   @Autowired
-  ObjectMapper objectMapper
+  OortHelper(OortService oortService, ObjectMapper objectMapper) {
+    this.oortService = oortService
+    this.objectMapper = objectMapper
+    converter = new JacksonConverter(objectMapper)
+  }
 
   List<Map> getSearchResults(String searchTerm, String type, String platform) {
     convert(oortService.getSearchResults(searchTerm, type, platform), List)
   }
 
   private <T> T convert(Response response, Class<T> type) {
-    def converter = new JacksonConverter(objectMapper)
     try {
       return type.cast(converter.fromBody(response.body, type))
     } catch (ConversionException ce) {
@@ -80,10 +87,10 @@ class OortHelper {
     // infer the app from the cluster prefix since this is used by quip and we want to be able to quick patch different apps from the same pipeline
     def app
     def clusterName
-    if(expectedAsgName) {
+    if (expectedAsgName) {
       app = expectedAsgName.substring(0, expectedAsgName.indexOf("-"))
       clusterName = expectedAsgName.substring(0, expectedAsgName.lastIndexOf("-"))
-    } else if(context?.clusterName?.indexOf("-") > 0) {
+    } else if (context?.clusterName?.indexOf("-") > 0) {
       app = context.clusterName.substring(0, context.clusterName.indexOf("-"))
       clusterName = context.clusterName
     } else {
@@ -101,7 +108,7 @@ class OortHelper {
 
     def region = context.region ?: context.source.region
 
-    if(!region) {
+    if (!region) {
       throw new RuntimeException("unable to determine region")
     }
 
@@ -111,24 +118,24 @@ class OortHelper {
 
     def searchAsg
     if (expectOneAsg) {
-      if(asgsForCluster.size() != 1) {
+      if (asgsForCluster.size() != 1) {
         throw new RuntimeException("there is more than one server group in the cluster : ${clusterName}:${region}")
       }
       searchAsg = asgsForCluster.get(0)
-    } else if(expectedAsgName) {
+    } else if (expectedAsgName) {
       searchAsg = asgsForCluster.findResult {
-        if(it.name == expectedAsgName) {
-        return it
+        if (it.name == expectedAsgName) {
+          return it
         }
       }
-      if(!searchAsg) {
+      if (!searchAsg) {
         throw new RuntimeException("did not find the expected asg name : ${expectedAsgName}")
       }
     }
 
     searchAsg.instances.each { instance ->
       String hostName = instance.publicDnsName
-      if(!hostName || hostName.isEmpty()) { // some instances dont have a public address, fall back to the private ip
+      if (!hostName || hostName.isEmpty()) { // some instances dont have a public address, fall back to the private ip
         hostName = instance.privateIpAddress
       }
 
@@ -146,14 +153,14 @@ class OortHelper {
         }
       }?.status
 
-      if(failIfAnyInstancesUnhealthy && (!healthCheckUrl || !status || status != "UP")) {
+      if (failIfAnyInstancesUnhealthy && (!healthCheckUrl || !status || status != "UP")) {
         throw new RuntimeException("at least one instance is DOWN or in the STARTING state, exiting")
       }
 
       Map instanceInfo = [
-        hostName : hostName,
-        healthCheckUrl : healthCheckUrl,
-        privateIpAddress: instance.privateIpAddress
+          hostName: hostName,
+          healthCheckUrl: healthCheckUrl,
+          privateIpAddress: instance.privateIpAddress
       ]
       instanceMap.put(instance.instanceId, instanceInfo)
     }

orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/kato/tasks/rollingpush/DetermineTerminationCandidatesTask.groovy

@@ -16,12 +16,12 @@
 
 package com.netflix.spinnaker.orca.kato.tasks.rollingpush
 
-import com.fasterxml.jackson.databind.ObjectMapper
-import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus
+
 import com.netflix.spinnaker.orca.api.pipeline.Task
-import com.netflix.spinnaker.orca.api.pipeline.models.StageExecution
 import com.netflix.spinnaker.orca.api.pipeline.TaskResult
-import com.netflix.spinnaker.orca.clouddriver.OortService
+import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus
+import com.netflix.spinnaker.orca.api.pipeline.models.StageExecution
+import com.netflix.spinnaker.orca.clouddriver.CloudDriverService
 import com.netflix.spinnaker.orca.kato.pipeline.support.StageData
 import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.stereotype.Component
@@ -32,18 +32,15 @@ import javax.annotation.Nonnull
 class DetermineTerminationCandidatesTask implements Task {
 
   @Autowired
-  OortService oortService
-  @Autowired
-  ObjectMapper objectMapper
+  CloudDriverService cloudDriverService
   @Autowired(required = false)
   List<ServerGroupInstanceIdCollector> serverGroupInstanceIdCollectors = []
 
   @Nonnull
   @Override
   TaskResult execute(@Nonnull StageExecution stage) {
     def stageData = stage.mapTo(StageData)
-    def response = oortService.getServerGroupFromCluster(stageData.application, stageData.account, stageData.cluster, stage.context.asgName, stage.context.region, stage.context.cloudProvider ?: 'aws')
-    def serverGroup = objectMapper.readValue(response.body.in(), Map)
+    def serverGroup = cloudDriverService.getServerGroupFromCluster(stageData.application, stageData.account, stageData.cluster, stage.context.asgName, stage.context.region, stage.context.cloudProvider ?: 'aws')
     boolean ascending = stage.context.termination?.order != 'newest'
     def serverGroupInstances = serverGroup.instances.sort { ascending ? it.launchTime : -it.launchTime }
     // need to use id instead of instanceIds for titus as the titus API doesn't yet support this yet.