Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: configurable choice of annotations for rolling update revision #1303

Merged
merged 8 commits into from
Aug 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,12 @@ target/
*.class
*.log

.metals
.vscode

# attachments created by scripts/prepare-downloads.sh
docs/src/main/paradox/attachments

# Metals detritus
.metals
metals.sbt
**/project/project
2 changes: 1 addition & 1 deletion docs/src/main/paradox/rolling-updates.md
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ Java

#### Configuration

The following configuration is required, more details for each and additional configurations can be found in [reference.conf](https://github.com/akka/akka-management/blob/main/rolling-updates-kubernetes/src/main/resources/reference.conf):
The following configuration is required, more details for each and additional configurations can be found in [reference.conf](https://github.com/akka/akka-management/blob/main/rolling-update-kubernetes/src/main/resources/reference.conf):
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how long the typo has been there...

I think the comment in reference.conf is sufficient documentation for a niche feature.


* `akka.rollingupdate.kubernetes.pod-name`: this can be provided by setting `KUBERNETES_POD_NAME` environment variable to `metadata.name` on the Kubernetes container spec.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ class KubernetesApiIntegrationTest
enabled = true,
crName = None,
cleanupAfter = 60.seconds
)
),
revisionAnnotation = "deployment.kubernetes.io/revision"
)

private val underTest =
Expand Down
4 changes: 4 additions & 0 deletions rolling-update-kubernetes/src/main/resources/reference.conf
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ akka.rollingupdate.kubernetes {
pod-name = ""
pod-name = ${?KUBERNETES_POD_NAME}

# Annotations to check to determine the revision. The default is suitable for "vanilla"
# Kubernetes Deployments, but other CI/CD systems may set a different annotation.
revision-annotation = "deployment.kubernetes.io/revision"

secure-api-server = true

# Configuration for the Pod Deletion Cost extension
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ import scala.util.control.NonFatal

import system.dispatcher

override val revisionAnnotation = settings.revisionAnnotation

private implicit val sys: ActorSystem = system
private val log = Logging(system, classOf[KubernetesApiImpl])
private val http = Http()(system)
Expand Down Expand Up @@ -93,6 +95,7 @@ import scala.util.control.NonFatal
case HttpResponse(status, _, e, _) =>
e.discardBytes()
throw new PodCostException(s"Request failed with status=$status")
// Can we make this exhaustive?
}
}

Expand Down Expand Up @@ -213,7 +216,7 @@ PUTs must contain resourceVersions. Response:
case None => Future.failed(new ReadRevisionException("No replica name found"))
}

