-
Notifications
You must be signed in to change notification settings - Fork 147
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Delay promotion if verifications are still running (#1834)
* feat(verifications): query to find pending verifications for an environment * feat(verifications): implicit contraint to delay promotion until any pending verifications complete * feat(verifications): check for running verifications for previous versions before launching * feat(verifications): make sure we update the state of any prior version verifications * chore(verifications): clarify some things * fix(verifications): fix a stupid logic bug
- Loading branch information
1 parent
65151e4
commit a2db125
Showing
8 changed files
with
367 additions
and
8 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
54 changes: 54 additions & 0 deletions
54
.../kotlin/com/netflix/spinnaker/keel/constraints/PendingVerificationsConstraintEvaluator.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| package com.netflix.spinnaker.keel.constraints | ||
|
|
||
| import com.netflix.spinnaker.keel.api.DeliveryConfig | ||
| import com.netflix.spinnaker.keel.api.Environment | ||
| import com.netflix.spinnaker.keel.api.artifacts.DeliveryArtifact | ||
| import com.netflix.spinnaker.keel.api.constraints.SupportedConstraintType | ||
| import com.netflix.spinnaker.keel.api.plugins.ConstraintEvaluator | ||
| import com.netflix.spinnaker.keel.api.support.EventPublisher | ||
| import com.netflix.spinnaker.keel.api.verification.VerificationRepository | ||
| import com.netflix.spinnaker.keel.core.api.PendingVerificationsConstraint | ||
| import org.slf4j.LoggerFactory | ||
|
|
||
| class PendingVerificationsConstraintEvaluator( | ||
| private val verificationRepository: VerificationRepository, | ||
| override val eventPublisher: EventPublisher | ||
| ) : ConstraintEvaluator<PendingVerificationsConstraint> { | ||
|
|
||
| override fun isImplicit() = true | ||
|
|
||
| override val supportedType = | ||
| SupportedConstraintType<PendingVerificationsConstraint>("pending-verifications") | ||
|
|
||
| override fun canPromote( | ||
| artifact: DeliveryArtifact, | ||
| version: String, | ||
| deliveryConfig: DeliveryConfig, | ||
| targetEnvironment: Environment | ||
| ): Boolean = | ||
| if (targetEnvironment.verifyWith.isEmpty()) { | ||
| log.debug("{} / {} has no verifications", deliveryConfig.name, targetEnvironment.name) | ||
| true | ||
| } else { | ||
| val pendingVerifications = | ||
| verificationRepository.pendingInEnvironment(deliveryConfig, targetEnvironment.name) | ||
| if (pendingVerifications.isNotEmpty()) { | ||
| log.info( | ||
| "{} / {} is awaiting results from {} before another deployment can proceed", | ||
| deliveryConfig.name, | ||
| targetEnvironment.name, | ||
| pendingVerifications.map { it.verification.id }.joinToString() | ||
| ) | ||
| false | ||
| } else { | ||
| log.debug( | ||
| "{} / {} has no pending verifications", | ||
| deliveryConfig.name, | ||
| targetEnvironment.name | ||
| ) | ||
| true | ||
| } | ||
| } | ||
|
|
||
| private val log by lazy { LoggerFactory.getLogger(javaClass) } | ||
| } |
5 changes: 5 additions & 0 deletions
5
...ore/src/main/kotlin/com/netflix/spinnaker/keel/core/api/PendingVerificationsConstraint.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| package com.netflix.spinnaker.keel.core.api | ||
|
|
||
| import com.netflix.spinnaker.keel.api.Constraint | ||
|
|
||
| class PendingVerificationsConstraint : Constraint("pending-verifications") |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
93 changes: 93 additions & 0 deletions
93
...in/com/netflix/spinnaker/keel/constraints/PendingVerificationsConstraintEvaluatorTests.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,93 @@ | ||
| package com.netflix.spinnaker.keel.constraints | ||
|
|
||
| import com.netflix.spinnaker.keel.api.DeliveryConfig | ||
| import com.netflix.spinnaker.keel.api.Environment | ||
| import com.netflix.spinnaker.keel.api.Verification | ||
| import com.netflix.spinnaker.keel.api.constraints.ConstraintStatus.PENDING | ||
| import com.netflix.spinnaker.keel.api.support.EventPublisher | ||
| import com.netflix.spinnaker.keel.api.verification.PendingVerification | ||
| import com.netflix.spinnaker.keel.api.verification.VerificationContext | ||
| import com.netflix.spinnaker.keel.api.verification.VerificationRepository | ||
| import com.netflix.spinnaker.keel.api.verification.VerificationState | ||
| import com.netflix.spinnaker.keel.artifacts.DockerArtifact | ||
| import io.mockk.every | ||
| import io.mockk.mockk | ||
| import org.junit.jupiter.api.Test | ||
| import strikt.api.expectCatching | ||
| import strikt.assertions.isFalse | ||
| import strikt.assertions.isSuccess | ||
| import strikt.assertions.isTrue | ||
| import java.time.Instant.now | ||
|
|
||
| internal class PendingVerificationsConstraintEvaluatorTests { | ||
|
|
||
| private val artifact = DockerArtifact( | ||
| name = "fnord", | ||
| reference = "fnord" | ||
| ) | ||
|
|
||
| private val environmentWithNoVerifications = Environment( | ||
| name = "test" | ||
| ) | ||
| private val verification = DummyVerification("1") | ||
| private val environmentWithVerification = | ||
| environmentWithNoVerifications.copy(verifyWith = listOf(verification)) | ||
|
|
||
| private val deliveryConfig = DeliveryConfig( | ||
| application = "fnord", | ||
| name = "fnord-manifest", | ||
| serviceAccount = "[email protected]", | ||
| artifacts = setOf(artifact), | ||
| environments = setOf(environmentWithNoVerifications) | ||
| ) | ||
|
|
||
| private val verificationRepository = mockk<VerificationRepository>() | ||
| private val eventPublisher = mockk<EventPublisher>() | ||
|
|
||
| private val subject = PendingVerificationsConstraintEvaluator(verificationRepository, eventPublisher) | ||
|
|
||
| @Test | ||
| fun `can promote if the environment has no verifications`() { | ||
| expectCatching { | ||
| subject.canPromote(artifact, "v1", deliveryConfig, environmentWithNoVerifications) | ||
| } | ||
| .isSuccess() | ||
| .isTrue() | ||
| } | ||
|
|
||
| @Test | ||
| fun `can promote if there are no verifications currently running`() { | ||
| with(deliveryConfig.copy(environments = setOf(environmentWithVerification))) { | ||
| every { verificationRepository.pendingInEnvironment(any(), any()) } returns emptyList() | ||
|
|
||
| expectCatching { | ||
| subject.canPromote(artifact, "v1", this, environmentWithVerification) | ||
| } | ||
| .isSuccess() | ||
| .isTrue() | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| fun `cannot promote if there are any verifications currently running`() { | ||
| with(deliveryConfig.copy(environments = setOf(environmentWithVerification))) { | ||
| every { verificationRepository.pendingInEnvironment(any(), any()) } returns listOf( | ||
| PendingVerification( | ||
| VerificationContext(this, environmentWithVerification.name, artifact.reference, "v1"), | ||
| verification, | ||
| VerificationState(PENDING, now(), null) | ||
| ) | ||
| ) | ||
|
|
||
| expectCatching { | ||
| subject.canPromote(artifact, "v1", this, environmentWithVerification) | ||
| } | ||
| .isSuccess() | ||
| .isFalse() | ||
| } | ||
| } | ||
|
|
||
| private data class DummyVerification(override val id: String) : Verification { | ||
| override val type: String = "dummy" | ||
| } | ||
| } |
Oops, something went wrong.