kubernetesdiscovery: fix jobset tracking (#6450)

ensures that if one JobSet has two jobs, we track all the pods. fixes #6383 Signed-off-by: Nick Santos <nick.santos@docker.com>
tilt-dev · Oct 24, 2024 · 8a7280f · 8a7280f
1 parent a94eb9f
commit 8a7280f
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 4 deletions.
diff --git a/internal/store/k8sconv/resource.go b/internal/store/k8sconv/resource.go
@@ -171,7 +171,10 @@ func FilterPods(filter *KubernetesApplyFilter, pods []v1alpha1.Pod) []v1alpha1.P
 	for _, pod := range pods {
 		// Ignore pods from an old replicaset.
 		newestOwner := newestOwnerByAncestorUID[pod.AncestorUID]
-		if hasValidOwner(pod) && newestOwner != nil && pod.Owner.Name != newestOwner.Name {
+		if hasValidOwner(pod) &&
+			newestOwner != nil &&
+			pod.Owner.Name != newestOwner.Name &&
+			pod.Owner.Kind == "ReplicaSet" {
 			continue
 		}
 

diff --git a/internal/store/k8sconv/resource_test.go b/internal/store/k8sconv/resource_test.go
@@ -43,9 +43,21 @@ func TestFilteredPodByOwner(t *testing.T) {
 	time1 := metav1.Time{Time: time.Now().Add(-time.Hour)}
 	time2 := metav1.Time{Time: time.Now().Add(-time.Minute)}
 
-	ownerA := &v1alpha1.PodOwner{Name: "rs-rev-1", CreationTimestamp: time1}
-	ownerB := &v1alpha1.PodOwner{Name: "rs-rev-2", CreationTimestamp: time2}
-	ownerC := &v1alpha1.PodOwner{Name: "alt-rs", CreationTimestamp: time1}
+	ownerA := &v1alpha1.PodOwner{
+		Name:              "rs-rev-1",
+		Kind:              "ReplicaSet",
+		CreationTimestamp: time1,
+	}
+	ownerB := &v1alpha1.PodOwner{
+		Name:              "rs-rev-2",
+		Kind:              "ReplicaSet",
+		CreationTimestamp: time2,
+	}
+	ownerC := &v1alpha1.PodOwner{
+		Name:              "alt-rs",
+		Kind:              "ReplicaSet",
+		CreationTimestamp: time1,
+	}
 	podA := v1alpha1.Pod{Name: "pod-1", Owner: ownerA, AncestorUID: "dep-1"}
 	podB := v1alpha1.Pod{Name: "pod-2", Owner: ownerA, AncestorUID: "dep-1"}
 	podAlt := v1alpha1.Pod{Name: "pod-alt", Owner: ownerC, AncestorUID: "alt-server"}
@@ -71,6 +83,35 @@ func TestFilteredPodByOwner(t *testing.T) {
 	assert.Equal(t, []v1alpha1.Pod{podAlt, podC}, filter(podAlt, podC, podB, podA))
 }
 
+func TestFilterPodsHanldesJobSets(t *testing.T) {
+	time1 := metav1.Time{Time: time.Now().Add(-time.Hour)}
+	time2 := metav1.Time{Time: time.Now().Add(-time.Minute)}
+
+	ownerA := &v1alpha1.PodOwner{
+		Name:              "job-1",
+		Kind:              "Job",
+		CreationTimestamp: time1,
+	}
+	ownerB := &v1alpha1.PodOwner{
+		Name:              "job-2",
+		Kind:              "Job",
+		CreationTimestamp: time2,
+	}
+	podA := v1alpha1.Pod{Name: "pod-1", Owner: ownerA, AncestorUID: "js-1"}
+	podB := v1alpha1.Pod{Name: "pod-2", Owner: ownerB, AncestorUID: "js-1"}
+
+	filter := func(pods ...v1alpha1.Pod) []v1alpha1.Pod {
+		discovery := newDiscovery(pods)
+		res, err := NewKubernetesResource(discovery, nil)
+		require.NoError(t, err)
+		return res.FilteredPods
+	}
+
+	// Ensure that if one JobSet has two Jobs,
+	// all pods are preserved.
+	assert.Equal(t, []v1alpha1.Pod{podA, podB}, filter(podA, podB))
+}
+
 func TestNewKubernetesApplyFilter_Sorted(t *testing.T) {
 	forDeploy, err := k8s.ParseYAMLFromString(testyaml.OutOfOrderYaml)
 	require.NoError(t, err, "Invalid test YAML")