Skip to content

Commit

Permalink
fix provision_observer2 to delete failing Jobs correctly
Browse files Browse the repository at this point in the history
The new provision observer (provision_observer2.go) introduced in #101 tries to
remove the owned jobs that is failing for a while, but it doesn't work well
because it looks for them by a wrong prefix.

This PR fixes the above problem by using constants.MountProbeNamePrefix and
constants.ProvisionProbeNamePrefix instead of constants.ProbeNamePrefix when
the provision observer looks for the jobs.
  • Loading branch information
ushitora-anqou committed Mar 29, 2024
1 parent e426b8a commit 0469120
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 1 deletion.
2 changes: 1 addition & 1 deletion internal/controller/provision_observer2.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func (p *provisionObserver2) getNodeNameAndStorageClass(ctx context.Context, nam
}

func isProbeJob2(o metav1.OwnerReference) bool {
return o.Kind == "Job" && strings.HasPrefix(o.Name, constants.ProbeNamePrefix)
return o.Kind == "Job" && (strings.HasPrefix(o.Name, constants.MountProbeNamePrefix) || strings.HasPrefix(o.Name, constants.ProvisionProbeNamePrefix))
}

func (p *provisionObserver2) deleteOwnerJobOfPod(ctx context.Context, namespace, podName string) error {
Expand Down
12 changes: 12 additions & 0 deletions test/e2e/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,18 @@ var _ = Describe("PieProbe resource", func() {
}
}
}).Should(Succeed())

// Check that failing Jobs are correctly removed.
Eventually(func(g Gomega) {
stdout, _, err := kubectl("get", "job", "-n", ns, "-l", "pie-probe=pie-probe-dummy-sc")
g.Expect(err).NotTo(HaveOccurred())
g.Expect(stdout).ShouldNot(BeEmpty())
}).Should(Succeed())
Eventually(func(g Gomega) {
stdout, _, err := kubectl("get", "job", "-n", ns, "-l", "pie-probe=pie-probe-dummy-sc")
g.Expect(err).NotTo(HaveOccurred())
g.Expect(stdout).Should(BeEmpty())
}).Should(Succeed())
})
})

Expand Down

0 comments on commit 0469120

Please sign in to comment.