Skip to content

Commit

Permalink
fix(cache): OnDemandType in no longer an enum (#4673)
Browse files Browse the repository at this point in the history
* fix(cache): OnDemandType in no longer an enum

Plugins that wanted to add an OnDemandType previously wouldn't be able to add to the enum.

* fix use of instance comparison of OnDemandType

* add unit test for OnDemandType
  • Loading branch information
claymccoy committed Jun 15, 2020
1 parent 05d2b68 commit 14c9515
Show file tree
Hide file tree
Showing 31 changed files with 207 additions and 89 deletions.
Expand Up @@ -34,6 +34,7 @@ import com.netflix.spinnaker.clouddriver.appengine.security.AppengineNamedAccoun
import com.netflix.spinnaker.clouddriver.cache.OnDemandAgent
import com.netflix.spinnaker.clouddriver.cache.OnDemandAgent.OnDemandResult
import com.netflix.spinnaker.clouddriver.cache.OnDemandMetricsSupport
import com.netflix.spinnaker.clouddriver.cache.OnDemandType
import groovy.util.logging.Slf4j

import java.util.concurrent.TimeUnit
Expand Down Expand Up @@ -78,12 +79,12 @@ class AppengineLoadBalancerCachingAgent extends AbstractAppengineCachingAgent im
metricsSupport = new OnDemandMetricsSupport(
registry,
this,
"$AppengineCloudProvider.ID:$OnDemandAgent.OnDemandType.LoadBalancer")
"$AppengineCloudProvider.ID:$OnDemandType.LoadBalancer")
}

@Override
boolean handles(OnDemandAgent.OnDemandType type, String cloudProvider) {
type == OnDemandAgent.OnDemandType.LoadBalancer && cloudProvider == AppengineCloudProvider.ID
boolean handles(OnDemandType type, String cloudProvider) {
type == OnDemandType.LoadBalancer && cloudProvider == AppengineCloudProvider.ID
}

@Override
Expand Down
Expand Up @@ -40,6 +40,7 @@ import com.netflix.spinnaker.clouddriver.appengine.provider.view.MutableCacheDat
import com.netflix.spinnaker.clouddriver.appengine.security.AppengineNamedAccountCredentials
import com.netflix.spinnaker.clouddriver.cache.OnDemandAgent
import com.netflix.spinnaker.clouddriver.cache.OnDemandMetricsSupport
import com.netflix.spinnaker.clouddriver.cache.OnDemandType
import groovy.util.logging.Slf4j

import static com.netflix.spinnaker.cats.agent.AgentDataType.Authority.AUTHORITATIVE
Expand Down Expand Up @@ -70,7 +71,7 @@ class AppengineServerGroupCachingAgent extends AbstractAppengineCachingAgent imp
this.metricsSupport = new OnDemandMetricsSupport(
registry,
this,
"$AppengineCloudProvider.ID:$OnDemandAgent.OnDemandType.ServerGroup")
"$AppengineCloudProvider.ID:$OnDemandType.ServerGroup")
}

@Override
Expand All @@ -89,8 +90,8 @@ class AppengineServerGroupCachingAgent extends AbstractAppengineCachingAgent imp
}

@Override
boolean handles(OnDemandAgent.OnDemandType type, String cloudProvider) {
type == OnDemandAgent.OnDemandType.ServerGroup && cloudProvider == AppengineCloudProvider.ID
boolean handles(OnDemandType type, String cloudProvider) {
type == OnDemandType.ServerGroup && cloudProvider == AppengineCloudProvider.ID
}

@Override
Expand Down
Expand Up @@ -34,6 +34,7 @@ import com.netflix.spinnaker.clouddriver.aws.security.AmazonClientProvider
import com.netflix.spinnaker.clouddriver.aws.security.NetflixAmazonCredentials
import com.netflix.spinnaker.clouddriver.cache.OnDemandAgent
import com.netflix.spinnaker.clouddriver.cache.OnDemandMetricsSupport
import com.netflix.spinnaker.clouddriver.cache.OnDemandType
import org.slf4j.Logger
import org.slf4j.LoggerFactory

Expand Down Expand Up @@ -117,12 +118,12 @@ abstract class AbstractAmazonLoadBalancerCachingAgent implements CachingAgent, O
this.region = region
this.objectMapper = objectMapper.copy().enable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS)
this.registry = registry
this.metricsSupport = new OnDemandMetricsSupport(registry, this, amazonCloudProvider.id + ":" + "${amazonCloudProvider.id}:${OnDemandAgent.OnDemandType.LoadBalancer}")
this.metricsSupport = new OnDemandMetricsSupport(registry, this, amazonCloudProvider.id + ":" + "${amazonCloudProvider.id}:${OnDemandType.LoadBalancer}")
}

@Override
boolean handles(OnDemandAgent.OnDemandType type, String cloudProvider) {
type == OnDemandAgent.OnDemandType.LoadBalancer && cloudProvider == amazonCloudProvider.id
boolean handles(OnDemandType type, String cloudProvider) {
type == OnDemandType.LoadBalancer && cloudProvider == amazonCloudProvider.id
}

abstract CacheResult loadDataInternal(ProviderCache providerCache)
Expand Down
Expand Up @@ -33,6 +33,7 @@
import com.netflix.spinnaker.clouddriver.aws.security.NetflixAmazonCredentials;
import com.netflix.spinnaker.clouddriver.cache.OnDemandAgent;
import com.netflix.spinnaker.clouddriver.cache.OnDemandMetricsSupport;
import com.netflix.spinnaker.clouddriver.cache.OnDemandType;
import java.util.*;
import java.util.stream.Collectors;
import lombok.extern.slf4j.Slf4j;
Expand Down
Expand Up @@ -36,6 +36,7 @@ import com.netflix.spinnaker.clouddriver.cache.OnDemandAgent
import com.netflix.spinnaker.clouddriver.cache.OnDemandMetricsSupport
import com.netflix.spinnaker.clouddriver.aws.cache.Keys
import com.netflix.spinnaker.clouddriver.aws.provider.AwsInfrastructureProvider
import com.netflix.spinnaker.clouddriver.cache.OnDemandType
import groovy.util.logging.Slf4j

import static com.netflix.spinnaker.cats.agent.AgentDataType.Authority.AUTHORITATIVE
Expand Down Expand Up @@ -71,7 +72,7 @@ class AmazonSecurityGroupCachingAgent implements CachingAgent, OnDemandAgent, Ac
this.objectMapper = objectMapper
this.registry = registry
this.eddaTimeoutConfig = eddaTimeoutConfig
this.metricsSupport = new OnDemandMetricsSupport(registry, this, "${AmazonCloudProvider.ID}:${OnDemandAgent.OnDemandType.SecurityGroup}")
this.metricsSupport = new OnDemandMetricsSupport(registry, this, "${AmazonCloudProvider.ID}:${OnDemandType.SecurityGroup}")
this.lastModifiedKey = Keys.getSecurityGroupKey('LAST_MODIFIED', 'LAST_MODIFIED', region, account.name, null)
}

Expand Down Expand Up @@ -125,8 +126,8 @@ class AmazonSecurityGroupCachingAgent implements CachingAgent, OnDemandAgent, Ac
}

@Override
boolean handles(OnDemandAgent.OnDemandType type, String cloudProvider) {
type == OnDemandAgent.OnDemandType.SecurityGroup && cloudProvider == AmazonCloudProvider.ID
boolean handles(OnDemandType type, String cloudProvider) {
type == OnDemandType.SecurityGroup && cloudProvider == AmazonCloudProvider.ID
}

@Override
Expand Down
Expand Up @@ -52,6 +52,7 @@ import com.netflix.spinnaker.clouddriver.aws.security.NetflixAmazonCredentials
import com.netflix.spinnaker.clouddriver.cache.OnDemandAgent
import com.netflix.spinnaker.clouddriver.cache.OnDemandMetricsSupport
import com.netflix.spinnaker.clouddriver.aws.data.Keys
import com.netflix.spinnaker.clouddriver.cache.OnDemandType
import org.slf4j.Logger
import org.slf4j.LoggerFactory

Expand Down Expand Up @@ -102,7 +103,7 @@ class ClusterCachingAgent implements CachingAgent, OnDemandAgent, AccountAware,
this.objectMapper = objectMapper.enable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS)
this.registry = registry
this.eddaTimeoutConfig = eddaTimeoutConfig
this.metricsSupport = new OnDemandMetricsSupport(registry, this, "${amazonCloudProvider.id}:${OnDemandAgent.OnDemandType.ServerGroup}")
this.metricsSupport = new OnDemandMetricsSupport(registry, this, "${amazonCloudProvider.id}:${OnDemandType.ServerGroup}")
}

