From bc22d258be9ee3fd098a0cfa1507849f717c4428 Mon Sep 17 00:00:00 2001 From: Adam Jordens Date: Wed, 22 May 2019 17:20:43 -0700 Subject: [PATCH] feat(sql): Support for finer grained toggling of where reads should be sent This only comes into play when the `CompositeStorageService` is active. Also enabling the actuator endpoints. --- front50-sql/front50-sql.gradle | 8 +- .../CompositeStorageServiceConfiguration.kt | 8 +- .../front50/model/CompositeStorageService.kt | 122 ++++++++++++------ .../model/CompositeStorageServiceTests.kt | 56 ++++++++ 4 files changed, 151 insertions(+), 43 deletions(-) create mode 100644 front50-sql/src/test/kotlin/com/netflix/spinnaker/front50/model/CompositeStorageServiceTests.kt diff --git a/front50-sql/front50-sql.gradle b/front50-sql/front50-sql.gradle index 6ac301ca4..904cfaeeb 100644 --- a/front50-sql/front50-sql.gradle +++ b/front50-sql/front50-sql.gradle @@ -11,13 +11,13 @@ dependencies { implementation "com.netflix.hystrix:hystrix-core" implementation "io.github.resilience4j:resilience4j-retry" - implementation "org.jetbrains.kotlinx:kotlinx-coroutines-core:1.2.1" - implementation "org.jetbrains.kotlinx:kotlinx-coroutines-slf4j:1.2.1" - implementation "org.jetbrains.kotlinx:kotlinx-coroutines-core-common:1.2.1" + implementation "org.jetbrains.kotlinx:kotlinx-coroutines-core" implementation "org.mariadb.jdbc:mariadb-java-client:2.2.3" -// runtimeOnly "mysql:mysql-connector-java" + // runtimeOnly "mysql:mysql-connector-java" + testImplementation "io.mockk:mockk" + testImplementation "com.ninja-squad:springmockk:1.1.0" testImplementation "mysql:mysql-connector-java" testImplementation "dev.minutest:minutest" testImplementation "org.testcontainers:mysql" diff --git a/front50-sql/src/main/kotlin/com/netflix/spinnaker/config/CompositeStorageServiceConfiguration.kt b/front50-sql/src/main/kotlin/com/netflix/spinnaker/config/CompositeStorageServiceConfiguration.kt index 16dbb24a9..8bec9f1cc 100644 --- a/front50-sql/src/main/kotlin/com/netflix/spinnaker/config/CompositeStorageServiceConfiguration.kt +++ b/front50-sql/src/main/kotlin/com/netflix/spinnaker/config/CompositeStorageServiceConfiguration.kt @@ -20,6 +20,7 @@ import com.netflix.spectator.api.Registry import com.netflix.spinnaker.front50.migrations.StorageServiceMigrator import com.netflix.spinnaker.front50.model.CompositeStorageService import com.netflix.spinnaker.front50.model.StorageService +import com.netflix.spinnaker.kork.dynamicconfig.DynamicConfigService import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty import org.springframework.boot.context.properties.ConfigurationProperties import org.springframework.boot.context.properties.EnableConfigurationProperties @@ -33,12 +34,13 @@ class CompositeStorageServiceConfiguration() { @Bean @Primary @ConditionalOnProperty("spinnaker.migration.compositeStorageService.enabled") - fun compositeStorageService(properties: StorageServiceMigratorConfigurationProperties, + fun compositeStorageService(dynamicConfigService: DynamicConfigService, + properties: StorageServiceMigratorConfigurationProperties, storageServices: List) = CompositeStorageService( + dynamicConfigService, storageServices.first { it.javaClass.canonicalName.equals(properties.primaryClass) }, - storageServices.first { it.javaClass.canonicalName.equals(properties.previousClass) }, - properties.writeOnly + storageServices.first { it.javaClass.canonicalName.equals(properties.previousClass) } ) @Bean diff --git a/front50-sql/src/main/kotlin/com/netflix/spinnaker/front50/model/CompositeStorageService.kt b/front50-sql/src/main/kotlin/com/netflix/spinnaker/front50/model/CompositeStorageService.kt index d6b1e62a4..4593054b9 100644 --- a/front50-sql/src/main/kotlin/com/netflix/spinnaker/front50/model/CompositeStorageService.kt +++ b/front50-sql/src/main/kotlin/com/netflix/spinnaker/front50/model/CompositeStorageService.kt @@ -17,18 +17,31 @@ package com.netflix.spinnaker.front50.model import com.netflix.spinnaker.front50.exception.NotFoundException +import com.netflix.spinnaker.kork.dynamicconfig.DynamicConfigService import org.slf4j.LoggerFactory +import org.springframework.scheduling.annotation.Scheduled class CompositeStorageService( + private val dynamicConfigService: DynamicConfigService, private val primary: StorageService, - private val previous: StorageService, - private val writeOnly: Boolean + private val previous: StorageService ) : StorageService { companion object { private val log = LoggerFactory.getLogger(CompositeStorageService::class.java) } + @Scheduled(fixedDelay = 60000L) + fun status() { + log.debug( + "Composite Storage Service Read Status ({}: {}, {}: {})", + primary.javaClass.simpleName, + isPrimaryReadEnabled(), + previous.javaClass.simpleName, + isPreviousReadEnabled() + ) + } + override fun ensureBucketExists() { primary.ensureBucketExists() previous.ensureBucketExists() @@ -39,18 +52,31 @@ class CompositeStorageService( } override fun loadObject(objectType: ObjectType?, objectKey: String?): T { - if (writeOnly || objectType == ObjectType.ENTITY_TAGS) { + if (objectType == ObjectType.ENTITY_TAGS) { + // entity tags are currently unsupported in SQL return previous.loadObject(objectType, objectKey) } - try { - return primary.loadObject(objectType, objectKey) - } catch (e: NotFoundException) { - log.debug("{}.loadObject({}, {}) not found (primary)", primary.javaClass.simpleName, objectType, objectKey) - return previous.loadObject(objectType, objectKey) - } catch (e: Exception) { - log.error("{}.loadObject({}, {}) failed (primary)", primary.javaClass.simpleName, objectType, objectKey) - return previous.loadObject(objectType, objectKey) + var exception: Exception? = null + + if (isPrimaryReadEnabled()) { + try { + return primary.loadObject(objectType, objectKey) + } catch (e: NotFoundException) { + log.debug("{}.loadObject({}, {}) not found (primary)", primary.javaClass.simpleName, objectType, objectKey) + + exception = e + } catch (e: Exception) { + log.error("{}.loadObject({}, {}) failed (primary)", primary.javaClass.simpleName, objectType, objectKey) + + exception = e + } + } + + return when { + isPreviousReadEnabled() -> previous.loadObject(objectType, objectKey) + exception != null -> throw exception + else -> throw IllegalStateException("Primary and previous storage services are disabled") } } @@ -94,47 +120,71 @@ class CompositeStorageService( } override fun listObjectKeys(objectType: ObjectType?): Map { - if (writeOnly) { - return previous.listObjectKeys(objectType) + val objectKeys = mutableMapOf() + + if (isPreviousReadEnabled()) { + objectKeys.putAll(previous.listObjectKeys(objectType)) } - val primaryObjectKeys = primary.listObjectKeys(objectType) - val previousObjectKeys = previous.listObjectKeys(objectType) + if (isPrimaryReadEnabled()) { + objectKeys.putAll(primary.listObjectKeys(objectType)) + } - return previousObjectKeys + primaryObjectKeys + return objectKeys } override fun listObjectVersions(objectType: ObjectType?, objectKey: String?, maxResults: Int): MutableCollection { - if (writeOnly) { - return previous.listObjectVersions(objectType, objectKey, maxResults) + var exception: Exception? = null + + if (isPrimaryReadEnabled()) { + try { + return primary.listObjectVersions(objectType, objectKey, maxResults) + } catch (e: Exception) { + log.error( + "{}.listObjectVersions({}, {}, {}) failed (primary)", + primary.javaClass.simpleName, + objectType, + objectKey, + maxResults + ) + + exception = e + } } - try { - return primary.listObjectVersions(objectType, objectKey, maxResults) - } catch (e: Exception) { - log.error( - "{}.listObjectVersions({}, {}, {}) failed (primary)", - primary.javaClass.simpleName, - objectType, - objectKey, - maxResults - ) - return previous.listObjectVersions(objectType, objectKey, maxResults) + return when { + isPreviousReadEnabled() -> return previous.listObjectVersions(objectType, objectKey, maxResults) + exception != null -> throw exception + else -> mutableListOf() } } override fun getLastModified(objectType: ObjectType?): Long { - if (writeOnly) { - return previous.getLastModified(objectType) + var exception: Exception? = null + + if (isPrimaryReadEnabled()) { + try { + return primary.getLastModified(objectType) + } catch (e: Exception) { + log.error("{}.getLastModified({}) failed (primary)", primary.javaClass.simpleName, objectType) + + exception = e + } } - try { - return primary.getLastModified(objectType) - } catch (e: Exception) { - log.error("{}.getLastModified({}) failed (primary)", primary.javaClass.simpleName, objectType) - return previous.getLastModified(objectType) + return when { + isPreviousReadEnabled() -> previous.getLastModified(objectType) + exception != null -> throw exception + else -> throw IllegalStateException("Primary and previous storage services are disabled") } } + + private fun isPrimaryReadEnabled() = isReadEnabled("primary") + + private fun isPreviousReadEnabled() = isReadEnabled("previous") + + private fun isReadEnabled(type: String) = + dynamicConfigService.getConfig(Boolean::class.java, "spinnaker.migration.compositeStorageService.reads.$type", false) } diff --git a/front50-sql/src/test/kotlin/com/netflix/spinnaker/front50/model/CompositeStorageServiceTests.kt b/front50-sql/src/test/kotlin/com/netflix/spinnaker/front50/model/CompositeStorageServiceTests.kt new file mode 100644 index 000000000..325c038e5 --- /dev/null +++ b/front50-sql/src/test/kotlin/com/netflix/spinnaker/front50/model/CompositeStorageServiceTests.kt @@ -0,0 +1,56 @@ +/* + * 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.front50.model + +import com.netflix.spinnaker.front50.model.tag.EntityTags +import com.netflix.spinnaker.kork.dynamicconfig.DynamicConfigService +import dev.minutest.junit.JUnit5Minutests +import dev.minutest.rootContext +import io.mockk.* +import strikt.api.expectThat +import strikt.assertions.isEqualTo + +internal object CompositeStorageServiceTests : JUnit5Minutests { + val dynamicConfigService: DynamicConfigService = mockk(relaxUnitFun = true) + val primary: StorageService = mockk(relaxUnitFun = true) + val previous: StorageService = mockk(relaxUnitFun = true) + + val subject = CompositeStorageService(dynamicConfigService, primary, previous) + + fun tests() = rootContext { + after { + clearMocks(dynamicConfigService, primary, previous) + } + + context("Entity Tags") { + test("should always load from 'previous'") { + every { + previous.loadObject(ObjectType.ENTITY_TAGS, "id-entitytags001") + } returns EntityTags().apply { id = "id-entitytags001" } + + expectThat( + subject.loadObject(ObjectType.ENTITY_TAGS, "id-entitytags001").id + ).isEqualTo("id-entitytags001") + + verifyAll { + primary wasNot Called + previous.loadObject(ObjectType.ENTITY_TAGS, "id-entitytags001") + } + } + } + } +}