Skip to content

OCPBUGS-4986: pkg/payload/precondition: Do not claim warnings would have blocked #876

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
openshift-merge-robot merged 1 commit into from
Dec 17, 2022
Merged

OCPBUGS-4986: pkg/payload/precondition: Do not claim warnings would have blocked #876

openshift-merge-robot merged 1 commit into from
Dec 17, 2022

Conversation

@wking
Copy link
Member

@wking wking commented Dec 16, 2022

Avoid errors like:

$ oc get -o json clusterversion version | jq -r '.status.history[0].acceptedRisks'
Forced through blocking failures: Precondition "ClusterVersionRecommendedUpdate" failed because of "UnknownUpdate": RetrievedUpdates=True (), so the update from 4.13.0-0.okd-2022-12-11-064650 to 4.13.0-0.okd-2022-12-13-052859 is probably neither recommended nor supported.

Instead, tweak the logic from 481bcde (#856), and only append the Forced through blocking failures: prefix when the forcing was required.

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 16, 2022
@wking wking force-pushed the do-not-claim-forced-warnings-would-have-blocked branch from 799f014 to e86a84f Compare December 16, 2022 04:14
@wking
pkg/payload/precondition: Do not claim warnings would have blocked
24c0b93 
Avoid errors like:

  $ oc get -o json clusterversion version | jq -r '.status.history[0].acceptedRisks'
  Forced through blocking failures: Precondition "ClusterVersionRecommendedUpdate" failed because of "UnknownUpdate": RetrievedUpdates=True (), so the update from 4.13.0-0.okd-2022-12-11-064650 to 4.13.0-0.okd-2022-12-13-052859 is probably neither recommended nor supported.

Instead, tweak the logic from 481bcde (OCPBUGS-2727: Do not fail
precondition check for UnknownUpdate, 2022-10-21, openshift#856), and only
append the "Forced through blocking failures:" prefix when the forcing
was required.

Also fail-closed and treat non *Error errors as blocking too, where
previously only !pferr.NonBlockingWarning were blocking.
@wking wking force-pushed the do-not-claim-forced-warnings-would-have-blocked branch from e86a84f to 24c0b93 Compare December 16, 2022 04:25
@bshephar
Copy link

bshephar commented Dec 16, 2022

Nice clean solution. fwiw, looks good to me.

LalatenduMohanty
LalatenduMohanty approved these changes Dec 16, 2022
View reviewed changes
Copy link
Member

@LalatenduMohanty LalatenduMohanty 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 openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 16, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Dec 16, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: LalatenduMohanty, wking

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [LalatenduMohanty,wking]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@wking wking changed the title pkg/payload/precondition: Do not claim warnings would have blocked OCPBUGS-4986: pkg/payload/precondition: Do not claim warnings would have blocked Dec 16, 2022
@openshift-ci-robot openshift-ci-robot added jira/severity-moderate Referenced Jira bug's severity is moderate for the branch this PR is targeting. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels Dec 16, 2022
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Dec 16, 2022

@wking: This pull request references Jira Issue OCPBUGS-4986, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.13.0) matches configured target version for branch (4.13.0)
  • bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @jiajliu

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Avoid errors like:

$ oc get -o json clusterversion version | jq -r '.status.history[0].acceptedRisks'
Forced through blocking failures: Precondition "ClusterVersionRecommendedUpdate" failed because of "UnknownUpdate": RetrievedUpdates=True (), so the update from 4.13.0-0.okd-2022-12-11-064650 to 4.13.0-0.okd-2022-12-13-052859 is probably neither recommended nor supported.

Instead, tweak the logic from 481bcde (#856), and only append the Forced through blocking failures: prefix when the forcing was required.

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 openshift-ci bot requested a review from jiajliu December 16, 2022 17:57
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Dec 16, 2022

/retest-required

Remaining retests: 0 against base HEAD 9c23665 and 2 for PR HEAD 24c0b93 in total

@wking
Copy link
Member Author

wking commented Dec 17, 2022

Install failed.

/retest

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Dec 17, 2022

/retest-required

Remaining retests: 0 against base HEAD 75c5b85 and 1 for PR HEAD 24c0b93 in total

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Dec 17, 2022

@wking: all tests passed!

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

@openshift-merge-robot openshift-merge-robot merged commit 4f9883a into openshift:master Dec 17, 2022
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Dec 17, 2022

@wking: All pull requests linked via external trackers have merged:

Jira Issue OCPBUGS-4986 has been moved to the MODIFIED state.

Details

In response to this:

Avoid errors like:

$ oc get -o json clusterversion version | jq -r '.status.history[0].acceptedRisks'
Forced through blocking failures: Precondition "ClusterVersionRecommendedUpdate" failed because of "UnknownUpdate": RetrievedUpdates=True (), so the update from 4.13.0-0.okd-2022-12-11-064650 to 4.13.0-0.okd-2022-12-13-052859 is probably neither recommended nor supported.

Instead, tweak the logic from 481bcde (#856), and only append the Forced through blocking failures: prefix when the forcing was required.

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.

@wking wking deleted the do-not-claim-forced-warnings-would-have-blocked branch December 18, 2022 16:23
@wking
Copy link
Member Author

wking commented Dec 21, 2022

/cherrypick release-4.12

@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Dec 21, 2022
Merged
@openshift-cherrypick-robot
Copy link

openshift-cherrypick-robot commented Dec 21, 2022

@wking: new pull request created: #878

Details

In response to this:

/cherrypick release-4.12

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

Reviewers

@DavidHurta DavidHurta Awaiting requested review from DavidHurta

@petr-muller petr-muller Awaiting requested review from petr-muller

@jiajliu jiajliu Awaiting requested review from jiajliu

1 more reviewer

@LalatenduMohanty LalatenduMohanty LalatenduMohanty approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@LalatenduMohanty LalatenduMohanty

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. jira/severity-moderate Referenced Jira bug's severity is moderate for the branch this PR is targeting. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@wking @bshephar @openshift-ci-robot @openshift-cherrypick-robot @LalatenduMohanty @openshift-merge-robot