From e775f0d8a26b8df1cc59474180844f6115eb9e48 Mon Sep 17 00:00:00 2001 From: Travis Tomsu Date: Wed, 21 Jun 2017 13:50:19 -0400 Subject: [PATCH] feat(cats): Adds cats scheduler from Clouddriver, which will eventually eliminate the need for fiat.writeMode.enabled (#185) --- fiat-roles/fiat-roles.gradle | 2 + .../fiat/config/CatsSchedulerConfig.java | 107 ++++++++++++++++++ .../spinnaker/fiat/config/RedisConfig.java | 4 +- .../RedisPermissionsRepository.java | 3 +- .../internal/ClouddriverService.java | 13 ++- .../providers/internal/Front50Service.java | 14 ++- .../spinnaker/fiat/redis/JedisPoolSource.java | 33 ------ .../spinnaker/fiat/redis/JedisSource.java | 23 ---- .../spinnaker/fiat/roles/UserRolesSyncer.java | 12 +- .../RedisPermissionsRepositorySpec.groovy | 18 +-- .../fiat/roles/UserRolesSyncerSpec.groovy | 2 +- .../fiat/config/ResourcesConfig.java | 5 + .../config/EmbeddedRedisConfig.groovy | 2 +- 13 files changed, 162 insertions(+), 76 deletions(-) create mode 100644 fiat-roles/src/main/java/com/netflix/spinnaker/fiat/config/CatsSchedulerConfig.java delete mode 100644 fiat-roles/src/main/java/com/netflix/spinnaker/fiat/redis/JedisPoolSource.java delete mode 100644 fiat-roles/src/main/java/com/netflix/spinnaker/fiat/redis/JedisSource.java diff --git a/fiat-roles/fiat-roles.gradle b/fiat-roles/fiat-roles.gradle index f58ab6523..c3efd8cdc 100644 --- a/fiat-roles/fiat-roles.gradle +++ b/fiat-roles/fiat-roles.gradle @@ -24,6 +24,8 @@ dependencies { compile spinnaker.dependency("bootActuator") compile spinnaker.dependency("bootWeb") compile spinnaker.dependency("korkHystrix") + // TODO(ttomsu): expose this in spinnaker-dependencies + compile "com.netflix.spinnaker.clouddriver:cats-redis:1.643.0" compile "redis.clients:jedis:2.6.2" compile "com.google.api-client:google-api-client:1.21.0" diff --git a/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/config/CatsSchedulerConfig.java b/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/config/CatsSchedulerConfig.java new file mode 100644 index 000000000..9b1ae66e1 --- /dev/null +++ b/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/config/CatsSchedulerConfig.java @@ -0,0 +1,107 @@ +/* + * Copyright 2017 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.fiat.config; + +import com.netflix.spinnaker.cats.agent.Agent; +import com.netflix.spinnaker.cats.agent.ExecutionInstrumentation; +import com.netflix.spinnaker.cats.redis.JedisSource; +import com.netflix.spinnaker.cats.redis.cluster.AgentIntervalProvider; +import com.netflix.spinnaker.cats.redis.cluster.ClusteredAgentScheduler; +import com.netflix.spinnaker.cats.redis.cluster.DefaultAgentIntervalProvider; +import com.netflix.spinnaker.cats.redis.cluster.DefaultNodeIdentity; +import com.netflix.spinnaker.cats.redis.cluster.DefaultNodeStatusProvider; +import com.netflix.spinnaker.cats.redis.cluster.NodeIdentity; +import com.netflix.spinnaker.cats.redis.cluster.NodeStatusProvider; +import com.netflix.spinnaker.fiat.roles.UserRolesSyncer; +import lombok.extern.slf4j.Slf4j; +import lombok.val; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.boot.autoconfigure.condition.ConditionalOnExpression; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.stereotype.Component; + +import javax.annotation.PreDestroy; +import java.util.concurrent.TimeUnit; + +@Slf4j +@Configuration +@ConditionalOnExpression("${fiat.writeMode.enabled:true}") +public class CatsSchedulerConfig { + + @Autowired + UserRolesSyncer userRolesSyncer; + + ClusteredAgentScheduler scheduler; + + @Value("${fiat.writeMode.syncDelayMs:600000}") + String syncDelayMs; + + @Bean + NodeIdentity nodeIdentity() { + return new DefaultNodeIdentity(); + } + + @Bean + AgentIntervalProvider agentIntervalProvider() { + return new DefaultAgentIntervalProvider(Long.parseLong(syncDelayMs), 10000); + } + + @Bean + NodeStatusProvider nodeStatusProvider() { + return new DefaultNodeStatusProvider(); + } + + @Bean + ClusteredAgentScheduler clusteredAgentScheduler(JedisSource jedisSource, + NodeIdentity nodeIdentity, + AgentIntervalProvider intervalProvider, + NodeStatusProvider nodeStatusProvider, + ExecutionInstrumentation executionInstrumentation) { + scheduler = new ClusteredAgentScheduler(jedisSource, nodeIdentity, intervalProvider, nodeStatusProvider); + scheduler.schedule(userRolesSyncer, userRolesSyncer.getAgentExecution(null), executionInstrumentation); + return scheduler; + } + + @PreDestroy + void cleanup() { + scheduler.unschedule(userRolesSyncer); + } + + @Slf4j + @Component + static class LoggingInstrumentation implements ExecutionInstrumentation { + + @Override + public void executionStarted(Agent agent) { + log.debug("{}:{} starting", agent.getProviderName(), agent.getAgentType()); + } + + @Override + public void executionCompleted(Agent agent, long durationMs) { + log.info("{}:{} completed in {}s", agent.getProviderName(), agent.getAgentType(), TimeUnit.MILLISECONDS.toSeconds(durationMs)); + } + + @Override + public void executionFailed(Agent agent, Throwable cause) { + log.warn(agent.getAgentType() + ":" + agent.getAgentType() + " failed", cause); + } + } +} diff --git a/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/config/RedisConfig.java b/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/config/RedisConfig.java index fbc8198ba..c15c98788 100644 --- a/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/config/RedisConfig.java +++ b/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/config/RedisConfig.java @@ -1,7 +1,7 @@ package com.netflix.spinnaker.fiat.config; -import com.netflix.spinnaker.fiat.redis.JedisPoolSource; -import com.netflix.spinnaker.fiat.redis.JedisSource; +import com.netflix.spinnaker.cats.redis.JedisPoolSource; +import com.netflix.spinnaker.cats.redis.JedisSource; import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.reflect.FieldUtils; diff --git a/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/permissions/RedisPermissionsRepository.java b/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/permissions/RedisPermissionsRepository.java index c675396b9..195ea92c4 100644 --- a/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/permissions/RedisPermissionsRepository.java +++ b/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/permissions/RedisPermissionsRepository.java @@ -22,12 +22,12 @@ import com.google.common.collect.ArrayTable; import com.google.common.collect.HashBasedTable; import com.google.common.collect.Table; +import com.netflix.spinnaker.cats.redis.JedisSource; import com.netflix.spinnaker.fiat.config.UnrestrictedResourceConfig; import com.netflix.spinnaker.fiat.model.UserPermission; import com.netflix.spinnaker.fiat.model.resources.Resource; import com.netflix.spinnaker.fiat.model.resources.ResourceType; import com.netflix.spinnaker.fiat.model.resources.Role; -import com.netflix.spinnaker.fiat.redis.JedisSource; import lombok.NonNull; import lombok.Setter; import lombok.extern.slf4j.Slf4j; @@ -39,7 +39,6 @@ import redis.clients.jedis.Pipeline; import redis.clients.jedis.Response; -import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; diff --git a/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/providers/internal/ClouddriverService.java b/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/providers/internal/ClouddriverService.java index a3b5dda22..1033856a2 100644 --- a/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/providers/internal/ClouddriverService.java +++ b/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/providers/internal/ClouddriverService.java @@ -23,6 +23,7 @@ import com.netflix.spinnaker.fiat.providers.ProviderHealthTracker; import lombok.Getter; import lombok.extern.slf4j.Slf4j; +import org.springframework.beans.factory.InitializingBean; import org.springframework.beans.factory.annotation.Autowired; import java.util.List; @@ -34,7 +35,7 @@ * health tracker, which will turn unhealthy after X number of failed cache refreshes. */ @Slf4j -public class ClouddriverService implements HealthTrackable { +public class ClouddriverService implements HealthTrackable, InitializingBean { private static final String GROUP_KEY = "clouddriverService"; @@ -51,6 +52,16 @@ public ClouddriverService(ClouddriverApi clouddriverApi) { this.clouddriverApi = clouddriverApi; } + @Override + public void afterPropertiesSet() throws Exception { + try { + getAccounts(); + getApplications(); + } catch (Exception e) { + log.warn("Cache initialization failed: ", e); + } + } + public List getAccounts() { return new SimpleJava8HystrixCommand<>( GROUP_KEY, diff --git a/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/providers/internal/Front50Service.java b/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/providers/internal/Front50Service.java index 9d1b15df3..a1cb4bc56 100644 --- a/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/providers/internal/Front50Service.java +++ b/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/providers/internal/Front50Service.java @@ -23,13 +23,14 @@ import com.netflix.spinnaker.fiat.providers.ProviderHealthTracker; import lombok.Getter; import lombok.extern.slf4j.Slf4j; +import org.springframework.beans.factory.InitializingBean; import org.springframework.beans.factory.annotation.Autowired; import java.util.List; import java.util.concurrent.atomic.AtomicReference; @Slf4j -public class Front50Service implements HealthTrackable { +public class Front50Service implements HealthTrackable, InitializingBean { private static final String GROUP_KEY = "front50Service"; @@ -46,6 +47,17 @@ public Front50Service(Front50Api front50Api) { this.front50Api = front50Api; } + @Override + public void afterPropertiesSet() throws Exception { + try { + // Initialize caches (also indicates service is healthy) + getAllApplicationPermissions(); + getAllServiceAccounts(); + } catch (Exception e) { + log.warn("Cache prime failed: ", e); + } + } + public List getAllApplicationPermissions() { return new SimpleJava8HystrixCommand<>( GROUP_KEY, diff --git a/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/redis/JedisPoolSource.java b/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/redis/JedisPoolSource.java deleted file mode 100644 index e32bed492..000000000 --- a/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/redis/JedisPoolSource.java +++ /dev/null @@ -1,33 +0,0 @@ -/* - * Copyright 2014 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.fiat.redis; - -import redis.clients.jedis.Jedis; -import redis.clients.jedis.JedisPool; - -public class JedisPoolSource implements JedisSource { - private final JedisPool pool; - - public JedisPoolSource(JedisPool pool) { - this.pool = pool; - } - - @Override - public Jedis getJedis() { - return pool.getResource(); - } -} diff --git a/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/redis/JedisSource.java b/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/redis/JedisSource.java deleted file mode 100644 index dcc62bf08..000000000 --- a/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/redis/JedisSource.java +++ /dev/null @@ -1,23 +0,0 @@ -/* - * Copyright 2014 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.fiat.redis; - -import redis.clients.jedis.Jedis; - -public interface JedisSource { - Jedis getJedis(); -} diff --git a/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/roles/UserRolesSyncer.java b/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/roles/UserRolesSyncer.java index 17d9deaf0..a94e9f579 100644 --- a/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/roles/UserRolesSyncer.java +++ b/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/roles/UserRolesSyncer.java @@ -17,6 +17,7 @@ package com.netflix.spinnaker.fiat.roles; import com.diffplug.common.base.Functions; +import com.netflix.spinnaker.cats.agent.RunnableAgent; import com.netflix.spinnaker.fiat.config.ResourceProvidersHealthIndicator; import com.netflix.spinnaker.fiat.config.UnrestrictedResourceConfig; import com.netflix.spinnaker.fiat.model.UserPermission; @@ -28,6 +29,7 @@ import com.netflix.spinnaker.fiat.permissions.PermissionsResolver; import com.netflix.spinnaker.fiat.providers.ProviderException; import com.netflix.spinnaker.fiat.providers.ResourceProvider; +import lombok.Getter; import lombok.Setter; import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; @@ -47,7 +49,12 @@ @Slf4j @Component @ConditionalOnExpression("${fiat.writeMode.enabled:true}") -public class UserRolesSyncer { +public class UserRolesSyncer implements RunnableAgent { + + @Getter + private final String agentType = "UserRoleSyncer"; + @Getter + private final String providerName = "Fiat"; @Autowired @Setter @@ -69,8 +76,7 @@ public class UserRolesSyncer { @Setter private long retryIntervalMs; - @Scheduled(fixedDelayString = "${fiat.writeMode.syncDelayMs:600000}") - public void sync() { + public void run() { syncAndReturn(); } diff --git a/fiat-roles/src/test/groovy/com/netflix/spinnaker/fiat/permissions/RedisPermissionsRepositorySpec.groovy b/fiat-roles/src/test/groovy/com/netflix/spinnaker/fiat/permissions/RedisPermissionsRepositorySpec.groovy index cbe963e3d..9f11110ba 100644 --- a/fiat-roles/src/test/groovy/com/netflix/spinnaker/fiat/permissions/RedisPermissionsRepositorySpec.groovy +++ b/fiat-roles/src/test/groovy/com/netflix/spinnaker/fiat/permissions/RedisPermissionsRepositorySpec.groovy @@ -18,13 +18,13 @@ package com.netflix.spinnaker.fiat.permissions import com.fasterxml.jackson.annotation.JsonInclude import com.fasterxml.jackson.databind.ObjectMapper +import com.netflix.spinnaker.cats.redis.JedisSource import com.netflix.spinnaker.fiat.config.UnrestrictedResourceConfig import com.netflix.spinnaker.fiat.model.UserPermission import com.netflix.spinnaker.fiat.model.resources.Account import com.netflix.spinnaker.fiat.model.resources.Application import com.netflix.spinnaker.fiat.model.resources.Role import com.netflix.spinnaker.fiat.model.resources.ServiceAccount -import com.netflix.spinnaker.fiat.redis.JedisSource import com.netflix.spinnaker.kork.jedis.EmbeddedRedis import redis.clients.jedis.Jedis import spock.lang.AutoCleanup @@ -277,26 +277,26 @@ class RedisPermissionsRepositorySpec extends Specification { def result = repo.getAllByRoles(["role1"]) then: - result == ["user1": user1.merge(unrestricted), - "user2": user2.merge(unrestricted), + result == ["user1" : user1.merge(unrestricted), + "user2" : user2.merge(unrestricted), (UNRESTRICTED): unrestricted] when: result = repo.getAllByRoles(["role3", "role4"]) then: - result == ["user2": user2.merge(unrestricted), - "user4": user4.merge(unrestricted), + result == ["user2" : user2.merge(unrestricted), + "user4" : user4.merge(unrestricted), (UNRESTRICTED): unrestricted]; when: result = repo.getAllByRoles(null); then: - result == ["user1": user1.merge(unrestricted), - "user2": user2.merge(unrestricted), - "user3": user3.merge(unrestricted), - "user4": user4.merge(unrestricted), + result == ["user1" : user1.merge(unrestricted), + "user2" : user2.merge(unrestricted), + "user3" : user3.merge(unrestricted), + "user4" : user4.merge(unrestricted), (UNRESTRICTED): unrestricted] when: diff --git a/fiat-roles/src/test/groovy/com/netflix/spinnaker/fiat/roles/UserRolesSyncerSpec.groovy b/fiat-roles/src/test/groovy/com/netflix/spinnaker/fiat/roles/UserRolesSyncerSpec.groovy index 8686fca35..e34d3205d 100644 --- a/fiat-roles/src/test/groovy/com/netflix/spinnaker/fiat/roles/UserRolesSyncerSpec.groovy +++ b/fiat-roles/src/test/groovy/com/netflix/spinnaker/fiat/roles/UserRolesSyncerSpec.groovy @@ -18,6 +18,7 @@ package com.netflix.spinnaker.fiat.roles import com.fasterxml.jackson.annotation.JsonInclude import com.fasterxml.jackson.databind.ObjectMapper +import com.netflix.spinnaker.cats.redis.JedisSource import com.netflix.spinnaker.fiat.config.ResourceProvidersHealthIndicator import com.netflix.spinnaker.fiat.config.UnrestrictedResourceConfig import com.netflix.spinnaker.fiat.model.UserPermission @@ -27,7 +28,6 @@ import com.netflix.spinnaker.fiat.model.resources.ServiceAccount import com.netflix.spinnaker.fiat.permissions.PermissionsResolver import com.netflix.spinnaker.fiat.permissions.RedisPermissionsRepository import com.netflix.spinnaker.fiat.providers.ResourceProvider -import com.netflix.spinnaker.fiat.redis.JedisSource import com.netflix.spinnaker.kork.jedis.EmbeddedRedis import org.springframework.boot.actuate.health.Health import redis.clients.jedis.Jedis diff --git a/fiat-web/src/main/java/com/netflix/spinnaker/fiat/config/ResourcesConfig.java b/fiat-web/src/main/java/com/netflix/spinnaker/fiat/config/ResourcesConfig.java index 8e9826d44..51375c7c0 100644 --- a/fiat-web/src/main/java/com/netflix/spinnaker/fiat/config/ResourcesConfig.java +++ b/fiat-web/src/main/java/com/netflix/spinnaker/fiat/config/ResourcesConfig.java @@ -17,11 +17,13 @@ package com.netflix.spinnaker.fiat.config; import com.fasterxml.jackson.databind.ObjectMapper; +import com.netflix.spinnaker.config.HystrixSpectatorConfig; import com.netflix.spinnaker.fiat.providers.ProviderHealthTracker; import com.netflix.spinnaker.fiat.providers.internal.ClouddriverApi; import com.netflix.spinnaker.fiat.providers.internal.ClouddriverService; import com.netflix.spinnaker.fiat.providers.internal.Front50Api; import com.netflix.spinnaker.fiat.providers.internal.Front50Service; +import com.netflix.spinnaker.hystrix.spectator.HystrixSpectatorPublisher; import lombok.Setter; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -31,6 +33,8 @@ import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.DependsOn; +import org.springframework.context.annotation.Import; import org.springframework.context.annotation.Scope; import retrofit.Endpoints; import retrofit.RestAdapter; @@ -39,6 +43,7 @@ @Configuration @EnableConfigurationProperties(ProviderCacheConfig.class) +@DependsOn("hystrixSpectatorPublisher") public class ResourcesConfig { @Autowired @Setter diff --git a/fiat-web/src/test/groovy/com/netflix/spinnaker/config/EmbeddedRedisConfig.groovy b/fiat-web/src/test/groovy/com/netflix/spinnaker/config/EmbeddedRedisConfig.groovy index 127c4547e..bac8876eb 100644 --- a/fiat-web/src/test/groovy/com/netflix/spinnaker/config/EmbeddedRedisConfig.groovy +++ b/fiat-web/src/test/groovy/com/netflix/spinnaker/config/EmbeddedRedisConfig.groovy @@ -17,7 +17,7 @@ package com.netflix.spinnaker.config -import com.netflix.spinnaker.fiat.redis.JedisSource +import com.netflix.spinnaker.cats.redis.JedisSource import com.netflix.spinnaker.kork.jedis.EmbeddedRedis import org.springframework.beans.factory.config.ConfigurableBeanFactory import org.springframework.context.annotation.Bean