fix: ensure lastScale is not cleared when the resource is not ready

..and enhance lastScaled tests to ensure it persists properly across scales
BrandwatchLtd · Jul 5, 2023 · 74ca900 · 74ca900
1 parent 26bf7f7
commit 74ca900
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 8 deletions.
diff --git a/src/main/java/com/brandwatch/kafka_pod_autoscaler/KafkaPodAutoscalerReconciler.java b/src/main/java/com/brandwatch/kafka_pod_autoscaler/KafkaPodAutoscalerReconciler.java
@@ -56,18 +56,17 @@ public UpdateControl<KafkaPodAutoscaler> reconcile(KafkaPodAutoscaler kafkaPodAu
  .rescheduleAfter(Duration.ofSeconds(10));
  }
 
- if (!resource.isReady()) {
+ if (resource.getReplicaCount() == 0) {
  statusLogger.clearStatus();
- statusLogger.log(targetKind + " is not ready. Skipping scale");
+ statusLogger.log(targetKind + " has been scaled to zero. Skipping scale");
  return UpdateControl.patchStatus(kafkaPodAutoscaler)
  .rescheduleAfter(Duration.ofSeconds(10));
  }
 
- if (resource.getReplicaCount() == 0) {
- statusLogger.clearStatus();
- statusLogger.log(targetKind + " has been scaled to zero. Skipping scale");
+ if (!resource.isReady()) {
+ statusLogger.log(targetKind + " is not ready. Skipping scale");
  return UpdateControl.patchStatus(kafkaPodAutoscaler)
-  .rescheduleAfter(Duration.ofSeconds(10));
+ .rescheduleAfter(Duration.ofSeconds(10));
  }
 
  var currentReplicaCount = kafkaPodAutoscaler.getSpec().getDryRun()

diff --git a/src/test/java/com/brandwatch/kafka_pod_autoscaler/KafkaPodAutoscalerReconcilerTest.java b/src/test/java/com/brandwatch/kafka_pod_autoscaler/KafkaPodAutoscalerReconcilerTest.java
@@ -21,6 +21,7 @@
 
 import brandwatch.com.v1alpha1.KafkaPodAutoscaler;
 import brandwatch.com.v1alpha1.KafkaPodAutoscalerSpec;
+import brandwatch.com.v1alpha1.KafkaPodAutoscalerStatus;
 import brandwatch.com.v1alpha1.kafkapodautoscalerspec.ScaleTargetRef;
 import brandwatch.com.v1alpha1.kafkapodautoscalerspec.Triggers;
 import brandwatch.com.v1alpha1.kafkapodautoscalerstatus.TriggerResults;
@@ -99,13 +100,19 @@ public void reconcile_missingResource() {
 
  @Test
  public void notReadyResource() {
+ var status = new KafkaPodAutoscalerStatus();
+ status.setLastScale("2000-01-02T03:04:05.006Z");
+ kpa.setStatus(status);
+
  when(deploymentResource.isReady()).thenReturn(false);
 
  var updateControl = reconciler.reconcile(kpa, mockContext);
 
  verify(deploymentResource, never()).scale(anyInt());
  assertThat(updateControl.getResource().getStatus().getMessage())
- .isEqualTo("Deployment is not ready. Skipping scale");
+ .isEqualTo("Deployment is not ready. Skipping scale");
+ assertThat(updateControl.getResource().getStatus().getLastScale())
+ .isEqualTo("2000-01-02T03:04:05.006Z");
  }
 
  @Test
@@ -140,14 +147,17 @@ public void scaledWithinCooloff() {
  when(deployment.getSpec().getReplicas()).thenReturn(1);
 
  var updateControl = reconciler.reconcile(kpa, mockContext);
+ var lastScale = updateControl.getResource().getStatus().getLastScale();
  verify(deploymentResource).scale(3);
 
  for (var i = 0; i < 5; i++) {
  updateControl = reconciler.reconcile(kpa, mockContext);
 
  verify(deploymentResource).scale(3);
  assertThat(updateControl.getResource().getStatus().getMessage())
- .isEqualTo("Deployment has been scaled recently. Skipping scale");
+ .isEqualTo("Deployment has been scaled recently. Skipping scale");
+ assertThat(updateControl.getResource().getStatus().getLastScale())
+ .isEqualTo(lastScale);
  assertThat(updateControl.getResource().getStatus().getCurrentReplicaCount())
  .isEqualTo(1);
  assertThat(updateControl.getResource().getStatus().getPartitionCount())