Skip to content

Adding checks for rescued tasks during reconciliation#3650

Merged
fabianvf merged 2 commits intooperator-framework:masterfrom
VenkatRamaraju:rescue-reconciliation
Aug 11, 2020
Merged

Adding checks for rescued tasks during reconciliation#3650
fabianvf merged 2 commits intooperator-framework:masterfrom
VenkatRamaraju:rescue-reconciliation

Conversation

@VenkatRamaraju
Copy link
Copy Markdown
Contributor

Description of the change:
Added a check to determine whether or not a task has been rescued.

Motivation for the change:
Avoid adding rescued tasks to the list of failures during reconciliation.
Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1856714

Copy link
Copy Markdown
Member

@fabianvf fabianvf left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 10, 2020
@fabianvf fabianvf merged commit cc0785b into operator-framework:master Aug 11, 2020
jmrodri pushed a commit that referenced this pull request Aug 13, 2020
* Added fragments file

* Fragment file

* Changes
func (je JobEvent) Rescued() bool {
if rescued, contains := je.EventData["rescued"]; contains {
for _, v := range rescued.(map[string]interface{}) {
if int(v.(float64)) == 1 {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is just a drive-by comment :) but I happened to look at this PR and wondered -- is it always the case that "rescued" will only ever be 0 or 1? Is it possible multiple tasks could be rescued and thus this count could be 2, or 3, or ???

In which case, wouldn't it be more correct to say:

if int(v.(float64)) >= 1 

joelanford pushed a commit to joelanford/operator-sdk that referenced this pull request Sep 17, 2020
…work#3650)

* Stopping reconciliation on rescue

* Added error check
joelanford pushed a commit to joelanford/operator-sdk that referenced this pull request Sep 17, 2020
…onciliation) (operator-framework#3729)

* Added fragments file

* Fragment file

* Changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants