fix provision_observer2 to delete failing Jobs correctly #114
Conversation
ushitora-anqou
force-pushed
the
fix-periodic-job-deletion
branch
from
0469120
to
48dde7c
Compare
|
|
ushitora-anqou
force-pushed
the
fix-periodic-job-deletion
branch
from
48dde7c
to
81787ea
Compare
| @@ -218,6 +218,18 @@ var _ = Describe("PieProbe resource", func() { | |||
| } | |||
| } | |||
| }).Should(Succeed()) | |||
|
|
|||
| // Check that failing Jobs are correctly removed. | |||
| Eventually(func(g Gomega) { | |||
There was a problem hiding this comment.
I consider this test is unstable. I guess the failed jobs can be removed before this Eventually section starts.
How about checking only the deletion of the failed jobs? I mean, it might be good to remove the first Eventually section.
There was a problem hiding this comment.
I agree with you. I updated my patch. The changes are: https://github.com/topolvm/pie/compare/81787eaa12e9bf8ade9d452c4d58ed026af3ec7a..b8496f03e5f5c70a5a03ff72476f85669e1652e7
the failed jobs can be removed before this Eventually section starts.
Actually, even in this situation, the test should pass because the cronjob will create another job, while it can take a long time. I wrote the first Eventually to make the test fail when no jobs are created during the whole run for some reason. But this situation cannot happen because we already check the metrics (on_time=false) for dummy-sc. So, I believe the first Eventually is essentially meaningless.
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. Signed-off-by: Ryotaro Banno <ryotaro.banno@gmail.com>
ushitora-anqou
force-pushed
the
fix-periodic-job-deletion
branch
from
81787ea
to
b8496f0
Compare
The new provision observer (provision_observer2.go) introduced in #101 tries to remove the owned jobs that are 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 failing jobs.