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.

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

Enhance User Errors #1193

Merged
merged 16 commits into from
May 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions frontend/src/components/ClusterMetrics.vue
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,3 @@ export default {
}
}
</script>

<style lang="scss" scoped>
.wrap-text {
white-space: normal;
}
</style>
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,6 @@ export default {
</script>

<style lang="scss" scoped>
.wrap-text {
white-space: normal;
}

.action-select {
align-self: flex-start;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ SPDX-License-Identifier: Apache-2.0
:shoot-item="shootItem"
:popper-key="`${shootNamespace}/${shootName}_lastOp`"
popper-placement="bottom"
showStatusText
show-status-text
>
</shoot-status>
</v-list-item-title>
Expand All @@ -37,7 +37,7 @@ SPDX-License-Identifier: Apache-2.0
<v-list-item-subtitle>Readiness</v-list-item-subtitle>
<v-list-item-title class="d-flex align-center pt-1">
<span v-if="!shootConditions.length">-</span>
<status-tags v-else :shoot-item="shootItem" popper-placement="bottom"></status-tags>
<status-tags v-else :shoot-item="shootItem" popper-placement="bottom" show-status-text></status-tags>
</v-list-item-title>
</v-list-item-content>
</v-list-item>
Expand Down
21 changes: 11 additions & 10 deletions frontend/src/components/ShootMessageDetails.vue
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,15 @@ SPDX-License-Identifier: Apache-2.0
<div v-for="(lastErrorDescription, index) in errorDescriptions" :key="index">
<v-divider v-if="index > 0" class="my-2"></v-divider>
<v-alert
v-for="({description, userError, infraAccountError}) in lastErrorDescription.errorCodeObjects" :key="description"
color="error"
v-for="({description, link, userError, infraAccountError}) in lastErrorDescription.errorCodeObjects" :key="description"
type="error"
:icon="userError ? 'mdi-account-alert' : 'mdi-alert'"
grolu marked this conversation as resolved.
Show resolved Hide resolved
:prominent="!!userError"
>
<h4 v-if="userError">Action required</h4>
<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

<span class="wrap-text">
<span v-if="infraAccountError">There is a problem with your secret
<code>
<router-link v-if="canLinkToSecret"
Expand All @@ -84,7 +86,8 @@ SPDX-License-Identifier: Apache-2.0
</router-link>
<span v-else>{{secretBindingName}}</span>
</code>:</span>
{{description}}
<span>{{description}}</span>
<div v-if="link"><external-link :url="link.url" class="inherit-color font-weight-bold">{{link.text}}</external-link></div>
</span>
</v-alert>
<ansi-text class="error--text" :text="lastErrorDescription.description"></ansi-text>
Expand All @@ -100,11 +103,13 @@ SPDX-License-Identifier: Apache-2.0
import AnsiText from '@/components/AnsiText'
import TimeString from '@/components/TimeString'
import isEmpty from 'lodash/isEmpty'
import ExternalLink from '@/components/ExternalLink.vue'

export default {
components: {
AnsiText,
TimeString
TimeString,
ExternalLink
},
props: {
statusTitle: {
Expand Down Expand Up @@ -147,8 +152,4 @@ export default {
overflow-y: auto;
}

.wrap {
white-space: pre-wrap;
}

</style>
4 changes: 2 additions & 2 deletions frontend/src/components/ShootMessages/ShootMessages.vue
Original file line number Diff line number Diff line change
Expand Up @@ -216,11 +216,11 @@ export default {
return [{
key: 'maintenanceConstraintWarning',
icon: 'mdi-alert-circle-outline',
color: 'warning',
color: 'error',
component: {
name: 'constraint-message',
props: {
constraintCaption: 'Maintenance precondition check failed. It may not be safe to start maintenance for your cluster due to the following reason',
constraintCaption: 'Maintenance precondition check failed. Gardener may be unable to perform required actions during maintenance',
constraintMessage: this.maintenancePreconditionSatisfiedMessage
}
}
Expand Down
13 changes: 6 additions & 7 deletions frontend/src/components/ShootStatus.vue
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,9 @@ SPDX-License-Identifier: Apache-2.0
<span v-if="showStatusText" class="d-flex align-center ml-2">{{statusTitle}}</span>
</div>
<template v-if="showStatusText">
<div v-for="({ description }) in tooltip.errorCodeObjects" :key="description">
<div class="font-weight-bold error--text wrap">{{description}}</div>
<div v-for="({ description, link }) in tooltip.errorCodeObjects" :key="description">
<div class="font-weight-bold error--text wrap-text">{{description}}</div>
<div v-if="link"><external-link :url="link.url" class="font-weight-bold error--text">{{link.text}}</external-link></div>
</div>
</template>
</template>
Expand All @@ -66,6 +67,7 @@ import map from 'lodash/map'
import GPopper from '@/components/GPopper'
import RetryOperation from '@/components/RetryOperation'
import ShootMessageDetails from '@/components/ShootMessageDetails'
import ExternalLink from '@/components/ExternalLink'

import { isUserError, objectsFromErrorCodes, errorCodesFromArray } from '@/utils/errorCodes'
import { shootItem } from '@/mixins/shootItem'
Expand All @@ -74,7 +76,8 @@ export default {
components: {
GPopper,
RetryOperation,
ShootMessageDetails
ShootMessageDetails,
ExternalLink
},
props: {
popperKey: {
Expand Down Expand Up @@ -232,10 +235,6 @@ export default {
font-size: 30px;
}

.wrap {
white-space: normal;
}

::v-deep .v-card {
.v-card__text {
padding: 0px;
Expand Down
3 changes: 0 additions & 3 deletions frontend/src/components/ShootTickets/TicketComment.vue
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,6 @@ export default {
padding: 0;
margin-bottom: 12px;
}
.wrap-text {
white-space: normal;
}

.comment-header {
border-top-left-radius: 2px;
Expand Down
46 changes: 30 additions & 16 deletions frontend/src/components/StatusTags.vue
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,24 @@ SPDX-License-Identifier: Apache-2.0
-->

<template>
<div class="d-flex flex-nowrap justify-center">
<status-tag
v-for="condition in conditions"
:condition="condition"
:popper-key="condition.id"
:key="condition.id"
:popper-placement="popperPlacement"
:secret-binding-name="shootSecretBindingName"
:namespace="shootNamespace">
<div>
<div class="d-flex flex-nowrap justify-start">
<status-tag
v-for="condition in conditions"
:condition="condition"
:popper-key="condition.id"
:key="condition.id"
:popper-placement="popperPlacement"
:secret-binding-name="shootSecretBindingName"
:namespace="shootNamespace">
</status-tag>
</div>
<template v-if="showStatusText">
<div v-for="({ description, link }) in errorCodeObjects" :key="description" class="mt-1">
<div class="font-weight-bold error--text wrap-text">{{description}}</div>
<div v-if="link"><external-link :url="link.url" class="font-weight-bold error--text">{{link.text}}</external-link></div>
</div>
</template>
</div>
</template>

Expand All @@ -30,18 +38,24 @@ import snakeCase from 'lodash/snakeCase'
import includes from 'lodash/includes'
import upperFirst from 'lodash/upperFirst'
import StatusTag from '@/components/StatusTag'
import isEmpty from 'lodash/isEmpty'
import ExternalLink from '@/components/ExternalLink'
import filter from 'lodash/filter'
import sortBy from 'lodash/sortBy'
import { shootItem } from '@/mixins/shootItem'
import { objectsFromErrorCodes, errorCodesFromArray } from '@/utils/errorCodes'

export default {
components: {
StatusTag
StatusTag,
ExternalLink
},
props: {
popperPlacement: {
type: String
},
showStatusText: {
type: Boolean,
default: false
}
},
mixins: [shootItem],
Expand All @@ -50,11 +64,7 @@ export default {
'conditionCache'
]),
conditions () {
if (isEmpty(this.shootConditions)) {
return []
}

const shootConditions = filter(this.shootConditions, condition => !!condition.lastTransitionTime)
const shootConditions = filter(this.shootReadiness, condition => !!condition.lastTransitionTime)
const conditions = map(shootConditions, condition => {
const { lastTransitionTime, message, status, type, codes } = condition
const id = type
Expand All @@ -64,6 +74,10 @@ export default {
})

return sortBy(conditions, 'shortName')
},
errorCodeObjects () {
const allErrorCodes = errorCodesFromArray(this.conditions)
return objectsFromErrorCodes(allErrorCodes)
}
},
methods: {
Expand Down
4 changes: 0 additions & 4 deletions frontend/src/components/TerminalShortcut.vue
Original file line number Diff line number Diff line change
Expand Up @@ -168,10 +168,6 @@ export default {

<style lang="scss" scoped>

.wrap-text {
white-space: normal;
}

::v-deep .popper {
text-align: initial;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,9 +157,3 @@ export default {
}
}
</script>

<style lang="scss" scoped>
.wrap-text {
white-space: normal;
}
</style>
18 changes: 15 additions & 3 deletions frontend/src/mixins/shootItem.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import flatMap from 'lodash/flatMap'
import cloneDeep from 'lodash/cloneDeep'
import find from 'lodash/find'
import some from 'lodash/some'
import filter from 'lodash/filter'
import { mapGetters } from 'vuex'

import {
Expand Down Expand Up @@ -185,6 +186,18 @@ export const shootItem = {
shootConditions () {
return get(this.shootItem, 'status.conditions', [])
},
shootConstraints () {
return get(this.shootItem, 'status.constraints', [])
},
shootReadiness () {
const shootConstraintsWithErrorCode = filter(this.shootConstraints, constraint => {
return constraint.codes && constraint.codes.length
})
return [
...this.shootConditions,
...shootConstraintsWithErrorCode
]
},
shootObservedGeneration () {
return get(this.shootItem, 'status.observedGeneration')
},
Expand All @@ -204,8 +217,7 @@ export const shootItem = {
return this.selectedAccessRestrictionsForShootByCloudProfileNameAndRegion({ shootResource: this.shootItem, cloudProfileName: this.shootCloudProfileName, region: this.shootRegion })
},
hibernationPossibleConstraint () {
const constraints = get(this.shootItem, 'status.constraints')
return find(constraints, ['type', 'HibernationPossible'])
return find(this.shootConstraints, ['type', 'HibernationPossible'])
},
isHibernationPossible () {
const status = get(this.hibernationPossibleConstraint, 'status', 'True')
Expand All @@ -215,7 +227,7 @@ export const shootItem = {
return get(this.hibernationPossibleConstraint, 'message', 'Hibernation currently not possible')
},
maintenancePreconditionSatisfiedConstraint () {
const constraints = get(this.shootItem, 'status.constraints')
const constraints = this.shootConstraints
return find(constraints, ['type', 'MaintenancePreconditionsSatisfied'])
},
isMaintenancePreconditionSatisfied () {
Expand Down
8 changes: 8 additions & 0 deletions frontend/src/sass/main.scss
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@
white-space: nowrap;
}

.wrap-text {
white-space: normal;
}

.alert-banner-message {
p {
display: inline !important;
Expand Down Expand Up @@ -80,3 +84,7 @@
.theme--dark .alternate-row-background > div:nth-of-type(even) {
background-color: rgba(0, 0, 0, .2);
}

.inherit-color {
color: inherit !important;
}
10 changes: 10 additions & 0 deletions frontend/src/store/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,16 @@ const state = {
displayName: 'System Components',
shortName: 'SC',
description: 'Indicates whether all system components in the kube-system namespace are up and running. Gardener manages these system components and should automatically take care that the components become healthy again.'
},
MaintenancePreconditionsSatisfied: {
displayName: 'Maintenance Preconditions Satisfied',
shortName: 'M',
description: 'Indicates whether Gardener is able to perform required actions during maintenance. If you do not resolve this issue your cluster will eventually turn into an error state.'
},
HibernationPossible: {
displayName: 'Hibernation Preconditions Satisfied',
shortName: 'H',
description: 'Indicates whether Gardener is able to hibernate this cluster. If you do not resolve this issue your hibernation schedule may not have any effect.'
}
},
darkTheme: false,
Expand Down
25 changes: 13 additions & 12 deletions frontend/src/utils/errorCodes.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,32 +6,23 @@

'use strict'

import isEmpty from 'lodash/isEmpty'
import get from 'lodash/get'
import compact from 'lodash/compact'
import uniq from 'lodash/uniq'
import flatMap from 'lodash/flatMap'
import map from 'lodash/map'
import find from 'lodash/find'
import some from 'lodash/some'

export function errorCodesFromArray (array) {
return uniq(compact(flatMap(array, 'codes')))
}

export function isUserError (errorCodesArray) {
if (isEmpty(errorCodesArray)) {
return false
}

return !!find(objectsFromErrorCodes(errorCodesArray), 'userError')
return some(objectsFromErrorCodes(errorCodesArray), 'userError')
}

export function isTemporaryError (errorCodesArray) {
if (isEmpty(errorCodesArray)) {
return false
}

return !!find(objectsFromErrorCodes(errorCodesArray), 'temporaryError')
return some(objectsFromErrorCodes(errorCodesArray), 'temporaryError')
}

export function objectsFromErrorCodes (errorCodesArray) {
Expand Down Expand Up @@ -107,5 +98,15 @@ const errorCodes = {
description: 'Error occurred due to dependent objects on the infrastructure level. The operation will be retried automatically.',
temporaryError: true,
userError: false
},
ERR_PROBLEMATIC_WEBHOOK: {
shortDescription: 'Misconfigured Webhook',
description: 'A misconfigured webhook prevents Gardener from performing operations. Please resolve this as this can lead to required actions not beeing performed which will eventually turn the cluster into an error state.',
link: {
text: 'Best practises',
url: 'https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#best-practices-and-warnings'
},
temporaryError: false,
userError: true
}
}
Loading