Skip to content
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

Enhance User Errors #1193

Merged
merged 16 commits into from
May 10, 2022
Merged

Enhance User Errors #1193

merged 16 commits into from
May 10, 2022

Conversation

grolu
Copy link
Contributor

@grolu grolu commented Apr 12, 2022

What this PR does / why we need it:
This PR improves the visibility of errors caused by misconfigurations and other issues that can be solved by the user.
It also adds several additional hints in case of MaintenacePrecondition failure (misconfigured webhook) - which will hopefully make users aware that there are measures to take on their end.

  • Failed MaintenacePrecondition constraint is now rendered as error (not warning)
  • Constraints that have an error code (see also Add ERROR_CODE to failed MaintenancePreconditionsSatisfied constraint gardener#5778) will now be added to the readiness chips with an user error icon and additional information on how to resolve the issue
  • Improved User error text to make users aware that they need to take action and the issue can not be solved by the gardener operators

Screen Shot 2022-04-12 at 13 35 26

Screen Shot 2022-04-12 at 13 35 37

Screen Shot 2022-04-12 at 13 35 55

Which issue(s) this PR fixes:
Fixes #1144

Special notes for your reviewer:

Release note:

Improved visibility of user errors

@gardener-robot gardener-robot added needs/review Needs review size/m Size of pull request is medium (see gardener-robot robot/bots/size.py) labels Apr 12, 2022
@grolu
Copy link
Contributor Author

grolu commented Apr 12, 2022

/hold
until gardener/gardener#5778 is merged

@gardener-robot gardener-robot added the reviewed/do-not-merge Has no approval for merging as it may break things, be of poor quality or have (ext.) dependencies label Apr 12, 2022
@gardener-robot-ci-1 gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Apr 12, 2022
@gardener-robot-ci-3 gardener-robot-ci-3 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Apr 12, 2022
@gardener-robot-ci-1 gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label May 4, 2022
@grolu
Copy link
Contributor Author

grolu commented May 4, 2022

/unhold as gardener/gardener#5855 has been merged

@gardener-robot gardener-robot removed the reviewed/do-not-merge Has no approval for merging as it may break things, be of poor quality or have (ext.) dependencies label May 4, 2022
@gardener-robot-ci-1 gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label May 4, 2022
petersutter
petersutter reviewed May 5, 2022
View reviewed changes
frontend/src/store/index.js Outdated Show resolved Hide resolved
frontend/src/utils/errorCodes.js Outdated Show resolved Hide resolved
@gardener-robot-ci-1 gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label May 6, 2022
@gardener-robot-ci-3 gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label May 6, 2022
@gardener-robot gardener-robot added the needs/rebase Needs git rebase label May 9, 2022
@gardener-robot
Copy link

@grolu You need rebase this pull request with latest master branch. Please check.

@gardener-robot-ci-1 gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels May 9, 2022
@grolu
Merge branch 'master' into enh/better_constraint_warning
1a228ae 
# Conflicts:
#	frontend/src/components/StatusTags.vue
@gardener-robot-ci-1 gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels May 9, 2022
petersutter
petersutter reviewed May 9, 2022
View reviewed changes
frontend/src/components/ShootMessageDetails.vue
<span class="wrap">
<h3 v-if="userError">Your Action is required</h3>
<h4 class="wrap-text font-weight-bold">This error is flagged as user error which indicates that no Gardener operator action is required.
Please read the error message carefully and take action.</h4>
Copy link
Member

Choose a reason for hiding this comment

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

Screenshot 2022-05-09 at 17 26 39

Please read the error message ..
Please double-check the error message ..

I understand that the second message is coming from gardener, but it's duplicative in this case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I understand your thoughts. However, for the other user errors there are completely different error messages and it is not duplicative. So I'm not sure what we should do here...

Copy link
Member

Choose a reason for hiding this comment

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

I guess we should update the message in gardener then

@gardener-robot-ci-3 gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label May 10, 2022
@gardener-robot-ci-1 gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label May 10, 2022
@gardener-robot-ci-3 gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label May 10, 2022
@gardener-robot-ci-2 gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label May 10, 2022
holgerkoser
holgerkoser approved these changes May 10, 2022
View reviewed changes
Copy link
Member

@holgerkoser holgerkoser left a comment

Choose a reason for hiding this comment

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

/lgtm

@gardener-robot gardener-robot added reviewed/lgtm Has approval for merging and removed needs/rebase Needs git rebase needs/review Needs review labels May 10, 2022
@gardener-robot-ci-3 gardener-robot-ci-3 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels May 10, 2022
@grolu grolu merged commit c473258 into master May 10, 2022
@grolu grolu deleted the enh/better_constraint_warning branch May 10, 2022 15:10
@gardener-robot gardener-robot added the status/closed Issue is closed (either delivered or triaged) label May 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@petersutter petersutter petersutter left review comments

@holgerkoser holgerkoser holgerkoser approved these changes

Assignees
No one assigned
Labels
needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) reviewed/lgtm Has approval for merging size/m Size of pull request is medium (see gardener-robot robot/bots/size.py) status/closed Issue is closed (either delivered or triaged)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Display failed shoot constraints
7 participants
@grolu @gardener-robot @holgerkoser @petersutter @gardener-robot-ci-1 @gardener-robot-ci-2 @gardener-robot-ci-3