Skip to content

Commit

Permalink
fix(gcp): Relaxed health check for GCP accounts (#6200) (#6202)
Browse files Browse the repository at this point in the history
(cherry picked from commit 28599eb)

Co-authored-by: Christos Arvanitis <[email protected]>
  • Loading branch information
mergify[bot] and christosarvanitis committed May 7, 2024
1 parent ab0d436 commit 20fd2c8
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package com.netflix.spinnaker.clouddriver.google.health

import com.netflix.spectator.api.Registry
import com.netflix.spinnaker.clouddriver.google.GoogleExecutorTraits
import com.netflix.spinnaker.clouddriver.google.config.GoogleConfigurationProperties
import com.netflix.spinnaker.clouddriver.google.security.GoogleCredentials
import com.netflix.spinnaker.clouddriver.google.security.GoogleNamedAccountCredentials
import com.netflix.spinnaker.clouddriver.security.AccountCredentialsProvider
Expand Down Expand Up @@ -49,6 +50,9 @@ class GoogleHealthIndicator implements HealthIndicator, GoogleExecutorTraits {

private final AtomicReference<Exception> lastException = new AtomicReference<>(null)

@Autowired
GoogleConfigurationProperties googleConfigurationProperties

@Override
Health health() {
def ex = lastException.get()
Expand All @@ -62,7 +66,9 @@ class GoogleHealthIndicator implements HealthIndicator, GoogleExecutorTraits {

@Scheduled(fixedDelay = 300000L)
void checkHealth() {
try {
try {
if (googleConfigurationProperties.getHealth().getVerifyAccountHealth()) {
LOG.info("google.health.verifyAccountHealth flag is enabled - verifying connection to the Google accounts")
credentialsTypeBaseConfiguration.credentialsRepository?.all?.forEach({
try {
timeExecute(it.compute.projects().get(it.project),
Expand All @@ -72,13 +78,16 @@ class GoogleHealthIndicator implements HealthIndicator, GoogleExecutorTraits {
throw new GoogleIOException(e)
}
})
lastException.set(null)
} catch (Exception ex) {
LOG.warn "Unhealthy", ex

lastException.set(ex)
} else {
LOG.info("google.health.verifyAccountHealth flag is disabled - Not verifying connection to the Google accounts");
}
lastException.set(null)
} catch (Exception ex) {
LOG.warn "Unhealthy", ex

lastException.set(ex)
}
}

@ResponseStatus(value = HttpStatus.SERVICE_UNAVAILABLE, reason = "Problem communicating with Google.")
@InheritConstructors
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,6 @@ class GoogleConfiguration {
new GoogleConfigurationProperties()
}

@Bean
@ConditionalOnProperty("google.health.verifyAccountHealth")
GoogleHealthIndicator googleHealthIndicator() {
new GoogleHealthIndicator()
}

@Bean
GoogleOperationPoller googleOperationPoller() {
new GoogleOperationPoller()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import com.google.common.collect.ImmutableList
import com.google.common.collect.ImmutableMap
import com.netflix.spectator.api.NoopRegistry
import com.netflix.spectator.api.Registry
import com.netflix.spinnaker.clouddriver.google.config.GoogleConfigurationProperties
import com.netflix.spinnaker.clouddriver.google.provider.agent.StubComputeFactory
import com.netflix.spinnaker.clouddriver.google.security.GoogleNamedAccountCredentials
import com.netflix.spinnaker.credentials.CredentialsRepository
Expand Down Expand Up @@ -66,7 +67,7 @@ class GoogleHealthIndicatorSpec extends Specification {
def credentialsTypeBaseConfiguration = new CredentialsTypeBaseConfiguration(applicationContext, null)
credentialsTypeBaseConfiguration.credentialsRepository = credentialsRepository

def indicator = new GoogleHealthIndicator()
def indicator = new GoogleHealthIndicator(googleConfigurationProperties: new GoogleConfigurationProperties())
indicator.registry = REGISTRY
indicator.credentialsTypeBaseConfiguration = credentialsTypeBaseConfiguration

Expand Down Expand Up @@ -107,7 +108,7 @@ class GoogleHealthIndicatorSpec extends Specification {
def credentialsTypeBaseConfiguration = new CredentialsTypeBaseConfiguration(applicationContext, null)
credentialsTypeBaseConfiguration.credentialsRepository = credentialsRepository

def indicator = new GoogleHealthIndicator()
def indicator = new GoogleHealthIndicator(googleConfigurationProperties: new GoogleConfigurationProperties())
indicator.registry = REGISTRY
indicator.credentialsTypeBaseConfiguration = credentialsTypeBaseConfiguration

Expand All @@ -120,4 +121,47 @@ class GoogleHealthIndicatorSpec extends Specification {

health == null
}

@Unroll
def "health succeeds when google is unreachable and verifyAccountHealth is false"() {
setup:
def applicationContext = Mock(ApplicationContext)
def project = new Project()
project.setName(PROJECT)

def compute = new StubComputeFactory()
.setProjects(project)
.setProjectException(new IOException("Read timed out"))
.create()

def googleNamedAccountCredentials =
new GoogleNamedAccountCredentials.Builder()
.project(PROJECT)
.name(ACCOUNT_NAME)
.compute(compute)
.regionToZonesMap(ImmutableMap.of(REGION, ImmutableList.of(ZONE)))
.build()

def credentials = [googleNamedAccountCredentials]
def credentialsRepository = Stub(CredentialsRepository) {
getAll() >> credentials
}

def credentialsTypeBaseConfiguration = new CredentialsTypeBaseConfiguration(applicationContext, null)
credentialsTypeBaseConfiguration.credentialsRepository = credentialsRepository

def indicator = new GoogleHealthIndicator(googleConfigurationProperties: new GoogleConfigurationProperties())
indicator.googleConfigurationProperties.health.setVerifyAccountHealth(false)
indicator.registry = REGISTRY
indicator.credentialsTypeBaseConfiguration = credentialsTypeBaseConfiguration


when:
indicator.checkHealth()
def health = indicator.health()

then:
health.status == Status.UP
health.details.isEmpty()
}
}

0 comments on commit 20fd2c8

Please sign in to comment.