Skip to content

Bug 1856714: Adding checks for rescued tasks during reconciliation#3727

Merged
jmrodri merged 7 commits intooperator-framework:v0.19.xfrom
VenkatRamaraju:v0.19.x
Aug 13, 2020
Merged

Bug 1856714: Adding checks for rescued tasks during reconciliation#3727
jmrodri merged 7 commits intooperator-framework:v0.19.xfrom
VenkatRamaraju:v0.19.x

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
Backporting the PR #3650

@openshift-ci-robot openshift-ci-robot added the bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. label Aug 12, 2020
@openshift-ci-robot
Copy link
Copy Markdown

@VenkatRamaraju: This pull request references Bugzilla bug 1856714, which is invalid:

  • expected Bugzilla bug 1856714 to depend on a bug in one of the following states: VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), but no dependents were found

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

Bug 1856714: Adding checks for rescued tasks during reconciliation

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci-robot openshift-ci-robot added the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Aug 12, 2020
@VenkatRamaraju VenkatRamaraju changed the title Bug 1856714: Adding checks for rescued tasks during reconciliation Closes Bug 1856714: Adding checks for rescued tasks during reconciliation Aug 12, 2020
@@ -186,7 +186,7 @@ func (r *AnsibleOperatorReconciler) Reconcile(request reconcile.Request) (reconc
return reconcile.Result{}, err
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It is missing the fragment with the bug fix which will be used to gen the CHANGELOG.
if it was not added in master we also need a PR with it there as well.

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.

Oh ok. I'll make that change here & open a PR on master as well then. Thanks.

Comment on lines +3 to +4
Added checks for rescued tasks during reconciliation
to avoid adding rescued tasks to the list of failures.
Copy link
Copy Markdown
Contributor

@camilamacedo86 camilamacedo86 Aug 12, 2020

Choose a reason for hiding this comment

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

Suggested change
Added checks for rescued tasks during reconciliation
to avoid adding rescued tasks to the list of failures.
Stop to reconcile tasks when the event raised is a rescue for Ansible-based Operators. More info: [Bugzilla 1856714](https://bugzilla.redhat.com/show_bug.cgi?id=1856714)

Added checks for rescued tasks during reconciliation
to avoid adding rescued tasks to the list of failures.

kind: "addition"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
kind: "addition"
kind: "bugfix"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same changes to the master one.

Copy link
Copy Markdown
Contributor

@camilamacedo86 camilamacedo86 left a comment

Choose a reason for hiding this comment

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

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 12, 2020
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Aug 12, 2020
Comment thread internal/util/projutil/project_util.go Outdated
if !goModOn {
return fmt.Errorf(`using go modules requires GO111MODULE="on", "auto", or unset.` +
` More info: https://sdk.operatorframework.io/docs/golang/quickstart/#a-note-on-dependency-management`)
` More info: https://sdk.operatorframework.io/docs/building-operators/golang/quickstart/`)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
` More info: https://sdk.operatorframework.io/docs/building-operators/golang/quickstart/`)
` More info: https://v0-19-x.sdk.operatorframework.io/docs/golang/quickstart/#a-note-on-dependency-management`)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we need to point out for the doc of this branch 0.19. Also, this is the places where has the info ^

Comment thread CHANGELOG.md Outdated

- Add "panic" level for --zap-stacktrace-level (allows "debug", "info", "error", "panic"). ([#3040](https://github.com/operator-framework/operator-sdk/pull/3040))
- The `operator-sdk` binary has a new CLI workflow and project layout for scaffolding Go operators that is aligned with Kubebuilder's CLI and project layout. See the new [Quickstart Guide](https://sdk.operatorframework.io/docs/golang/quickstart/) and the new [CLI reference](https://v0-19-x.sdk.operatorframework.io/docs/new-cli/) for more details. ([#3190](https://github.com/operator-framework/operator-sdk/pull/3190))
- The `operator-sdk` binary has a new CLI workflow and project layout for scaffolding Go operators that is aligned with Kubebuilder's CLI and project layout. See the new [Quickstart Guide](https://sdk.operatorframework.io/docs/building-operators/golang/quickstart/) and the new [CLI reference](https://v0-19-x.sdk.operatorframework.io/docs/new-cli/) for more details. ([#3190](https://github.com/operator-framework/operator-sdk/pull/3190))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Comment thread cmd/operator-sdk/main.go Outdated
depMsg := "Operator SDK has a new CLI and project layout that is aligned with Kubebuilder.\n" +
"See `operator-sdk init -h` and the following doc on how to scaffold a new project:\n" +
"https://sdk.operatorframework.io/docs/golang/quickstart/\n" +
"https://sdk.operatorframework.io/docs/building-operators/golang/quickstart/\n" +
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@jmrodri jmrodri changed the title Closes Bug 1856714: Adding checks for rescued tasks during reconciliation Bug 1856714: Adding checks for rescued tasks during reconciliation Aug 13, 2020
Copy link
Copy Markdown
Member

@jmrodri jmrodri left a comment

Choose a reason for hiding this comment

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

/lgtm

Backported #3650 and things look good.

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 13, 2020
@jmrodri jmrodri merged commit 91c499d into operator-framework:v0.19.x Aug 13, 2020
@openshift-ci-robot
Copy link
Copy Markdown

@VenkatRamaraju: All pull requests linked via external trackers have merged: operator-framework/operator-sdk#3650. Bugzilla bug 1856714 has been moved to the MODIFIED state.

Details

In response to this:

Bug 1856714: Adding checks for rescued tasks during reconciliation

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

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

Labels

bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants