Complete GKE testing (#75)

* adds istio to function tests

* adds more functional test data

and uses prescribed "testdata" directory instead of "fixtures"

* fixes build-arg validation

we only need to ensure that a single "=" char exists between the key and
value, not if the value has equal signs in it

* build-arg validation accounts for blank "key"

* adds many many many gke tests

* adds more gke tests

- fixes teardown
- extracts tasks into reusable helper funcs
- adds test funcs for checking log and message delivery

* adds mutli-stage build context

* adds multi-stage and concurrent tests

* updates tooling

* updates buildkit and cobra deps

* updates buildkit workerpool

the previous "abnormal" descriptor didn't really encompass the details
of what's happening in k8s. it's possible that pod can be Running but
not ready, in which it is "starting" and should be afforded an
opportunity to finish the process.

furthermore, when performing math, it makes more sense to have the
subtractions variable declared before the loop. this allows us to gather
pods into groups like "starting" or "pending" for display purposes but
still be able to apply offsets for certain conditions using the
subtractions var

* refactors buildkit worker pool scale determination

YAWPR (yet another worker pool refactor)

trying to collect builder states and determine the proper statefulset
scale along with servicing build requests resulted in a BRUTAL for loop
inside the worker pool. furthermore, there are so many edge cases that
this required special "offsets" to get all test cases to pass. this
change extracts that logic into a dedicated type that focuses singularly
on evaluating pod states and, as a later step, calculates the proper
replica count

additionally, this improves the worker pool tests so that we test
behaviors more thoroughly by adding more tests with requests > 0

* adds DSE docker-context

* updates buildkit storage size for DSE

* adds dse test

and updates createBuild timeout to account for this gargantuan image
build time

* updates testenv dep

adds bigger gke nodes so we don't have to scale up the cluster during
testing

* vector ignores empty messages during testing

so we can validate the payload sent to redis w/o erroring out since some
JSON payloads were sent with an empty "msg" field. this is not an error,
the vector config just needed tweaking

* adds log field to ScaleArbiter

and logs out valuable state information when invoking DetermineReplicas

* updates buildkit image tag

to align with library upgrade

* adds note to garbageClientHack

so the peoples understand why

api/openapi-spec/swagger.json

@@ -3073,6 +3073,7 @@
           "$ref": "#/definitions/.SecretCredentials"
         },
         "server": {
+          "description": "NOTE: this field was previously used to assert the presence of an auth entry inside of secret credentials. if the\n Server was missing, then an error was raised. this design is limiting because it requires users to create\n several `registryAuth` items with the same secret if they want to verify the presence. in a future api version,\n we may remove the Server field from this type and replace it with one or more fields that service the needs all\n credential types.",
           "type": "string"
         }
       }

deployments/crds/hephaestus.dominodatalab.com_imagebuildmessages.yaml

@@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.8.0
+    controller-gen.kubebuilder.io/version: v0.10.0
   creationTimestamp: null
   name: imagebuildmessages.hephaestus.dominodatalab.com
 spec:
@@ -118,9 +118,3 @@ spec:
     storage: true
     subresources:
       status: {}
-status:
-  acceptedNames:
-    kind: ""
-    plural: ""
-  conditions: []
-  storedVersions: []

deployments/crds/hephaestus.dominodatalab.com_imagebuilds.yaml

@@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.8.0
+    controller-gen.kubebuilder.io/version: v0.10.0
   creationTimestamp: null
   name: imagebuilds.hephaestus.dominodatalab.com
 spec:
@@ -236,9 +236,3 @@ spec:
     storage: true
     subresources:
       status: {}
-status:
-  acceptedNames:
-    kind: ""
-    plural: ""
-  conditions: []
-  storedVersions: []

deployments/crds/hephaestus.dominodatalab.com_imagecaches.yaml

@@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.8.0
+    controller-gen.kubebuilder.io/version: v0.10.0
   creationTimestamp: null
   name: imagecaches.hephaestus.dominodatalab.com
 spec:
@@ -178,9 +178,3 @@ spec:
     storage: true
     subresources:
       status: {}
-status:
-  acceptedNames:
-    kind: ""
-    plural: ""
-  conditions: []
-  storedVersions: []

deployments/helm/hephaestus/values.yaml

@@ -280,7 +280,7 @@ buildkit:
   image:
     registry: ""
     repository: moby/buildkit
-    tag: v0.10.4-rootless
+    tag: v0.10.5-rootless
     pullPolicy: IfNotPresent
     pullSecrets: []
 

