From af660b7c3362e10b512106dfb7ca57a0b13f870f Mon Sep 17 00:00:00 2001
From: Lucian Carata <lc525@users.noreply.github.com>
Date: Wed, 8 May 2024 09:34:20 +0100
Subject: [PATCH] fix(k6): allow k8s CR updates during load test (#5587)

This fixes a issue with the k8s-enabled k6 tests, where we had to
first check whether seldon objects existed before creating them.

This check is now removed, with new k6 tests being able to update
existing model parameters via kube `apply`. Previously, the way
the memory requirements were defined in the scenario script
(by not using units of measure for the memory requirements)
triggered a conversion of units on the seldon-controller side, with
this component also taking the ownership of the field (becoming
FieldManager).

By specifying units of measure for model memory requirements,
this no longer happens, allowing k8s updates to be performed via k6.
---
 tests/k6/components/k8s.js        | 40 ++++++++++++-------------------
 tests/k6/configs/k8s/base/k6.yaml |  7 +++++-
 2 files changed, 21 insertions(+), 26 deletions(-)

diff --git a/tests/k6/components/k8s.js b/tests/k6/components/k8s.js
index 4472c899e0..32982de322 100644
--- a/tests/k6/components/k8s.js
+++ b/tests/k6/components/k8s.js
@@ -37,17 +37,11 @@ function seldonObjExists(kind, name, ns) {
 }
 
 export function loadModel(modelName, data, awaitReady=true) {
-    // TODO: Update existing model with new CR definition.
-    // At the moment, if an object with the same name exists, it will not be
-    // re-loaded with different settings. This is because we get a k8s apply
-    // conflict caused by a FieldManager being set on `.spec.memory`
-    if(!seldonObjExists(seldonObjectType.MODEL, modelName, namespace)) {
-        kubeclient.apply(data)
-        let created = kubeclient.get(seldonObjectType.MODEL.description, modelName, namespace)
-        if ('uid' in created.metadata) {
-            if (awaitReady && schedulerClient != null) {
-                awaitStatus(modelName, "ModelAvailable")
-            }
+    kubeclient.apply(data)
+    let created = kubeclient.get(seldonObjectType.MODEL.description, modelName, namespace)
+    if ('uid' in created.metadata) {
+        if (awaitReady && schedulerClient != null) {
+            awaitStatus(modelName, "ModelAvailable")
         }
     }
 }
@@ -62,13 +56,11 @@ export function unloadModel(modelName, awaitReady=true) {
 }
 
 export function loadPipeline(pipelineName, data, awaitReady=true) {
-    if(!seldonObjExists(seldonObjectType.PIPELINE, pipelineName, namespace)) {
-        kubeclient.apply(data)
-        let created = kubeclient.get(seldonObjectType.PIPELINE.description, pipelineName, namespace)
-        if ('uid' in created.metadata) {
-            if (awaitReady && schedulerClient != null) {
-                awaitStatus(pipelineName, "PipelineReady")
-            }
+    kubeclient.apply(data)
+    let created = kubeclient.get(seldonObjectType.PIPELINE.description, pipelineName, namespace)
+    if ('uid' in created.metadata) {
+        if (awaitReady && schedulerClient != null) {
+            awaitStatus(pipelineName, "PipelineReady")
         }
     }
 }
@@ -83,13 +75,11 @@ export function unloadPipeline(pipelineName, awaitReady = true) {
 }
 
 export function loadExperiment(experimentName, data, awaitReady=true) {
-    if(!seldonObjExists(seldonObjectType.EXPERIMENT, experimentName, namespace)) {
-        kubeclient.apply(data)
-        let created = kubeclient.get(seldonObjectType.EXPERIMENT.description, experimentName, namespace)
-        if ('uid' in created.metadata) {
-            if (awaitReady && schedulerClient != null) {
-                awaitExperimentStart(experimentName)
-            }
+    kubeclient.apply(data)
+    let created = kubeclient.get(seldonObjectType.EXPERIMENT.description, experimentName, namespace)
+    if ('uid' in created.metadata) {
+        if (awaitReady && schedulerClient != null) {
+            awaitExperimentStart(experimentName)
         }
     }
 }
diff --git a/tests/k6/configs/k8s/base/k6.yaml b/tests/k6/configs/k8s/base/k6.yaml
index f005a361a6..d4adda8eb0 100644
--- a/tests/k6/configs/k8s/base/k6.yaml
+++ b/tests/k6/configs/k8s/base/k6.yaml
@@ -57,8 +57,13 @@ spec:
           value: "tfsimplea,pytorch-cifar10a,tfmnista,mlflow-winea,irisa"
         - name: MODEL_TYPE
           value: "tfsimple,pytorch_cifar10,tfmnist,mlflow_wine,iris"
+        # Specify MODEL_MEMORY_BYTES with suffixes (b, k, m, g) rather than
+        # numbers without units of measure. If supplying "naked numbers",
+        # the seldon operator will take care of converting the number for you
+        # but also take ownership of the field (as FieldManager), so the next
+        # time you run the scenario creating/updating of the model CR will fail.
         - name: MODEL_MEMORY_BYTES
-          value: "400000,8000000,43000000,200000,3000000"
+          value: "400k,8m,43m,200k,3m"
         - name: MAX_NUM_MODELS
           value: "800,100,25,100,100"
           # value: "0,0,25,100,100"