@Override
Expand Down Expand Up @@ -170,8 +171,8 @@ class ClusterCachingAgent implements CachingAgent, OnDemandAgent, AccountAware,
}

@Override
boolean handles(OnDemandAgent.OnDemandType type, String cloudProvider) {
type == OnDemandAgent.OnDemandType.ServerGroup && cloudProvider == amazonCloudProvider.id
boolean handles(OnDemandType type, String cloudProvider) {
type == OnDemandType.ServerGroup && cloudProvider == amazonCloudProvider.id
}

@Override
Expand Down
Expand Up @@ -33,6 +33,7 @@ import com.netflix.spinnaker.clouddriver.aws.cache.Keys
import com.netflix.spinnaker.clouddriver.aws.security.AmazonClientProvider
import com.netflix.spinnaker.clouddriver.aws.security.NetflixAmazonCredentials
import com.netflix.spinnaker.clouddriver.cache.OnDemandAgent
import com.netflix.spinnaker.clouddriver.cache.OnDemandType
import spock.lang.Shared
import spock.lang.Specification
import spock.lang.Subject
Expand Down Expand Up @@ -213,9 +214,9 @@ class AmazonCloudFormationCachingAgentSpec extends Specification {
where:
onDemandType | provider || expected
OnDemandAgent.OnDemandType.CloudFormation | AmazonCloudProvider.ID || true
OnDemandAgent.OnDemandType.CloudFormation | "other" || false
OnDemandAgent.OnDemandType.Job | AmazonCloudProvider.ID || false
OnDemandType.CloudFormation | AmazonCloudProvider.ID || true
OnDemandType.CloudFormation | "other" || false
OnDemandType.Job | AmazonCloudProvider.ID || false
}
@Unroll
Expand Down
Expand Up @@ -28,6 +28,7 @@ import com.netflix.spinnaker.clouddriver.azure.AzureCloudProvider
import com.netflix.spinnaker.clouddriver.azure.resources.common.cache.provider.AzureInfrastructureProvider
import com.netflix.spinnaker.clouddriver.azure.security.AzureCredentials
import com.netflix.spinnaker.clouddriver.cache.OnDemandAgent
import com.netflix.spinnaker.clouddriver.cache.OnDemandType

abstract class AzureCachingAgent implements CachingAgent, OnDemandAgent, AccountAware {

Expand Down Expand Up @@ -84,7 +85,7 @@ abstract class AzureCachingAgent implements CachingAgent, OnDemandAgent, Account
}

@Override
boolean handles(OnDemandAgent.OnDemandType type, String cloudProvider) {
boolean handles(OnDemandType type, String cloudProvider) {
type == getOnDemandType() && cloudProvider == azureCloudProvider.id
}

Expand All @@ -101,7 +102,7 @@ abstract class AzureCachingAgent implements CachingAgent, OnDemandAgent, Account

abstract Boolean validKeys(Map<String, ? extends Object> data)

abstract protected OnDemandAgent.OnDemandType getOnDemandType()
abstract protected OnDemandType getOnDemandType()

def static parseOnDemandCache(Collection<CacheData> results, long lastReadTime) {
List<String> evictions = new ArrayList<String>()
Expand Down
Expand Up @@ -37,6 +37,7 @@ import com.netflix.spinnaker.clouddriver.azure.resources.common.cache.provider.A
import com.netflix.spinnaker.clouddriver.azure.security.AzureCredentials
import com.netflix.spinnaker.clouddriver.cache.OnDemandAgent
import com.netflix.spinnaker.clouddriver.cache.OnDemandMetricsSupport
import com.netflix.spinnaker.clouddriver.cache.OnDemandType
import groovy.transform.WithWriteLock
import groovy.util.logging.Slf4j

Expand All @@ -59,7 +60,7 @@ class AzureAppGatewayCachingAgent extends AzureCachingAgent {
Registry registry) {
super(azureCloudProvider, accountName, creds, region, objectMapper)
this.registry = registry
this.metricsSupport = new OnDemandMetricsSupport(registry, this, "${azureCloudProvider.id}:${OnDemandAgent.OnDemandType.LoadBalancer}")
this.metricsSupport = new OnDemandMetricsSupport(registry, this, "${azureCloudProvider.id}:${OnDemandType.LoadBalancer}")
}

@Override
Expand All @@ -79,13 +80,13 @@ class AzureAppGatewayCachingAgent extends AzureCachingAgent {
}

@Override
OnDemandAgent.OnDemandType getOnDemandType() {
OnDemandAgent.OnDemandType.LoadBalancer
OnDemandType getOnDemandType() {
OnDemandType.LoadBalancer
}

@Override
boolean handles(OnDemandAgent.OnDemandType type, String cloudProvider) {
type == OnDemandAgent.OnDemandType.LoadBalancer && cloudProvider == azureCloudProvider.id
boolean handles(OnDemandType type, String cloudProvider) {
type == OnDemandType.LoadBalancer && cloudProvider == azureCloudProvider.id
}

@Override
Expand Down
Expand Up @@ -36,6 +36,7 @@ import com.netflix.spinnaker.clouddriver.azure.resources.loadbalancer.model.Azur
import com.netflix.spinnaker.clouddriver.azure.security.AzureCredentials
import com.netflix.spinnaker.clouddriver.cache.OnDemandAgent
import com.netflix.spinnaker.clouddriver.cache.OnDemandMetricsSupport
import com.netflix.spinnaker.clouddriver.cache.OnDemandType
import groovy.transform.WithWriteLock
import groovy.util.logging.Slf4j

Expand Down Expand Up @@ -68,7 +69,7 @@ class AzureLoadBalancerCachingAgent implements CachingAgent, OnDemandAgent, Acco
this.region = region
this.objectMapper = objectMapper.enable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS)
this.registry = registry
this.metricsSupport = new OnDemandMetricsSupport(registry, this, "${azureCloudProvider.id}:${OnDemandAgent.OnDemandType.LoadBalancer}")
this.metricsSupport = new OnDemandMetricsSupport(registry, this, "${azureCloudProvider.id}:${OnDemandType.LoadBalancer}")
}

@Override
Expand Down Expand Up @@ -97,8 +98,8 @@ class AzureLoadBalancerCachingAgent implements CachingAgent, OnDemandAgent, Acco
}

@Override
boolean handles(OnDemandAgent.OnDemandType type, String cloudProvider) {
type == OnDemandAgent.OnDemandType.LoadBalancer && cloudProvider == azureCloudProvider.id
boolean handles(OnDemandType type, String cloudProvider) {
type == OnDemandType.LoadBalancer && cloudProvider == azureCloudProvider.id
}

@Override
Expand Down
Expand Up @@ -36,6 +36,7 @@ import com.netflix.spinnaker.clouddriver.azure.resources.securitygroup.model.Azu
import com.netflix.spinnaker.clouddriver.azure.security.AzureCredentials
import com.netflix.spinnaker.clouddriver.cache.OnDemandAgent
import com.netflix.spinnaker.clouddriver.cache.OnDemandMetricsSupport
import com.netflix.spinnaker.clouddriver.cache.OnDemandType
import groovy.transform.WithWriteLock
import groovy.util.logging.Slf4j

Expand Down Expand Up @@ -68,7 +69,7 @@ class AzureSecurityGroupCachingAgent implements CachingAgent, OnDemandAgent, Acc
this.region = region
this.objectMapper = objectMapper.enable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS)
this.registry = registry
this.metricsSupport = new OnDemandMetricsSupport(registry, this, "${azureCloudProvider.id}:${OnDemandAgent.OnDemandType.SecurityGroup}")
this.metricsSupport = new OnDemandMetricsSupport(registry, this, "${azureCloudProvider.id}:${OnDemandType.SecurityGroup}")
}

@Override
Expand Down Expand Up @@ -97,8 +98,8 @@ class AzureSecurityGroupCachingAgent implements CachingAgent, OnDemandAgent, Acc
}