val revision = replicaSet.map(_.metadata.annotations.`deployment.kubernetes.io/revision`)
val revision = replicaSet.map(_.metadata.annotations.revision)
revision.map(Some(_)).recoverWith {
case ex =>
if (tries >= maxTries) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,16 @@ import akka.annotation.InternalApi
import akka.http.scaladsl.marshallers.sprayjson.SprayJsonSupport
import spray.json.DefaultJsonProtocol
import spray.json.JsonFormat
import spray.json.JsObject
import spray.json.RootJsonFormat
import spray.json.JsString
import spray.json.JsValue

/**
* INTERNAL API
*/
@InternalApi
case class ReplicaAnnotation(`deployment.kubernetes.io/revision`: String)
case class ReplicaAnnotation(revision: String, source: String, otherAnnotations: Map[String, JsValue])

/**
* INTERNAL API
Expand Down Expand Up @@ -75,6 +78,8 @@ case class Spec(pods: immutable.Seq[PodCost])
*/
@InternalApi
trait KubernetesJsonSupport extends SprayJsonSupport with DefaultJsonProtocol {
val revisionAnnotation: String

// If adding more formats here, remember to also add in META-INF/native-image reflect config
implicit val metadataFormat: JsonFormat[Metadata] = jsonFormat2(Metadata.apply)
implicit val podCostFormat: JsonFormat[PodCost] = jsonFormat5(PodCost.apply)
Expand All @@ -86,7 +91,30 @@ trait KubernetesJsonSupport extends SprayJsonSupport with DefaultJsonProtocol {
implicit val podMetadataFormat: JsonFormat[PodMetadata] = jsonFormat1(PodMetadata.apply)
implicit val podFormat: RootJsonFormat[Pod] = jsonFormat1(Pod.apply)

implicit val replicaAnnotationFormat: JsonFormat[ReplicaAnnotation] = jsonFormat1(ReplicaAnnotation.apply)
implicit val replicaAnnotationFormat: RootJsonFormat[ReplicaAnnotation] =
new RootJsonFormat[ReplicaAnnotation] {
// Not sure if we ever write this out, but if we do, this will let us write out exactly what we took in
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we add the "writes" part only then if really need?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then we can even further simplify the case class ReplicaAnnotation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a JsonFormat so needs a writes. If we're going to implement it, we might as well implement it right

def write(ra: ReplicaAnnotation): JsValue =
if (ra.revision.nonEmpty && ra.source.nonEmpty) {
JsObject(ra.otherAnnotations + (ra.source -> JsString(ra.revision)))
} else JsObject(ra.otherAnnotations)

def read(json: JsValue): ReplicaAnnotation = {
json match {
case JsObject(fields) =>
fields.get(revisionAnnotation) match {
case Some(JsString(foundRevision)) =>
ReplicaAnnotation(foundRevision, revisionAnnotation, fields - revisionAnnotation)

case _ =>
ReplicaAnnotation("", "", fields)
}

case _ => spray.json.deserializationError("expected an object")
}
}
}

implicit val replicaSetMedatataFormat: JsonFormat[ReplicaSetMetadata] = jsonFormat1(ReplicaSetMetadata.apply)
implicit val podReplicaSetFormat: RootJsonFormat[ReplicaSet] = jsonFormat1(ReplicaSet.apply)
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@

package akka.rollingupdate.kubernetes

import scala.concurrent.duration._
import akka.util.JavaDurationConverters._
import akka.annotation.InternalApi
import com.typesafe.config.Config

import scala.concurrent.duration._
import scala.jdk.CollectionConverters.ListHasAsScala

/**
* INTERNAL API
*/
Expand Down Expand Up @@ -47,7 +49,8 @@ private[kubernetes] object KubernetesSettings {
config.getString("pod-name"),
config.getBoolean("secure-api-server"),
config.getDuration("api-service-request-timeout").asScala,
customResourceSettings
customResourceSettings,
config.getString("revision-annotation")
)
}
}
Expand All @@ -67,6 +70,7 @@ private[kubernetes] class KubernetesSettings(
val secure: Boolean,
val apiServiceRequestTimeout: FiniteDuration,
val customResourceSettings: CustomResourceSettings,
val revisionAnnotation: String,
val bodyReadTimeout: FiniteDuration = 1.second
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ class KubernetesApiSpec
podName = podName,
secure = false,
apiServiceRequestTimeout = 2.seconds,
customResourceSettings = new CustomResourceSettings(enabled = false, crName = None, 60.seconds)
customResourceSettings = new CustomResourceSettings(enabled = false, crName = None, 60.seconds),
revisionAnnotation = "deployment.kubernetes.io/revision"
)
}

Expand Down Expand Up @@ -167,6 +168,42 @@ class KubernetesApiSpec
}
}

"parse pod and replica responses to get the revision from custom annotations" in {
stubPodResponse()
stubReplicaResponse(defaultReplicaResponseJson.replaceAllLiterally("deployment.kubernetes.io", "custom.akka.io"))

val customSettings = {
val base = settings(podName1)
new KubernetesSettings(
apiCaPath = base.apiCaPath,
apiTokenPath = base.apiTokenPath,
apiServiceHost = base.apiServiceHost,
apiServicePort = base.apiServicePort,
namespace = base.namespace,
namespacePath = base.namespacePath,
podName = base.podName,
secure = base.secure,
apiServiceRequestTimeout = base.apiServiceRequestTimeout,
customResourceSettings = base.customResourceSettings,
revisionAnnotation = "custom.akka.io/revision"
)
}

val customKubernetesApi =
new KubernetesApiImpl(
system,
customSettings,
namespace,
apiToken = "apiToken",
clientHttpsConnectionContext = None)

EventFilter
.info(pattern = "Reading revision from Kubernetes: akka.cluster.app-version was set to 1", occurrences = 1)
.intercept {
customKubernetesApi.readRevision().futureValue should be("1")
}
}

"retry and then fail when pod not found" in {
stubFor(getPod(podName1).willReturn(aResponse().withStatus(404)))
EventFilter
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,8 @@ class PodDeletionCostAnnotatorCrSpec
podName = podName,
secure = false,
apiServiceRequestTimeout = 2.seconds,
customResourceSettings = new CustomResourceSettings(enabled = false, crName = None, 60.seconds)
customResourceSettings = new CustomResourceSettings(enabled = false, crName = None, 60.seconds),
revisionAnnotation = "deployment.kubernetes.io/revision"
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,8 @@ class PodDeletionCostAnnotatorSpec
podName = podName,
secure = false,
apiServiceRequestTimeout = 2.seconds,
customResourceSettings = new CustomResourceSettings(enabled = false, crName = None, 60.seconds)
customResourceSettings = new CustomResourceSettings(enabled = false, crName = None, 60.seconds),
revisionAnnotation = "deployment.kubernetes.io/revision"
)
}

Expand Down
Loading