Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(clouddriver) : InstanceService - Eliminated the usages of RetrofitError exception handling and replaced it with SpinnakerRetrofitErrorHandler #4580

Merged
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.netflix.spinnaker.orca.kato.tasks

import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerRetrofitErrorHandler
import com.netflix.spinnaker.orca.api.pipeline.models.StageExecution
import com.netflix.spinnaker.orca.clouddriver.model.Instance.InstanceInfo

Expand Down Expand Up @@ -121,6 +122,7 @@ class JarDiffsTask implements DiffTask {
RestAdapter restAdapter = new RestAdapter.Builder()
.setEndpoint(address)
.setClient(new OkClient(okHttpClient))
.setErrorHandler(SpinnakerRetrofitErrorHandler.getInstance())
.build()

return restAdapter.create(InstanceService.class)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.netflix.spinnaker.orca.kato.tasks.quip

import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerRetrofitErrorHandler
import com.netflix.spinnaker.orca.api.pipeline.Task
import com.netflix.spinnaker.orca.clouddriver.InstanceService
import com.squareup.okhttp.OkHttpClient
Expand All @@ -30,6 +31,7 @@ abstract class AbstractQuipTask implements Task {
.setEndpoint(address)
.setClient(new OkClient(new OkHttpClient(retryOnConnectionFailure: false)))
.setLogLevel(BASIC)
.setErrorHandler(SpinnakerRetrofitErrorHandler.getInstance())
.build()
return restAdapter.create(InstanceService.class)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.netflix.spinnaker.orca.kato.tasks.quip

import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException
import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus
import com.netflix.spinnaker.orca.api.pipeline.RetryableTask
import com.netflix.spinnaker.orca.api.pipeline.models.StageExecution
Expand All @@ -24,7 +25,6 @@ import com.netflix.spinnaker.orca.clouddriver.model.Instance.InstanceInfo
import com.netflix.spinnaker.orca.clouddriver.utils.OortHelper
import org.springframework.beans.factory.annotation.Autowired
import org.springframework.stereotype.Component
import retrofit.RetrofitError

@Component
@Deprecated
Expand Down Expand Up @@ -61,7 +61,7 @@ class InstanceHealthCheckTask extends AbstractQuipTask implements RetryableTask
def instanceService = createInstanceService("http://${host}:${healthCheckUrl.port}")
try { // keep trying until we get a 200 or time out
instanceService.healthCheck(healthCheckUrl.path.substring(1))
} catch (RetrofitError e) {
} catch (SpinnakerServerException e) {
executionStatus = ExecutionStatus.RUNNING
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@
package com.netflix.spinnaker.orca.kato.tasks.quip

import com.fasterxml.jackson.databind.ObjectMapper
import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException
import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus
import com.netflix.spinnaker.orca.api.pipeline.RetryableTask
import com.netflix.spinnaker.orca.api.pipeline.models.StageExecution
import com.netflix.spinnaker.orca.api.pipeline.TaskResult
import org.springframework.beans.factory.annotation.Autowired
import org.springframework.stereotype.Component
import retrofit.RetrofitError
import retrofit.client.Client

@Deprecated
Expand Down Expand Up @@ -69,7 +69,7 @@ class MonitorQuipTask extends AbstractQuipTask implements RetryableTask {
} else {
throw new RuntimeException("quip task failed for ${hostName} with a result of ${status}, see http://${hostName}:5050/tasks/${taskId}")
}
} catch(RetrofitError e) {
} catch(SpinnakerServerException e) {
result = TaskResult.ofStatus(ExecutionStatus.RUNNING)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package com.netflix.spinnaker.orca.kato.tasks.quip

import com.fasterxml.jackson.databind.ObjectMapper
import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException
import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus
import com.netflix.spinnaker.orca.api.pipeline.RetryableTask
import com.netflix.spinnaker.orca.api.pipeline.models.StageExecution
Expand All @@ -25,7 +26,6 @@ import com.netflix.spinnaker.orca.clouddriver.InstanceService
import groovy.util.logging.Slf4j
import org.springframework.beans.factory.annotation.Autowired
import org.springframework.stereotype.Component
import retrofit.RetrofitError
import retrofit.client.Client

@Deprecated
Expand Down Expand Up @@ -75,7 +75,7 @@ class TriggerQuipTask extends AbstractQuipTask implements RetryableTask {
def ref = objectMapper.readValue(instanceResponse.body.in().text, Map).ref
taskIdMap.put(instanceHostName, ref.substring(1 + ref.lastIndexOf('/')))
patchedInstanceIds << instanceId
} catch (RetrofitError e) {
} catch (SpinnakerServerException e) {
log.warn("Error in Quip request: {}", e.message)
}
}
Expand Down Expand Up @@ -108,7 +108,7 @@ class TriggerQuipTask extends AbstractQuipTask implements RetryableTask {
if (version && !version.isEmpty()) {
return version
}
} catch (RetrofitError e) {
} catch (SpinnakerServerException e) {
//retry
}
sleep(instanceVersionSleep)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@
package com.netflix.spinnaker.orca.kato.tasks.quip

import com.fasterxml.jackson.databind.ObjectMapper
import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException
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 org.springframework.beans.factory.annotation.Autowired
import org.springframework.stereotype.Component
import retrofit.RetrofitError
import retrofit.client.Client

import javax.annotation.Nonnull
Expand Down Expand Up @@ -68,7 +68,7 @@ class VerifyQuipTask extends AbstractQuipTask implements Task {
def instanceService = createInstanceService("http://${hostName}:5050")
try {
instanceService.listTasks()
} catch(RetrofitError e) {
} catch(SpinnakerServerException e) {
allInstancesHaveQuip = false
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package com.netflix.spinnaker.orca.kato.tasks

import com.fasterxml.jackson.databind.ObjectMapper
import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException
import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus
import com.netflix.spinnaker.orca.api.pipeline.TaskResult
import com.netflix.spinnaker.orca.clouddriver.InstanceService
Expand Down Expand Up @@ -103,7 +104,7 @@ class JarDiffsTaskSpec extends Specification {
Response jarsResponse = new Response('http://foo.com', 200, 'OK', [], new TypedString(sourceJarsResponse))

when:
1 * instanceService.getJars() >> {throw new RetrofitError(null, null, null, null, null, null, null)}
1 * instanceService.getJars() >> {throw new SpinnakerServerException(new RetrofitError(null, null, null, null, null, null, null))}
1 * instanceService.getJars() >> jarsResponse
def result = task.getJarList([foo: [hostName : "bar"], foo2: [hostName : "bar2"]])

Expand All @@ -121,7 +122,7 @@ class JarDiffsTaskSpec extends Specification {
5 * task.createInstanceService("http://bar:8077") >> instanceService

when:
5 * instanceService.getJars() >> {throw new RetrofitError(null, null, null, null, null, null, null)}
5 * instanceService.getJars() >> {throw new SpinnakerServerException(new RetrofitError(null, null, null, null, null, null, null))}
def result = task.getJarList((1..5).collectEntries { ["${it}": [hostName : "bar"]] })

then:
Expand All @@ -138,7 +139,7 @@ class JarDiffsTaskSpec extends Specification {
Response jarsResponse = new Response('http://foo.com', 200, 'OK', [], new TypedString(sourceJarsResponse))

when:
1 * instanceService.getJars() >> {throw new RetrofitError(null, null, null, null, null, null, null)}
1 * instanceService.getJars() >> {throw new SpinnakerServerException(new RetrofitError(null, null, null, null, null, null, null))}
1 * instanceService.getJars() >> jarsResponse
def result = task.getJarList([foo: [hostName : "bar"], foo2: [hostName : "bar2"], foo3: [hostName : "bar3"]])

Expand Down Expand Up @@ -215,7 +216,7 @@ class JarDiffsTaskSpec extends Specification {
task.oortHelper = oortHelper
1 * oortHelper.getInstancesForCluster(stage.context, "myapp-v000", false) >> sourceExpectedInstances
1 * oortHelper.getInstancesForCluster(stage.context, "myapp-v002", false) >> targetExpectedInstances
1 * instanceService.getJars() >> {throw new RetrofitError(null, null, null, null, null, null, null)}
1 * instanceService.getJars() >> {throw new SpinnakerServerException(new RetrofitError(null, null, null, null, null, null, null))}
1 * instanceService.getJars() >> targetResponse

TaskResult result = task.execute(stage)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.netflix.spinnaker.orca.kato.tasks.quip

import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException
import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus
import com.netflix.spinnaker.orca.clouddriver.InstanceService
import com.netflix.spinnaker.orca.clouddriver.ModelUtils
Expand Down Expand Up @@ -60,7 +61,7 @@ class InstanceHealthCheckTaskSpec extends Specification {
if (responseCode.get(i) == 200) {
1 * instanceService.healthCheck("healthCheck") >> responses.get(i)
} else {
1 * instanceService.healthCheck("healthCheck") >> { throw new RetrofitError(null, null, null, null, null, null, null) }
1 * instanceService.healthCheck("healthCheck") >> { throw new SpinnakerServerException(new RetrofitError(null, null, null, null, null, null, null)) }
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package com.netflix.spinnaker.orca.kato.tasks.quip

import com.fasterxml.jackson.databind.ObjectMapper
import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException
import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus
import com.netflix.spinnaker.orca.api.pipeline.TaskResult
import com.netflix.spinnaker.orca.clouddriver.InstanceService
Expand Down Expand Up @@ -118,15 +119,14 @@ class MonitorQuipTaskSpec extends Specification {
def stage = new StageExecutionImpl(pipe, 'monitorQuip', [:])
stage.context.instances = instances
stage.context.taskIds = taskIds

task.createInstanceService(_) >> instanceService

when:
TaskResult result = task.execute(stage)

then:
taskIds.eachWithIndex { def entry, int i ->
instanceService.listTask(entry.value) >> { throw new RetrofitError(null, null, null, null, null, null, null)}
instanceService.listTask(entry.value) >> { throw new SpinnakerServerException(new RetrofitError(null, null, null, null, null, null, null))}
}
result.status == ExecutionStatus.RUNNING

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@

package com.netflix.spinnaker.orca.kato.tasks.quip

import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerNetworkException
import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException

import java.nio.charset.Charset
import com.fasterxml.jackson.databind.ObjectMapper
import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus
Expand Down Expand Up @@ -176,7 +179,7 @@ class TriggerQuipTaskSpec extends Specification {
// need to do this since I can't stick exceptions on the data table
if (it) {
1 * instanceService.patchInstance(app, patchVersion, "") >> {
throw new RetrofitError(null, null, null, null, null, null, null)
throw new SpinnakerServerException(new RetrofitError(null, null, null, null, null, null, null))
}
} else {
1 * instanceService.patchInstance(app, patchVersion, "") >> instanceResponse
Expand Down Expand Up @@ -250,7 +253,7 @@ class TriggerQuipTaskSpec extends Specification {

then:
2 * instanceService.getCurrentVersion(app) >> {
throw RetrofitError.networkError('http://foo', new IOException('failed'))
throw new SpinnakerNetworkException(RetrofitError.networkError('http://foo', new IOException('failed')))
} >> mkResponse([version: patchVersion])

result.context.skippedInstances.keySet() == ["i-1234"] as Set
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package com.netflix.spinnaker.orca.kato.tasks.quip

import com.fasterxml.jackson.databind.ObjectMapper
import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException
import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus
import com.netflix.spinnaker.orca.api.pipeline.TaskResult
import com.netflix.spinnaker.orca.clouddriver.InstanceService
Expand Down Expand Up @@ -233,7 +234,7 @@ class VerifyQuipTaskSpec extends Specification {
2 * task.createInstanceService(_) >> instanceService
1 * instanceService.listTasks() >> instanceResponse
1 * instanceService.listTasks() >> {
throw new RetrofitError(null, null, null, null, null, null, null)
throw new SpinnakerServerException(new RetrofitError(null, null, null, null, null, null, null))
}
!result?.context
thrown(RuntimeException)
Expand Down