@Override
boolean handles(OnDemandAgent.OnDemandType type, String cloudProvider) {
type == OnDemandAgent.OnDemandType.SecurityGroup && cloudProvider == azureCloudProvider.id
boolean handles(OnDemandType type, String cloudProvider) {
type == OnDemandType.SecurityGroup && cloudProvider == azureCloudProvider.id
}

@Override
Expand Down
Expand Up @@ -32,6 +32,7 @@ import com.netflix.spinnaker.clouddriver.azure.common.cache.AzureCachingAgent
import com.netflix.spinnaker.clouddriver.azure.common.cache.MutableCacheData
import com.netflix.spinnaker.clouddriver.azure.resources.common.cache.Keys
import com.netflix.spinnaker.clouddriver.azure.resources.loadbalancer.model.AzureLoadBalancer
import com.netflix.spinnaker.clouddriver.cache.OnDemandType

import static com.netflix.spinnaker.clouddriver.azure.resources.common.cache.Keys.Namespace.*
import com.netflix.spinnaker.clouddriver.azure.resources.servergroup.model.AzureServerGroupDescription
Expand Down Expand Up @@ -343,8 +344,8 @@ class AzureServerGroupCachingAgent extends AzureCachingAgent {
}

@Override
OnDemandAgent.OnDemandType getOnDemandType() {
OnDemandAgent.OnDemandType.ServerGroup
OnDemandType getOnDemandType() {
OnDemandType.ServerGroup
}

private static void cache(List<CacheData> data, Map<String, CacheData> cacheDataById) {
Expand Down
Expand Up @@ -29,6 +29,7 @@
import com.netflix.spinnaker.cats.cache.DefaultCacheData;
import com.netflix.spinnaker.clouddriver.cache.OnDemandAgent;
import com.netflix.spinnaker.clouddriver.cache.OnDemandMetricsSupport;
import com.netflix.spinnaker.clouddriver.cache.OnDemandType;
import com.netflix.spinnaker.clouddriver.cloudfoundry.cache.ResourceCacheData;
import com.netflix.spinnaker.clouddriver.cloudfoundry.client.CloudFoundryClient;
import com.netflix.spinnaker.clouddriver.cloudfoundry.model.Views;
Expand Down
Expand Up @@ -33,7 +33,7 @@
import com.netflix.spinnaker.cats.cache.CacheData;
import com.netflix.spinnaker.cats.cache.RelationshipCacheFilter;
import com.netflix.spinnaker.cats.provider.ProviderCache;
import com.netflix.spinnaker.clouddriver.cache.OnDemandAgent;
import com.netflix.spinnaker.clouddriver.cache.OnDemandType;
import com.netflix.spinnaker.clouddriver.cloudfoundry.cache.Keys;
import com.netflix.spinnaker.clouddriver.cloudfoundry.cache.ResourceCacheData;
import com.netflix.spinnaker.clouddriver.cloudfoundry.client.model.RouteId;
Expand Down Expand Up @@ -151,7 +151,7 @@ private CacheData setCacheData(

@Override
public boolean handles(OnDemandType type, String cloudProvider) {
return type.equals(OnDemandAgent.OnDemandType.LoadBalancer)
return type.equals(OnDemandType.LoadBalancer)
&& cloudProvider.equals(CloudFoundryProvider.PROVIDER_ID);
}

Expand Down
Expand Up @@ -35,7 +35,7 @@
import com.netflix.spinnaker.cats.cache.DefaultCacheData;
import com.netflix.spinnaker.cats.cache.RelationshipCacheFilter;
import com.netflix.spinnaker.cats.provider.ProviderCache;
import com.netflix.spinnaker.clouddriver.cache.OnDemandAgent;
import com.netflix.spinnaker.clouddriver.cache.OnDemandType;
import com.netflix.spinnaker.clouddriver.cloudfoundry.cache.Keys;
import com.netflix.spinnaker.clouddriver.cloudfoundry.cache.ResourceCacheData;
import com.netflix.spinnaker.clouddriver.cloudfoundry.model.*;
Expand Down Expand Up @@ -132,8 +132,8 @@ public CacheResult loadData(ProviderCache providerCache) {
}

@Override
public boolean handles(OnDemandAgent.OnDemandType type, String cloudProvider) {
return type.equals(OnDemandAgent.OnDemandType.ServerGroup)
public boolean handles(OnDemandType type, String cloudProvider) {
return type.equals(OnDemandType.ServerGroup)
&& cloudProvider.equals(CloudFoundryProvider.PROVIDER_ID);
}

Expand Down
Expand Up @@ -31,6 +31,7 @@
import com.netflix.spinnaker.cats.agent.DefaultCacheResult;
import com.netflix.spinnaker.cats.cache.CacheData;
import com.netflix.spinnaker.cats.provider.ProviderCache;
import com.netflix.spinnaker.clouddriver.cache.OnDemandType;
import com.netflix.spinnaker.clouddriver.cloudfoundry.cache.Keys;
import com.netflix.spinnaker.clouddriver.cloudfoundry.cache.ResourceCacheData;
import com.netflix.spinnaker.clouddriver.cloudfoundry.model.CloudFoundrySpace;
Expand Down
Expand Up @@ -50,17 +50,17 @@ class CatsOnDemandCacheUpdater implements OnDemandCacheUpdater {
}

@Override
boolean handles(OnDemandAgent.OnDemandType type, String cloudProvider) {
boolean handles(OnDemandType type, String cloudProvider) {
onDemandAgents.any { it.handles(type, cloudProvider) }
}

@Override
OnDemandCacheResult handle(OnDemandAgent.OnDemandType type, String cloudProvider, Map<String, ?> data) {
OnDemandCacheResult handle(OnDemandType type, String cloudProvider, Map<String, ?> data) {
Collection<OnDemandAgent> onDemandAgents = onDemandAgents.findAll { it.handles(type, cloudProvider) }
return handle(type, onDemandAgents, data)
}

OnDemandCacheResult handle(OnDemandAgent.OnDemandType type, Collection<OnDemandAgent> onDemandAgents, Map<String, ? extends Object> data) {
OnDemandCacheResult handle(OnDemandType type, Collection<OnDemandAgent> onDemandAgents, Map<String, ? extends Object> data) {
log.debug("Calling handle on data: {}, onDemandAgents: {}, type: {}", data, onDemandAgents, type)
boolean hasOnDemandResults = false
Map<String, List<String>> cachedIdentifiersByType = [:].withDefault { [] }
Expand Down Expand Up @@ -139,7 +139,7 @@ class CatsOnDemandCacheUpdater implements OnDemandCacheUpdater {
}

@Override
Collection<Map> pendingOnDemandRequests(OnDemandAgent.OnDemandType type, String cloudProvider) {
Collection<Map> pendingOnDemandRequests(OnDemandType type, String cloudProvider) {
if (agentScheduler.atomic) {
return []
}
Expand All @@ -152,7 +152,7 @@ class CatsOnDemandCacheUpdater implements OnDemandCacheUpdater {
}

@Override
Map pendingOnDemandRequest(OnDemandAgent.OnDemandType type, String cloudProvider, String id) {
Map pendingOnDemandRequest(OnDemandType type, String cloudProvider, String id) {
if (agentScheduler.atomic) {
return null
}
Expand Down

0 comments on commit 14c9515

Please sign in to comment.