Skip to content

Bug 2011896: pkg/cvo/upgradeable: Include messages for multiple-reason Upgradeable=False #670

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
Oct 8, 2021
Merged

Bug 2011896: pkg/cvo/upgradeable: Include messages for multiple-reason Upgradeable=False #670

openshift-merge-robot merged 1 commit into from
Oct 8, 2021

Conversation

@wking
Copy link
Member

@wking wking commented Oct 7, 2021

Because:

Upgradeable=False

  Reason: MultipleReasons
  Message: Cluster cannot be upgraded between minor versions for multiple reasons: AdminAckRequired,IncompatibleOperatorsInstalled

doesn't include all the useful information needed to resolve those issues. This pivots to using the same approach we use today when aggregating multiple Upgradeable=False ClusterOperators.

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 7, 2021
@wking wking force-pushed the include-messages-in-multi-reason-upgradeable-message branch from 9202c28 to 97adc79 Compare October 7, 2021 16:07
@vrutkovs
Copy link

vrutkovs commented Oct 7, 2021

Looks good

/test unit
as it appears to be racing

@wking wking force-pushed the include-messages-in-multi-reason-upgradeable-message branch from 97adc79 to db432ec Compare October 7, 2021 16:22
@wking wking changed the title pkg/cvo/upgradeable: Include messages for multiple-reason Upgradeable=False Bug 2011896: pkg/cvo/upgradeable: Include messages for multiple-reason Upgradeable=False Oct 7, 2021
@openshift-ci openshift-ci bot added bugzilla/severity-urgent Referenced Bugzilla bug's severity is urgent 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 Oct 7, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 7, 2021

@wking: This pull request references Bugzilla bug 2011896, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

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

Requesting review from QA contact:
/cc @jianlinliu

Details

In response to this:

Bug 2011896: pkg/cvo/upgradeable: Include messages for multiple-reason Upgradeable=False

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 jianlinliu October 7, 2021 16:28
@wking
pkg/cvo/upgradeable: Include messages for multiple-reason Upgradeable…
9a1e58c 
…=False

Because:

  Upgradeable=False

    Reason: MultipleReasons
    Message: Cluster cannot be upgraded between minor versions for multiple reasons: AdminAckRequired,IncompatibleOperatorsInstalled

doesn't include all the useful information needed to resolve those
issues.  This pivots to using the same approach we use today when
aggregating multiple Upgradeable=False ClusterOperators.

Also shift the patch-blocking overrides check in front of the other
minor blocking checks.  It's more important (because it blocks more
targets), and it also will not included a nested sub-list (which might
be confusing, because today I'm not adding indents or any such markers
to make it easy to distinguish levels in a nested list hierarchy).
@wking wking force-pushed the include-messages-in-multi-reason-upgradeable-message branch from db432ec to 9a1e58c Compare October 7, 2021 16:29
@wking
Copy link
Member Author

wking commented Oct 7, 2021

Two unrelated pod-sandbox errors:

/retest

@wking wking mentioned this pull request Oct 7, 2021
Merged
LalatenduMohanty
LalatenduMohanty approved these changes Oct 8, 2021
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 Oct 8, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 8, 2021

[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
Copy link
Member Author

wking commented Oct 8, 2021

Unrelated sandbox issue:

/override ci/prow/e2e-agnostic-upgrade

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 8, 2021

@wking: Overrode contexts on behalf of wking: ci/prow/e2e-agnostic-upgrade

Details

In response to this:

Unrelated sandbox issue:

/override ci/prow/e2e-agnostic-upgrade

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-merge-robot openshift-merge-robot merged commit 755be36 into openshift:master Oct 8, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 8, 2021

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

Bugzilla bug 2011896 has been moved to the MODIFIED state.

Details

In response to this:

Bug 2011896: pkg/cvo/upgradeable: Include messages for multiple-reason Upgradeable=False

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 include-messages-in-multi-reason-upgradeable-message branch October 8, 2021 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jottofar jottofar Awaiting requested review from jottofar

@jianlinliu jianlinliu Awaiting requested review from jianlinliu

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/severity-urgent Referenced Bugzilla bug's severity is urgent 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. 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.

4 participants

@wking @vrutkovs @LalatenduMohanty @openshift-merge-robot