Skip to content

Commit

Permalink
Fix bug with skip-current-step
Browse files Browse the repository at this point in the history
We have observed a bug in the `skip-current-step` action that manifests itself when the current step is `Paused` due to `InconclusiveAnalysisRun`. Although executing the action moves the `currentStepIndex` forward, the rollout remains `Paused`. We then have to run the `resume` action to unpause the rollout.

I have found by experimentation that the reason the rollout remains paused is because the `skip-current-step` action sets `obj.status.controllerPause` to `false`. The `controllerPause` variable is not meant to be modified – it's sufficient to set `obj.status.pauseConditions` to `nil`. When `pauseConditions` is the only variable modified by the action, `skip-current-step` works properly for us and we don't experience the bug described above.

In addition, the `obj.spec.pause = false` operation is incorrect, because the rollout specification defines `obj.spec.paused` (with the letter `d`). In addition, `obj.spec.paused` indicates a manual pause (triggered by a user, rather than by the rollout controller) and it's debatable if `skip-current-step` should resume a manual pause. The `resume` action can be used for that purpose. I have therefore removed that line entirely. 

I have also added an additional if statement that checks if `obj.status.pauseConditions` is defined and has at least one element. Otherwise the variable isn't touched. This logic is modeled on the `resume` action.
  • Loading branch information
akorzy-pl authored Nov 30, 2023
1 parent f912805 commit 61030d5
Showing 1 changed file with 3 additions and 3 deletions.
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
if obj.status ~= nil then
if obj.spec.strategy.canary ~= nil and obj.spec.strategy.canary.steps ~= nil and obj.status.currentStepIndex < table.getn(obj.spec.strategy.canary.steps) then
obj.status.pauseConditions = nil
obj.spec.pause = false
if obj.status.pauseConditions ~= nil and table.getn(obj.status.pauseConditions) > 0 then
obj.status.pauseConditions = nil
end
obj.status.currentStepIndex = obj.status.currentStepIndex + 1
obj.status.controllerPause = false
end
end
return obj

0 comments on commit 61030d5

Please sign in to comment.