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

feat(upsertScalingPolicyTask): make upsertScalingPolicyTask retryable #2703

Merged
merged 6 commits into from
Mar 7, 2019
Original file line number Diff line number Diff line change
Expand Up @@ -16,31 +16,42 @@

package com.netflix.spinnaker.orca.clouddriver.tasks.scalingpolicy

import java.util.concurrent.TimeUnit
import com.netflix.spinnaker.orca.ExecutionStatus
import com.netflix.spinnaker.orca.Task
import com.netflix.spinnaker.orca.RetryableTask
import com.netflix.spinnaker.orca.TaskResult
import com.netflix.spinnaker.orca.clouddriver.KatoService
import com.netflix.spinnaker.orca.clouddriver.tasks.AbstractCloudProviderAwareTask
import com.netflix.spinnaker.orca.pipeline.model.Stage
import org.springframework.beans.factory.annotation.Autowired
import org.springframework.stereotype.Component
import groovy.util.logging.Slf4j

@Component
class UpsertScalingPolicyTask extends AbstractCloudProviderAwareTask implements Task {
@Slf4j
class UpsertScalingPolicyTask extends AbstractCloudProviderAwareTask implements RetryableTask {

@Autowired
KatoService kato

long backoffPeriod = TimeUnit.SECONDS.toMillis(5)
long timeout = TimeUnit.SECONDS.toMillis(100)

@Override
TaskResult execute(Stage stage) {
def taskId = kato.requestOperations(getCloudProvider(stage), [[upsertScalingPolicy: stage.context]])
try {
def taskId = kato.requestOperations(getCloudProvider(stage), [[upsertScalingPolicy: stage.context]])
.toBlocking()
.first()

new TaskResult(ExecutionStatus.SUCCEEDED, [
new TaskResult(ExecutionStatus.SUCCEEDED, [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stylistically ... even though this is groovy, let's prefix this with a return.

It stands out now that it's wrapped inside a try/catch.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

"deploy.account.name" : stage.context.credentials,
"kato.last.task.id" : taskId,
"deploy.server.groups": [(stage.context.region): [stage.context.serverGroupName]]
])
])
}
catch (Exception e) {
log.error("Failed upsertScalingPolicy task (stageId: ${stage.id}, executionId: ${stage.execution.id})", e)
return new TaskResult(ExecutionStatus.RUNNING)
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
/*
* Copyright 2019 Netflix, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.netflix.spinnaker.orca.clouddriver.tasks.scalingpolicy

import com.netflix.spinnaker.orca.clouddriver.KatoService
import com.netflix.spinnaker.orca.clouddriver.model.TaskId
import com.netflix.spinnaker.orca.pipeline.model.Execution
import com.netflix.spinnaker.orca.pipeline.model.Stage
import spock.lang.Shared
import spock.lang.Specification
import spock.lang.Unroll
import rx.Observable

class UpsertScalingPolicyTaskSpec extends Specification {

@Shared
def taskId = new TaskId(UUID.randomUUID().toString())

@Unroll
def "should retry task on exception"() {

given:
KatoService katoService = Mock(KatoService)
def task = new UpsertScalingPolicyTask(kato: katoService)
def stage = new Stage(Execution.newPipeline("orca"), "upsertScalingPolicy",
[credentials : "abc", cloudProvider: "aCloud",
estimatedInstanceWarmup : "300",
targetValue : "75",
targetTrackingConfiguration:
[predefinedMetricSpecification:
[predefinedMetricType: "ASGAverageCPUUtilization"]]])

when:
def result = task.execute(stage)

then:
1 * katoService.requestOperations(_, _) >> { throw new Exception() }
result.status.toString() == "RUNNING"

}

@Unroll
def "should set the task status to SUCCEEDED for successful execution"() {

given:
KatoService katoService = Mock(KatoService)
def task = new UpsertScalingPolicyTask(kato: katoService)
def stage = new Stage(Execution.newPipeline("orca"), "upsertScalingPolicy",
[credentials : "abc", cloudProvider: "aCloud",
estimatedInstanceWarmup : "300",
targetValue : "75",
targetTrackingConfiguration:
[predefinedMetricSpecification:
[predefinedMetricType: "ASGAverageCPUUtilization"]]])

when:
def result = task.execute(stage)

then:
1 * katoService.requestOperations(_, _) >> { Observable.from(taskId) }
result.status.toString() == "SUCCEEDED"
}
}