From 79b0ddd40cb9de532950373035884b055f8d5c68 Mon Sep 17 00:00:00 2001
From: Shayan Hosseini <shayan@neon.tech>
Date: Fri, 9 Feb 2024 15:37:35 -0800
Subject: [PATCH] neonvm: Update VM's .Status.PodName immediately on API server
 (#797)

also added e2e test for runner pod

Fixes #794

---------

Co-authored-by: Em Sharnoff <sharnoff@neon.tech>
---
 .../controllers/virtualmachine_controller.go  |  5 +++
 tests/e2e/runner-pod/00-assert.yaml           | 33 +++++++++++++++++
 tests/e2e/runner-pod/00-create-vm.yaml        | 23 ++++++++++++
 tests/e2e/runner-pod/01-assert.yaml           | 35 +++++++++++++++++++
 tests/e2e/runner-pod/01-delete-pod.yaml       |  8 +++++
 tests/e2e/runner-pod/README.md                |  4 +++
 6 files changed, 108 insertions(+)
 create mode 100644 tests/e2e/runner-pod/00-assert.yaml
 create mode 100644 tests/e2e/runner-pod/00-create-vm.yaml
 create mode 100644 tests/e2e/runner-pod/01-assert.yaml
 create mode 100644 tests/e2e/runner-pod/01-delete-pod.yaml
 create mode 100644 tests/e2e/runner-pod/README.md

diff --git a/neonvm/controllers/virtualmachine_controller.go b/neonvm/controllers/virtualmachine_controller.go
index 20e304488..82d9b03b9 100644
--- a/neonvm/controllers/virtualmachine_controller.go
+++ b/neonvm/controllers/virtualmachine_controller.go
@@ -321,6 +321,11 @@ func (r *VirtualMachineReconciler) doReconcile(ctx context.Context, virtualmachi
 	// Generate runner pod name
 	if len(virtualmachine.Status.PodName) == 0 {
 		virtualmachine.Status.PodName = names.SimpleNameGenerator.GenerateName(fmt.Sprintf("%s-", virtualmachine.Name))
+		// Update the .Status on API Server to avoid creating multiple pods for a single VM
+		// See https://github.com/neondatabase/autoscaling/issues/794 for the context
+		if err := r.Status().Update(ctx, virtualmachine); err != nil {
+			return fmt.Errorf("Failed to update VirtualMachine status: %w", err)
+		}
 	}
 
 	switch virtualmachine.Status.Phase {
diff --git a/tests/e2e/runner-pod/00-assert.yaml b/tests/e2e/runner-pod/00-assert.yaml
new file mode 100644
index 000000000..7ac4263f0
--- /dev/null
+++ b/tests/e2e/runner-pod/00-assert.yaml
@@ -0,0 +1,33 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestAssert
+timeout: 60
+commands:
+  - script: kubectl get pods -n "$NAMESPACE" --no-headers=true -l vm.neon.tech/name=example | [ $(wc -l) -eq 1 ]
+---
+apiVersion: vm.neon.tech/v1
+kind: VirtualMachine
+metadata:
+  name: example
+status:
+  phase: Running
+  conditions:
+    - type: Available
+      status: "True"
+  cpus: 250m
+  memorySize: 1Gi
+  restartCount: 0
+---
+apiVersion: v1
+kind: Pod
+metadata:
+  labels:
+    app.kubernetes.io/name: NeonVM
+    vm.neon.tech/name: example
+  ownerReferences:
+  - apiVersion: vm.neon.tech/v1
+    blockOwnerDeletion: true
+    controller: true
+    kind: VirtualMachine
+    name: example
+status:
+  phase: Running
diff --git a/tests/e2e/runner-pod/00-create-vm.yaml b/tests/e2e/runner-pod/00-create-vm.yaml
new file mode 100644
index 000000000..335741642
--- /dev/null
+++ b/tests/e2e/runner-pod/00-create-vm.yaml
@@ -0,0 +1,23 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+unitTest: false
+---
+apiVersion: vm.neon.tech/v1
+kind: VirtualMachine
+metadata:
+  name: example
+spec:
+  schedulerName: autoscale-scheduler
+  guest:
+    cpus:
+      min: 0.25
+      use: 0.25
+      max: 0.25
+    memorySlotSize: 1Gi
+    memorySlots:
+      min: 1
+      use: 1
+      max: 1
+    rootDisk:
+      image: vm-postgres:15-bullseye
+      size: 1Gi
diff --git a/tests/e2e/runner-pod/01-assert.yaml b/tests/e2e/runner-pod/01-assert.yaml
new file mode 100644
index 000000000..a26f74f49
--- /dev/null
+++ b/tests/e2e/runner-pod/01-assert.yaml
@@ -0,0 +1,35 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestAssert
+timeout: 60
+commands:
+  - script: kubectl get pods -n "$NAMESPACE" --no-headers=true -l vm.neon.tech/name=example | [ $(wc -l) -eq 1 ]
+---
+apiVersion: vm.neon.tech/v1
+kind: VirtualMachine
+metadata:
+  name: example
+status:
+  phase: Running
+  conditions:
+    - type: Available
+      status: "True"
+    - type: Degraded
+      status: "True"
+  cpus: 250m
+  memorySize: 1Gi
+  restartCount: 1
+---
+apiVersion: v1
+kind: Pod
+metadata:
+  labels:
+    app.kubernetes.io/name: NeonVM
+    vm.neon.tech/name: example
+  ownerReferences:
+  - apiVersion: vm.neon.tech/v1
+    blockOwnerDeletion: true
+    controller: true
+    kind: VirtualMachine
+    name: example
+status:
+  phase: Running
diff --git a/tests/e2e/runner-pod/01-delete-pod.yaml b/tests/e2e/runner-pod/01-delete-pod.yaml
new file mode 100644
index 000000000..f88876049
--- /dev/null
+++ b/tests/e2e/runner-pod/01-delete-pod.yaml
@@ -0,0 +1,8 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+unitTest: false
+commands:
+  - script: |
+      set -eux
+      pod="$(kubectl get neonvm -n "$NAMESPACE" example -o jsonpath='{.status.podName}')"
+      kubectl delete -n "$NAMESPACE" pod $pod
diff --git a/tests/e2e/runner-pod/README.md b/tests/e2e/runner-pod/README.md
new file mode 100644
index 000000000..07822aa5b
--- /dev/null
+++ b/tests/e2e/runner-pod/README.md
@@ -0,0 +1,4 @@
+`runner-pod` tests our basic guarantees around runner pod. This includes:
+1. Creating the runner with the right spec and status values.
+2. Having exactly one runner pod running for each VM.
+3. If a pod fails, we create a new pod and start the VM again.
\ No newline at end of file