Skip to content

Commit

Permalink
fix(google): Made instrumentation a little more consistent
Browse files Browse the repository at this point in the history
  • Loading branch information
Eric Wiseblatt committed Mar 10, 2017
1 parent 79b5f80 commit ec3338d
Show file tree
Hide file tree
Showing 11 changed files with 71 additions and 40 deletions.
Expand Up @@ -1198,7 +1198,7 @@ class GCEUtil {
return loadBalancerNames
}

def static getTargetProxyFromRule(Compute compute, String project, ForwardingRule forwardingRule, SafeRetry safeRetry, GoogleExecutorTraits executor) {
def static getTargetProxyFromRule(Compute compute, String project, ForwardingRule forwardingRule, String phase, SafeRetry safeRetry, GoogleExecutorTraits executor) {
String target = forwardingRule.getTarget()
GoogleTargetProxyType targetProxyType = Utils.getTargetProxyType(target)
String targetProxyName = getLocalName(target)
Expand All @@ -1209,7 +1209,7 @@ class GCEUtil {
case GoogleTargetProxyType.HTTP:
proxyGet = { executor.timeExecute(
compute.targetHttpProxies().get(project, targetProxyName),
"compute.targetHttpProxies",
"compute.targetHttpProxies.get",
executor.TAG_SCOPE, executor.SCOPE_GLOBAL)
}
operationName = "compute.targetHttpProxies.get"
Expand Down Expand Up @@ -1241,7 +1241,7 @@ class GCEUtil {
null,
[400, 403, 412],
[],
[action: "get", operation: operationName],
[action: "get", phase: phase, operation: operationName, (executor.TAG_SCOPE): executor.SCOPE_GLOBAL],
executor.registry
)
return retrievedTargetProxy
Expand All @@ -1256,6 +1256,7 @@ class GCEUtil {
static Operation deleteGlobalListener(Compute compute,
String project,
String forwardingRuleName,
String phase,
SafeRetry safeRetry,
GoogleExecutorTraits executor) {
ForwardingRule ruleToDelete = safeRetry.doRetry(
Expand All @@ -1268,7 +1269,7 @@ class GCEUtil {
null,
[400, 412],
[404],
[action: "get", operation: "compute.globalForwardingRules.get", (executor.TAG_SCOPE): executor.SCOPE_GLOBAL],
[action: "get", phase: phase, operation: "compute.globalForwardingRules.get", (executor.TAG_SCOPE): executor.SCOPE_GLOBAL],
executor.registry
) as ForwardingRule
if (ruleToDelete) {
Expand Down Expand Up @@ -1320,7 +1321,7 @@ class GCEUtil {
null,
[400, 412],
[404],
[action: "delete", operation: operation_name],
[action: "delete", phase: phase, operation: operation_name, (executor.TAG_SCOPE): executor.SCOPE_GLOBAL],
executor.registry
) as Operation
return result
Expand All @@ -1331,10 +1332,10 @@ class GCEUtil {
String component,
String project,
Task task,
String phase,
Map tags,
SafeRetry safeRetry,
GoogleExecutorTraits executor) {
task.updateStatus phase, "Deleting $component for $project..."
task.updateStatus tags['phase'], "Deleting $component for $project..."
Operation deleteOp
try {
deleteOp = safeRetry.doRetry(
Expand All @@ -1343,7 +1344,7 @@ class GCEUtil {
task,
[400, 412],
[404],
[action: "delete", phase: phase, operation: "DeleteIfNotInUse"],
tags,
executor.registry
) as Operation
} catch (GoogleJsonResponseException e) {
Expand Down
Expand Up @@ -84,37 +84,65 @@ class DestroyGoogleServerGroupAtomicOperation extends GoogleAtomicOperation<Void
task.updateStatus BASE_PHASE, "Checking for autoscaler..."

if (serverGroup.autoscalingPolicy) {
destroy(destroyAutoscaler(compute, serverGroupName, project, region, zone, isRegional), "autoscaler")
def tags = [action: "delete", phase: BASE_PHASE]
if (isRegional) {
tags[TAG_SCOPE] = SCOPE_REGIONAL
tags[TAG_REGION] = region
tags['operation'] = 'compute.regionAutoscalers.delete'
} else {
tags[TAG_SCOPE] = SCOPE_ZONAL
tags[TAG_ZONE] = zone
tags['operation'] = 'compute.autoscalers.delete'
}
destroy(destroyAutoscaler(compute, serverGroupName, project, region, zone, isRegional),
"autoscaler", tags)
task.updateStatus BASE_PHASE, "Deleted autoscaler"
}

task.updateStatus BASE_PHASE, "Checking for associated HTTP(S) load balancer backend services..."

destroy(destroyHttpLoadBalancerBackends(compute, project, serverGroup, googleLoadBalancerProvider), "Http load balancer backends")
destroy(destroyHttpLoadBalancerBackends(compute, project, serverGroup, googleLoadBalancerProvider), "Http load balancer backends",
[action: 'destroy', operation: 'destroyHttpLoadBalancerBackends', phase: BASE_PHASE, (TAG_SCOPE): SCOPE_GLOBAL])

task.updateStatus BASE_PHASE, "Checking for associated Internal load balancer backend services..."

destroy(destroyInternalLoadBalancerBackends(compute, project, serverGroup, googleLoadBalancerProvider), "Internal load balancer backends")
destroy(destroyInternalLoadBalancerBackends(compute, project, serverGroup, googleLoadBalancerProvider), "Internal load balancer backends",
[action: 'destroy', operation: 'destroyInternalLoadBalancerBackends', phase: BASE_PHASE, (TAG_SCOPE): SCOPE_GLOBAL])

task.updateStatus BASE_PHASE, "Checking for associated Ssl load balancer backend services..."

destroy(destroySslLoadBalancerBackends(compute, project, serverGroup, googleLoadBalancerProvider), "Ssl load balancer backends")

destroy(destroyInstanceGroup(compute, serverGroupName, project, region, zone, isRegional), "instance group")
destroy(destroySslLoadBalancerBackends(compute, project, serverGroup, googleLoadBalancerProvider), "Ssl load balancer backends",
[action: 'destroy', operation: 'destroySslLoadBalancerBackends', phase: BASE_PHASE, (TAG_SCOPE): SCOPE_GLOBAL])
if (true) {
// We're putting a nested scope here to define a local tags.
// The "if true" is because groovy wants to make this a closure to the
// previous function call.
def tags = [action: "delete", phase: BASE_PHASE]
if (isRegional) {
tags[TAG_SCOPE] = SCOPE_REGIONAL
tags[TAG_REGION] = region
tags['operation'] = 'compute.regionInstanceGroupManagers.delete'
} else {
tags[TAG_SCOPE] = SCOPE_ZONAL
tags[TAG_ZONE] = zone
tags['operation'] = 'compute.instanceGroupManagers.delete'
}
destroy(destroyInstanceGroup(compute, serverGroupName, project, region, zone, isRegional), "instance group", tags)
}

task.updateStatus BASE_PHASE, "Deleted instance group."

destroy(destroyInstanceTemplate(compute, instanceTemplateName, project), "instance template")
destroy(destroyInstanceTemplate(compute, instanceTemplateName, project), "instance template",
[action: 'delete', operation: 'compute.instanceTemplates.delete', phase: BASE_PHASE, (TAG_SCOPE): SCOPE_GLOBAL])

task.updateStatus BASE_PHASE, "Deleted instance template."

task.updateStatus BASE_PHASE, "Deleted instance template."
task.updateStatus BASE_PHASE, "Done destroying server group $serverGroupName in $region."
null
}

void destroy(Closure operation, String resource) {
safeRetry.doRetry(operation, resource, task, RETRY_ERROR_CODES, SUCCESSFUL_ERROR_CODES,
[action: "destroy", phase: BASE_PHASE, (TAG_SCOPE): SCOPE_GLOBAL], registry)
void destroy(Closure operation, String resource, Map tags) {
safeRetry.doRetry(operation, resource, task, RETRY_ERROR_CODES, SUCCESSFUL_ERROR_CODES, tags, registry)
}

Closure destroyInstanceTemplate(Compute compute, String instanceTemplateName, String project) {
Expand Down
Expand Up @@ -100,7 +100,7 @@ class DeleteGoogleHttpLoadBalancerAtomicOperation extends DeleteGoogleLoadBalanc
// Target HTTP(S) proxy.
task.updateStatus BASE_PHASE, "Retrieving target proxy $targetProxyName..."

def retrievedTargetProxy = GCEUtil.getTargetProxyFromRule(compute, project, forwardingRule, safeRetry, this)
def retrievedTargetProxy = GCEUtil.getTargetProxyFromRule(compute, project, forwardingRule, BASE_PHASE, safeRetry, this)

if (!retrievedTargetProxy) {
GCEUtil.updateStatusAndThrowNotFoundException("Target proxy $targetProxyName not found for $project", task,
Expand All @@ -110,7 +110,7 @@ class DeleteGoogleHttpLoadBalancerAtomicOperation extends DeleteGoogleLoadBalanc

List<String> listenersToDelete = []
projectForwardingRules.each { ForwardingRule rule ->
def proxy = GCEUtil.getTargetProxyFromRule(compute, project, rule, safeRetry, this)
def proxy = GCEUtil.getTargetProxyFromRule(compute, project, rule, BASE_PHASE, safeRetry, this)
if (GCEUtil.getLocalName(proxy?.urlMap) == urlMapName) {
listenersToDelete << rule.getName()
}
Expand Down Expand Up @@ -164,7 +164,7 @@ class DeleteGoogleHttpLoadBalancerAtomicOperation extends DeleteGoogleLoadBalanc

listenersToDelete.each { String ruleName ->
task.updateStatus BASE_PHASE, "Deleting listener $ruleName..."
Operation operation = GCEUtil.deleteGlobalListener(compute, project, ruleName, safeRetry, this)
Operation operation = GCEUtil.deleteGlobalListener(compute, project, ruleName, BASE_PHASE, safeRetry, this)
googleOperationPoller.waitForGlobalOperation(compute, project, operation.getName(),
timeoutSeconds, task, "listener " + ruleName, BASE_PHASE)
}
Expand Down Expand Up @@ -199,7 +199,7 @@ class DeleteGoogleHttpLoadBalancerAtomicOperation extends DeleteGoogleLoadBalanc
"Backend service $backendServiceName",
project,
task,
BASE_PHASE,
[action: 'delete', operation: 'compute.backendServices.delete', phase: BASE_PHASE, (TAG_SCOPE): SCOPE_GLOBAL],
safeRetry,
this
)
Expand Down Expand Up @@ -230,7 +230,7 @@ class DeleteGoogleHttpLoadBalancerAtomicOperation extends DeleteGoogleLoadBalanc
"Http health check $healthCheckName",
project,
task,
BASE_PHASE,
[action: 'delete', operation: 'compute.httpHealthChecks.delete', phase: BASE_PHASE, (TAG_SCOPE): SCOPE_GLOBAL],
safeRetry,
this
)
Expand Down
Expand Up @@ -193,7 +193,7 @@ class DeleteGoogleInternalLoadBalancerAtomicOperation extends GoogleAtomicOperat
"Region backend service $backendServiceName",
project,
task,
BASE_PHASE,
[action: 'delete', operation: 'compute.regionBackendServices.delete', phase: BASE_PHASE, (TAG_SCOPE): SCOPE_REGIONAL, (TAG_REGION): region],
safeRetry,
this
)
Expand Down Expand Up @@ -234,7 +234,7 @@ class DeleteGoogleInternalLoadBalancerAtomicOperation extends GoogleAtomicOperat
"Health check $healthCheckName",
project,
task,
BASE_PHASE,
[action: 'delete', operation: 'compute.' + healthCheckType + '.delete', phase: BASE_PHASE, (TAG_SCOPE): SCOPE_GLOBAL],
safeRetry,
this
)
Expand Down
Expand Up @@ -172,7 +172,7 @@ class DeleteGoogleLoadBalancerAtomicOperation extends GoogleAtomicOperation<Void
"Http health check $healthCheckName",
project,
task,
BASE_PHASE,
[action: 'delete', operation: 'compute.httpsHealthChecks.delete', phase: BASE_PHASE, (TAG_SCOPE): SCOPE_GLOBAL],
safeRetry,
this
)
Expand Down
Expand Up @@ -68,7 +68,7 @@ class DeleteGoogleSslLoadBalancerAtomicOperation extends DeleteGoogleLoadBalance

List<ForwardingRule> projectForwardingRules = timeExecute(
compute.globalForwardingRules().list(project),
"compute.globalForwardingRules",
"compute.globalForwardingRules.list",
TAG_SCOPE, SCOPE_GLOBAL).getItems()

ForwardingRule forwardingRule = projectForwardingRules.find { it.name == forwardingRuleName }
Expand All @@ -81,7 +81,7 @@ class DeleteGoogleSslLoadBalancerAtomicOperation extends DeleteGoogleLoadBalance
// Target HTTP(S) proxy.
task.updateStatus BASE_PHASE, "Retrieving target proxy $targetProxyName..."

TargetSslProxy retrievedTargetProxy = GCEUtil.getTargetProxyFromRule(compute, project, forwardingRule, safeRetry, this) as TargetSslProxy
TargetSslProxy retrievedTargetProxy = GCEUtil.getTargetProxyFromRule(compute, project, forwardingRule, BASE_PHASE, safeRetry, this) as TargetSslProxy

if (!retrievedTargetProxy) {
GCEUtil.updateStatusAndThrowNotFoundException("Target proxy $targetProxyName not found for $project", task,
Expand All @@ -91,7 +91,7 @@ class DeleteGoogleSslLoadBalancerAtomicOperation extends DeleteGoogleLoadBalance

List<String> listenersToDelete = []
projectForwardingRules.each { ForwardingRule rule ->
def proxy = GCEUtil.getTargetProxyFromRule(compute, project, rule, safeRetry, this)
def proxy = GCEUtil.getTargetProxyFromRule(compute, project, rule, BASE_PHASE, safeRetry, this)
if (GCEUtil.getLocalName(proxy?.service) == backendServiceName) {
listenersToDelete << rule.getName()
}
Expand Down Expand Up @@ -130,7 +130,7 @@ class DeleteGoogleSslLoadBalancerAtomicOperation extends DeleteGoogleLoadBalance
task,
[400, 403, 412],
[],
[action: "get", phase: BASE_PHASE, opeation: "compute.healthChecks.get"],
[action: "get", phase: BASE_PHASE, operation: "compute.healthChecks.get", (TAG_SCOPE): SCOPE_GLOBAL],
getRegistry()
) as HealthCheck
if (!retrievedHealthCheck) {
Expand All @@ -144,7 +144,7 @@ class DeleteGoogleSslLoadBalancerAtomicOperation extends DeleteGoogleLoadBalance

listenersToDelete.each { String ruleName ->
task.updateStatus BASE_PHASE, "Deleting listener $ruleName..."
Operation operation = GCEUtil.deleteGlobalListener(compute, project, ruleName, safeRetry, this)
Operation operation = GCEUtil.deleteGlobalListener(compute, project, ruleName, BASE_PHASE, safeRetry, this)
googleOperationPoller.waitForGlobalOperation(compute, project, operation.getName(),
timeoutSeconds, task, "listener " + ruleName, BASE_PHASE)
}
Expand All @@ -157,7 +157,7 @@ class DeleteGoogleSslLoadBalancerAtomicOperation extends DeleteGoogleLoadBalance
"Backend service $backendServiceName",
project,
task,
BASE_PHASE,
[action: 'delete', operation: 'compute.backendServices.delete', phase: BASE_PHASE, (TAG_SCOPE): SCOPE_GLOBAL],
safeRetry,
this
)
Expand All @@ -173,7 +173,7 @@ class DeleteGoogleSslLoadBalancerAtomicOperation extends DeleteGoogleLoadBalance
"Health check $healthCheckName",
project,
task,
BASE_PHASE,
[action: 'delete', operation: 'compute.healthChecks.delete', phase: BASE_PHASE, (TAG_SCOPE): SCOPE_GLOBAL],
safeRetry,
this
)
Expand Down
Expand Up @@ -485,7 +485,7 @@ class UpsertGoogleHttpLoadBalancerAtomicOperation extends UpsertGoogleLoadBalanc
// Delete extraneous listeners.
description.listenersToDelete?.each { String forwardingRuleName ->
task.updateStatus BASE_PHASE, "Deleting listener ${forwardingRuleName}..."
GCEUtil.deleteGlobalListener(compute, project, forwardingRuleName, safeRetry, this)
GCEUtil.deleteGlobalListener(compute, project, forwardingRuleName, BASE_PHASE, safeRetry, this)
}

task.updateStatus BASE_PHASE, "Done upserting HTTP load balancer $httpLoadBalancerName"
Expand Down
Expand Up @@ -309,7 +309,7 @@ class UpsertGoogleSslLoadBalancerAtomicOperation extends UpsertGoogleLoadBalance
// Delete extraneous listeners.
description.listenersToDelete?.each { String forwardingRuleName ->
task.updateStatus BASE_PHASE, "Deleting listener ${forwardingRuleName}..."
GCEUtil.deleteGlobalListener(compute, project, forwardingRuleName, safeRetry, this)
GCEUtil.deleteGlobalListener(compute, project, forwardingRuleName, BASE_PHASE, safeRetry, this)
}

task.updateStatus BASE_PHASE, "Done upserting load balancer $description.loadBalancerName."
Expand Down
Expand Up @@ -564,7 +564,7 @@ class DestroyGoogleServerGroupAtomicOperationUnitSpec extends Specification {
destroy.safeRetry = safeRetry
destroy.destroy(
destroy.destroyHttpLoadBalancerBackends(computeMock, PROJECT_NAME, serverGroup, googleLoadBalancerProviderMock),
"Http load balancer backends"
"Http load balancer backends", [action: 'test']
)

then:
Expand Down Expand Up @@ -598,7 +598,7 @@ class DestroyGoogleServerGroupAtomicOperationUnitSpec extends Specification {
when:
destroy.destroy(
destroy.destroyHttpLoadBalancerBackends(computeMock, PROJECT_NAME, serverGroup, googleLoadBalancerProviderMock),
"Http load balancer backends"
"Http load balancer backends", [action: 'test']
)

then:
Expand Down
Expand Up @@ -35,6 +35,7 @@ import spock.lang.Specification
import spock.lang.Subject

class DeleteGoogleHttpLoadBalancerAtomicOperationUnitSpec extends Specification {
private static final BASE_PHASE = "test-phase"
private static final ACCOUNT_NAME = "auto"
private static final PROJECT_NAME = "my_project"
private static final HTTP_LOAD_BALANCER_NAME = "default"
Expand Down Expand Up @@ -482,7 +483,7 @@ class DeleteGoogleHttpLoadBalancerAtomicOperationUnitSpec extends Specification
accountName: ACCOUNT_NAME,
credentials: credentials)
@Subject def operation = new DeleteGoogleHttpLoadBalancerAtomicOperation(description)
GCEUtil.deleteGlobalListener(computeMock, PROJECT_NAME, HTTP_LOAD_BALANCER_NAME, safeRetry, operation) >> targetHttpProxiesDeleteOp
GCEUtil.deleteGlobalListener(computeMock, PROJECT_NAME, HTTP_LOAD_BALANCER_NAME, BASE_PHASE, safeRetry, operation) >> targetHttpProxiesDeleteOp
operation.googleOperationPoller =
new GoogleOperationPoller(
googleConfigurationProperties: new GoogleConfigurationProperties(),
Expand Down
Expand Up @@ -34,6 +34,7 @@ import spock.lang.Specification
import spock.lang.Subject

class DeleteGoogleSslLoadBalancerAtomicOperationUnitSpec extends Specification {
private static final BASE_PHASE = "test-phase"
private static final ACCOUNT_NAME = "auto"
private static final PROJECT_NAME = "my_project"
private static final SSL_LOAD_BALANCER_NAME = "default"
Expand Down Expand Up @@ -228,7 +229,7 @@ class DeleteGoogleSslLoadBalancerAtomicOperationUnitSpec extends Specification {
def targetSslProxiesDeleteOp = new Operation(
name: TARGET_SSL_PROXY_DELETE_OP_NAME,
status: PENDING)
GCEUtil.deleteGlobalListener(computeMock, PROJECT_NAME, SSL_LOAD_BALANCER_NAME, safeRetry) >> targetSslProxiesDeleteOp
GCEUtil.deleteGlobalListener(computeMock, PROJECT_NAME, SSL_LOAD_BALANCER_NAME, BASE_PHASE, safeRetry) >> targetSslProxiesDeleteOp

def globalOperations = Mock(Compute.GlobalOperations)
def targetSslProxiesOperationGet = Mock(Compute.GlobalOperations.Get)
Expand Down

0 comments on commit ec3338d

Please sign in to comment.