go.mod

@@ -27,10 +27,10 @@ require (
 	github.com/go-logr/zapr v1.2.3
 	github.com/gofrs/flock v0.7.3
 	github.com/h2non/filetype v1.1.1
-	github.com/moby/buildkit v0.10.4
+	github.com/moby/buildkit v0.10.5
 	github.com/pkg/errors v0.9.1
 	github.com/rabbitmq/amqp091-go v1.5.0
-	github.com/spf13/cobra v1.4.0
+	github.com/spf13/cobra v1.6.0
 	github.com/stretchr/testify v1.7.0
 	go.uber.org/zap v1.21.0
 	golang.org/x/crypto v0.0.0-20220315160706-3147a52a75dd
@@ -112,7 +112,7 @@ require (
 	github.com/grpc-ecosystem/grpc-gateway v1.16.0 // indirect
 	github.com/hashicorp/go-cleanhttp v0.5.1 // indirect
 	github.com/imdario/mergo v0.3.12 // indirect
-	github.com/inconshreveable/mousetrap v1.0.0 // indirect
+	github.com/inconshreveable/mousetrap v1.0.1 // indirect
 	github.com/jmespath/go-jmespath v0.4.0 // indirect
 	github.com/josharian/intern v1.0.0 // indirect
 	github.com/json-iterator/go v1.1.12 // indirect
@@ -145,7 +145,7 @@ require (
 	go.opentelemetry.io/proto/otlp v0.12.0 // indirect
 	go.uber.org/atomic v1.7.0 // indirect
 	golang.org/x/net v0.0.0-20220722155237-a158d28d115b // indirect
-	golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f // indirect
+	golang.org/x/sys v0.1.0 // indirect
 	golang.org/x/term v0.0.0-20210927222741-03fcf44c2211 // indirect
 	golang.org/x/text v0.3.7 // indirect
 	golang.org/x/time v0.0.0-20220609170525-579cf78fd858 // indirect

go.sum

@@ -171,7 +171,7 @@ github.com/coreos/go-semver v0.2.0/go.mod h1:nnelYz7RCh+5ahJtPPxZlU+153eP4D4r3Ee
 github.com/coreos/go-systemd v0.0.0-20190321100706-95778dfbb74e/go.mod h1:F5haX7vjVVG0kc13fIWeqUViNPyEJxv/OmvnBo0Yme4=
 github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f/go.mod h1:E3G3o1h8I7cfcXa63jLwjI0eiQQMgzzUDFVpN/nH/eA=
 github.com/cpuguy83/go-md2man/v2 v2.0.0/go.mod h1:maD7wRr/U5Z6m/iR4s+kqSMx2CaBsrgA7czyZG/E6dU=
-github.com/cpuguy83/go-md2man/v2 v2.0.1/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o=
+github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o=
 github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
 github.com/creack/pty v1.1.11 h1:07n33Z8lZxZ2qwegKbObQohDhXDQxiMMz1NOUGYlesw=
 github.com/creack/pty v1.1.11/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
@@ -391,8 +391,9 @@ github.com/ianlancetaylor/demangle v0.0.0-20181102032728-5e5cf60278f6/go.mod h1:
 github.com/ianlancetaylor/demangle v0.0.0-20200824232613-28f6c0f3b639/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc=
 github.com/imdario/mergo v0.3.12 h1:b6R2BslTbIEToALKP7LxUvijTsNI9TAe80pLWN2g/HU=
 github.com/imdario/mergo v0.3.12/go.mod h1:jmQim1M+e3UYxmgPu/WyfjB3N3VflVyUjjjwH0dnCYA=
-github.com/inconshreveable/mousetrap v1.0.0 h1:Z8tu5sraLXCXIcARxBp/8cbvlwVa7Z1NHg9XEKhtSvM=
 github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8=
+github.com/inconshreveable/mousetrap v1.0.1 h1:U3uMjPSQEBMNp1lFxmllqCPM6P5u/Xq7Pgzkat/bFNc=
+github.com/inconshreveable/mousetrap v1.0.1/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw=
 github.com/jessevdk/go-flags v1.4.0/go.mod h1:4FA24M0QyGHXBuZZK/XkWh8h0e1EYbRYJSGM75WSRxI=
 github.com/jmespath/go-jmespath v0.4.0 h1:BEgLn5cpjn8UN1mAw4NjwDrS35OdebyEtFe+9YPoQUg=
 github.com/jmespath/go-jmespath v0.4.0/go.mod h1:T8mJZnbsbmF+m6zOOFylbeCJqk5+pHWvzYPziyZiYoo=
@@ -438,8 +439,8 @@ github.com/matttproud/golang_protobuf_extensions v1.0.2-0.20181231171920-c182aff
 github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG+4E0Y=
 github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0=
 github.com/mitchellh/mapstructure v1.1.2/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y=
-github.com/moby/buildkit v0.10.4 h1:FvC+buO8isGpUFZ1abdSLdGHZVqg9sqI4BbFL8tlzP4=
-github.com/moby/buildkit v0.10.4/go.mod h1:Yajz9vt1Zw5q9Pp4pdb3TCSUXJBIroIQGQ3TTs/sLug=
+github.com/moby/buildkit v0.10.5 h1:d9krS/lG3dn6N7y+R8o9PTgIixlYAaDk35f3/B4jZOw=
+github.com/moby/buildkit v0.10.5/go.mod h1:Yajz9vt1Zw5q9Pp4pdb3TCSUXJBIroIQGQ3TTs/sLug=
 github.com/moby/locker v1.0.1 h1:fOXqR41zeveg4fFODix+1Ch4mj/gT0NE1XJbp/epuBg=
 github.com/moby/sys/mountinfo v0.6.0 h1:gUDhXQx58YNrpHlK4nSL+7y2pxFZkUcXqzFDKWdC0Oo=
 github.com/moby/sys/signal v0.6.0 h1:aDpY94H8VlhTGa9sNYUFCFsMZIUh5wm0B6XkIoJj/iY=
@@ -545,8 +546,8 @@ github.com/spf13/afero v1.1.2/go.mod h1:j4pytiNVoe2o6bmDsKpLACNPDBIoEAkihy7loJ1B
 github.com/spf13/afero v1.2.2/go.mod h1:9ZxEEn6pIJ8Rxe320qSDBk6AsU0r9pR7Q4OcevTdifk=
 github.com/spf13/cast v1.3.0/go.mod h1:Qx5cxh0v+4UWYiBimWS+eyWzqEqokIECu5etghLkUJE=
 github.com/spf13/cobra v1.0.0/go.mod h1:/6GTrnGXV9HjY+aR4k0oJ5tcvakLuG6EuKReYlHNrgE=
-github.com/spf13/cobra v1.4.0 h1:y+wJpx64xcgO1V+RcnwW0LEHxTKRi2ZDPSBjWnrg88Q=
-github.com/spf13/cobra v1.4.0/go.mod h1:Wo4iy3BUC+X2Fybo0PDqwJIv3dNRiZLHQymsfxlB84g=
+github.com/spf13/cobra v1.6.0 h1:42a0n6jwCot1pUmomAp4T7DeMD+20LFv4Q54pxLf2LI=
+github.com/spf13/cobra v1.6.0/go.mod h1:IOw/AERYS7UzyrGinqmz6HLUo219MORXGxhbaJUqzrY=
 github.com/spf13/jwalterweatherman v1.0.0/go.mod h1:cQK4TGJAtQXfYWX+Ddv3mKDzgVb68N+wFjFa4jdeBTo=
 github.com/spf13/pflag v0.0.0-20170130214245-9ff6c6923cff/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4=
 github.com/spf13/pflag v1.0.3/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4=
@@ -815,8 +816,8 @@ golang.org/x/sys v0.0.0-20211107104306-e0b2ad06fe42/go.mod h1:oPkhp1MJrh7nUepCBc
 golang.org/x/sys v0.0.0-20211124211545-fe61309f8881/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220114195835-da31bd327af9/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220412211240-33da011f77ad/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f h1:v4INt8xihDGvnrfjMDVXGxw9wrfxYyCjk0KbXjhR55s=
-golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.1.0 h1:kunALQeHf1/185U1i0GOB/fy1IPRDDpuoOOqRReG57U=
+golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
 golang.org/x/term v0.0.0-20210927222741-03fcf44c2211 h1:JGgROgKl9N8DuW20oFS5gxc+lE67/N3FcwmBPMe7ArY=
 golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=

pkg/api/hephaestus/v1/imagebuild_webhook.go

@@ -54,7 +54,7 @@ func (in *ImageBuild) validateImageBuild(action string) error {
 	}
 
 	for idx, arg := range in.Spec.BuildArgs {
-		if len(strings.Split(arg, "=")) != 2 {
+		if ss := strings.SplitN(arg, "=", 2); len(ss) != 2 || strings.TrimSpace(ss[0]) == "" {
 			log.V(1).Info("Build arg is invalid", "arg", arg)
 			errList = append(errList, field.Invalid(
 				fp.Child("buildArgs").Index(idx), arg, "must use a <key>=<value> format",

pkg/buildkit/worker/pool.go

@@ -42,6 +42,11 @@ var (
 
 	newUUID          = uuid.NewUUID
 	statefulPodRegex = regexp.MustCompile(`^.*-(\d+)$`)
+
+	// NOTE: https://github.com/kubernetes/client-go/issues/970
+	// 	we have to hack the client during testing because the current version
+	// 	of the k8s fake client does not support the ApplyPatchType
+	garbageClientHack = false
 )
 
 const (
@@ -137,7 +142,7 @@ func NewPool(
 		defer ticker.Stop()
 
 		for {
-			if err := wp.updateWorkers(wp.ctx); err != nil {
+			if err := wp.reconcileWorkers(wp.ctx); err != nil {
 				wp.log.Error(err, "Failed to update worker pool")
 			}
 
@@ -354,7 +359,7 @@ func (p *AutoscalingPool) extractHostname(epSlice *discoveryv1.EndpointSlice, po
 }
 
 // reconcile pods in worker pool
-func (p *AutoscalingPool) updateWorkers(ctx context.Context) error {
+func (p *AutoscalingPool) reconcileWorkers(ctx context.Context) error {
 	p.mu.Lock()
 	defer p.mu.Unlock()
 
@@ -369,108 +374,25 @@ func (p *AutoscalingPool) updateWorkers(ctx context.Context) error {
 		return getOrdinal(podList.Items[i].Name) < getOrdinal(podList.Items[j].Name)
 	})
 
-	var allPods []string
-	var leased []string
-	var pending []string
-	var removals []string
-	var abnormal []string
+	arbiter := NewScaleArbiter(p.log, p.podClient, p.podMaxIdleTime)
 
-	// mark pods for removal based on state and lease pods when requests exist
 	for _, pod := range podList.Items {
-		allPods = append(allPods, pod.Name)
-
-		log := p.log.WithValues("podName", pod.Name)
-		log.Info("Evaluating pod metadata and status")
-
-		if id, ok := pod.Annotations[managerIDAnnotation]; ok && id != p.uuid { // mark unmanaged pods
-			log.Info("Eligible for termination, manager id mismatch", "expected", p.uuid, "actual", id)
-
-			removals = append(removals, pod.Name)
-		} else if _, hasLease := pod.Annotations[leasedByAnnotation]; hasLease { // mark leased pods
-			log.Info("Ineligible for termination, pod is leased")
-
-			leased = append(leased, pod.Name)
-		} else if pod.Status.Phase == corev1.PodPending { // mark pending pods
-			pending = append(pending, pod.Name)
-
-			if elapsed := time.Since(pod.CreationTimestamp.Time); elapsed < p.podMaxIdleTime {
-				log.Info("Pending pod is not old enough for termination", "gracePeriod", p.podMaxIdleTime-elapsed)
-			} else {
-				removals = append(removals, pod.Name)
-			}
-		} else if p.isOperationalPod(ctx, pod.Name) { // dispatch builds/check expiry/check age on operation pods
-			log.Info("Pod is operational")
-
-			if req := p.requests.Dequeue(); req != nil {
-				log.Info("Processing dequeued pod request with operational pod")
-				if p.processPodRequest(ctx, req, pod) {
-					leased = append(leased, pod.Name)
-				}
-				continue
-			}
-
-			if ts, ok := pod.Annotations[expiryTimeAnnotation]; ok {
-				expiry, err := time.Parse(time.RFC3339, ts)
-
-				if err != nil {
-					log.Info("Cannot parse expiry time, assuming expired", "expiry", expiry)
-					removals = append(removals, pod.Name)
-				} else if time.Now().After(expiry) {
-					log.Info("Eligible for termination, ttl has expired", "expiry", expiry)
-					removals = append(removals, pod.Name)
-				}
-			} else if time.Since(pod.CreationTimestamp.Time) > p.podMaxIdleTime {
-				log.Info("Eligible for termination, missing expiry time and pod age older than max idle time")
-				removals = append(removals, pod.Name)
-			}
-		} else { // mark abnormal pods
-			// TODO: if a buildkit pod is coming up initially and is NOT in a pending state, then the controller never
-			// 	gives it a chances to reach a steady state and terminates it prematurely. this check should be a little
-			// 	bit more robust. i think we need to make this function a little less unwieldy with all its if/else if
-			// 	branches.
-			log.Info(
-				"Unknown phase detected",
-				"phase", pod.Status.Phase,
-				"conditions", pod.Status.Conditions,
-				"containerStatuses", pod.Status.ContainerStatuses,
-			)
-			abnormal = append(abnormal, pod.Name)
-		}
-	}
-
-	// collect names of pods that might be terminated
-	subtractionMap := make(map[string]bool)
-	for _, name := range removals {
-		subtractionMap[name] = true
-	}
-	for _, name := range abnormal {
-		subtractionMap[name] = true
+		p.log.Info("Evaluating pod metadata and status", "podName", pod.Name)
+		arbiter.EvaluatePod(ctx, p.uuid, pod)
 	}
-
-	// calculate which pods can be removed based reverse-ordinal position
-	var subtractions int
-	for idx := range allPods {
-		reverseIdx := len(allPods) - 1 - idx
-
-		if subtractionMap[allPods[reverseIdx]] {
-			subtractions++
-		} else {
+	for _, observation := range arbiter.LeasablePods() {
+		req := p.requests.Dequeue()
+		if req == nil {
 			break
 		}
+
+		p.log.Info("Processing dequeued pod request with operational pod")
+		if p.processPodRequest(ctx, req, observation.Pod) || garbageClientHack {
+			observation.MarkLeased()
+		}
 	}
 
-	podCount := len(allPods)
-	requestCount := p.requests.Len()
-	replicas := int32(podCount + requestCount - subtractions)
-
-	p.log.Info("Pod evaluation complete",
-		"allPods", allPods,
-		"leasedPods", leased,
-		"pendingPods", pending,
-		"removalPods", removals,
-		"abnormalPods", abnormal,
-		"podRequests", requestCount,
-	)
+	replicas := arbiter.DetermineReplicas(p.requests.Len())
 
 	p.log.Info("Using statefulset scale", "replicas", replicas)
 	_, err = p.statefulSetClient.UpdateScale(
@@ -481,7 +403,7 @@ func (p *AutoscalingPool) updateWorkers(ctx context.Context) error {
 				Name:      p.statefulSetName,
 				Namespace: p.namespace,
 			},
-			Spec: autoscalingv1.ScaleSpec{Replicas: replicas},
+			Spec: autoscalingv1.ScaleSpec{Replicas: int32(replicas)},
 		},
 		metav1.UpdateOptions{FieldManager: fieldManagerName},
 	)
@@ -532,46 +454,6 @@ func (p *AutoscalingPool) triggerReconcile() {
 	}
 }
 
-// ensure pod is operational by checking its phase and conditions
-func (p *AutoscalingPool) isOperationalPod(ctx context.Context, podName string) (verdict bool) {
-	// fetch the latest version of the pod
-	pod, err := p.podClient.Get(ctx, podName, metav1.GetOptions{})
-	if err != nil {
-		p.log.Error(err, "Failed to check if pod is operational")
-		return
-	}
-
-	// this does not mean the pod is usable but is a good sanity check
-	if pod.Status.Phase != corev1.PodRunning {
-		return
-	}
-
-	// assert the following:
-	// 	- pod has been scheduled on a node
-	// 	- all init containers have completed successfully
-	// 	- all containers in the pod are ready
-	// 	- the pod is able to serve requests and should be added to the load balancing pools of all matching Services
-	var scheduled, initialized, containersReady, podReady bool
-	for _, condition := range pod.Status.Conditions {
-		switch condition.Type {
-		case corev1.PodScheduled:
-			scheduled = condition.Status == corev1.ConditionTrue
-		case corev1.PodInitialized:
-			initialized = condition.Status == corev1.ConditionTrue
-		case corev1.ContainersReady:
-			containersReady = condition.Status == corev1.ConditionTrue
-		case corev1.PodReady:
-			podReady = condition.Status == corev1.ConditionTrue
-		}
-	}
-
-	// lastly, the status fields are not updated when a pod is terminated, so we have to check the metadata to determine
-	// if the pod was deleted by a scale-down event and the process is taking longer than expected to exit
-	notDeleted := pod.DeletionTimestamp == nil
-
-	return scheduled && initialized && containersReady && podReady && notDeleted
-}
-
 // diagnose elements that could lead to a failure
 func (p *AutoscalingPool) diagnoseFailure(ctx context.Context, pod corev1.Pod) {
 	log := p.log.WithName("diagnosis").WithValues("podName", pod.Name)