Skip to content

fix(resource/pipeline): fix permit_restart_from_failed_steps = false#160

Merged
masontikhonov merged 5 commits intomasterfrom
CR-26963-fix-restart-from-failed-steps
Apr 22, 2025
Merged

fix(resource/pipeline): fix permit_restart_from_failed_steps = false#160
masontikhonov merged 5 commits intomasterfrom
CR-26963-fix-restart-from-failed-steps

Conversation

@masontikhonov
Copy link
Copy Markdown
Contributor

@masontikhonov masontikhonov commented Mar 19, 2025

What

This fixes permit_restart_from_failed_steps behavior in pipeline resource.

Before this fix, setting permit_restart_from_failed_steps = false or omitting this key resulted in “Permit restart from failed step: Use account settings” for both cases.

New syntax/behavior is:
permit_restart_from_failed_steps = [true|false] with the default value true (“Permit”).
If false, the policy is set to “Forbid”.

New flag is added: permit_restart_from_failed_steps_use_account_settings = [false|true] with the default value false (“do not use account settings”).
If true, permit_restart_from_failed_steps will be ignored and pipeline policy will be set to “Use account settings”.

Warning

BREAKING CHANGES!

  • Previously, permit_restart_from_failed_steps = false resulted in “Permit restart from failed step: Use account settings”.
    From now on, setting permit_restart_from_failed_steps = false will result in “Permit restart from failed step: Forbid”

Fixes #CR-26963

Notes

Checklist

@masontikhonov masontikhonov self-assigned this Mar 19, 2025
@masontikhonov
Copy link
Copy Markdown
Contributor Author

/test

@masontikhonov masontikhonov requested a review from ATGardner March 19, 2025 14:22
@masontikhonov masontikhonov marked this pull request as ready for review March 19, 2025 14:22
`permit_restart_from_failed_steps` property of pipeline spec behaves differently when omit vs set to `false`. This means we should not omit `false` value while sending requests to API.

Fixes #CR-26963
@masontikhonov masontikhonov force-pushed the CR-26963-fix-restart-from-failed-steps branch from 9ac80ec to 2adecb7 Compare March 19, 2025 16:46
@masontikhonov masontikhonov marked this pull request as draft March 19, 2025 16:46
@masontikhonov
Copy link
Copy Markdown
Contributor Author

/test

@masontikhonov masontikhonov changed the title fix(resource/pipeline): fix permit_restart_from_failed_steps to JSON fix(resource/pipeline): fix permit_restart_from_failed_steps = false Mar 19, 2025
@masontikhonov masontikhonov marked this pull request as ready for review March 19, 2025 17:42
@masontikhonov
Copy link
Copy Markdown
Contributor Author

/test

@github-actions github-actions Bot added the docs label Apr 8, 2025
@masontikhonov
Copy link
Copy Markdown
Contributor Author

/test

Comment thread codefresh/resource_pipeline.go Outdated
},
}

hasPermitRestartChanged := d.HasChange("spec.0.permit_restart_from_failed_steps")
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@masontikhonov HasChange functions are normally used in custom diff functions. We should instead just check if permit_restart_from_failed_steps_use_account_settings is set to true and in such case set the property to nil

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@masontikhonov
Copy link
Copy Markdown
Contributor Author

/test

@masontikhonov masontikhonov merged commit 2945191 into master Apr 22, 2025
5 checks passed
@masontikhonov masontikhonov deleted the CR-26963-fix-restart-from-failed-steps branch April 22, 2025 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants