search

kubernetes-sigs / prow

Prow is a Kubernetes based CI/CD system developed to serve the Kubernetes community. This repository contains Prow source code and Hugo sources for Prow documentation site.
https://docs.prow.k8s.io
Apache License 2.0
129 stars 99 forks source link

Tide re-ran an overridden job #238

Open dhaiducek opened 3 months ago

dhaiducek commented 3 months ago

We have a job I overrode when I submitted the PR but then it took a while for it to get approved. Once it finally did get approved, Tide ran the overridden job, which it shouldn't have done.

petr-muller commented 2 months ago

it shouldn't have done.

There are opinions on this, it's not entirely clear. Tide retests jobs when HEAD of the target branch moves since the job executed, because target branch change means the job results before that are essentially obsolete (ran on different code than would be the result of the merge now). Now, when people override, they do it for two reasons:

  1. I don't care of this job at all, it is simply broken - these people want override to stick
  2. I have reviewed the results and they are not problematic (like, will be fixed later). But HEAD move makes these results obsolete, you may get new ones.

I'm not saying we should do nothing, but accommodating use case 1 too much incentives repos to have broken jobs and not fix them or disable them? Or maybe we need to distinguish a sticky override and normal one. I'm not sure.

dhaiducek commented 2 months ago

Those are fair points, and if the HEAD of the target branch changed, I agree it does seem it would make sense for Tide to retry the job before merging the PR.

Would it make a difference if the job it re-ran is a required job that's set to always_run: false? I guess my preference would be to reset the override and not retry the job to let the user determine whether it should be run or overridden again? (And maybe post a comment to that effect?)