Evaluate MaintenancePreconditionsSatisfied constraint #918

grolu · 2020-12-18T16:12:51Z

What this PR does / why we need it:
We now show warnings for the Maintenance and Hibernation Constraint Checks.

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

Special notes for your reviewer:

Release note:

The `MaintenancePreconditionsSatisfied` constraint is now evaluated and the Dashboard shows a warning on cluster list and cluster details pages, if it is not safe for a cluster to trigger maintenance

In addition to the warning on the hibernation schedule configuration popup, the `HibernationPossible` constraint is now evaluated and the Dashboard shows a warning on cluster list and cluster details pages if the hibernation schedules might not be executed

petersutter · 2020-12-18T16:20:41Z

Should we disable the action or instead show a warning?
cc @rfranzke

rfranzke · 2020-12-18T16:55:50Z

Only show the warning

grolu · 2020-12-21T13:50:57Z

@rfranzke does Gardener still trigger the automatic maintenance in case the precondition is not satisfied?

grolu · 2020-12-22T07:15:05Z

According to gardener/gardener#3122 (comment) it seems that maintenance is performed anyway, so we need to think of a way to show this information more prominent, not only on the dialogs.

… and hibernation constraint issues

holgerkoser · 2021-01-05T09:26:33Z

frontend/src/components/ShootMaintenance/MaintenanceConstraintWarning.vue

+-->
+
+<template>
+  <div v-if="!isMaintenancePreconditionSatisfied" :class="{ 'd-flex' : icon }">


The v-if="!isMaintenancePreconditionSatisfied" should be outside of the component. If the visibility of the component is controlled via a property it should follow the pattern of the toggleable mixin.

Do we really need the flexbox container in case of an icon?

flex box does vertical align the icon so that it matches the layout of other icons / texts in the same line

holgerkoser · 2021-01-05T09:27:08Z

frontend/src/components/ShootHibernation/HibernationConstraintWarning.vue

+-->
+
+<template>
+  <div v-if="!isHibernationPossible && hasHibernationSchedules" :class="{ 'd-flex' : icon }">


The v-if="!isHibernationPossible && hasHibernationSchedules" should be outside of the component. If the visibility of the component is controlled via a property it should follow the pattern of the toggleable mixin.

Do we really need the flexbox container in case of an icon?

holgerkoser · 2021-01-05T09:30:32Z

frontend/src/components/ShootMaintenance/MaintenanceConstraintWarning.vue

+
+<script>
+export default {
+  name: 'maintenanceConstraintWarning',


Suggested change

name: 'maintenanceConstraintWarning',

name: 'MaintenanceConstraintWarning',

Component Names are kebab-case or PascalCase https://vuejs.org/v2/guide/components-registration.html#Name-Casing

I will fix this with a separate PR for all cases where we use camel case instead of kebab case

holgerkoser · 2021-01-05T09:31:39Z

frontend/src/components/ShootHibernation/HibernationConstraintWarning.vue

+
+<script>
+export default {
+  name: 'hibernationConstraintWarning',


Suggested change

name: 'hibernationConstraintWarning',

name: 'HibernationConstraintWarning',

Component Names are kebab-case or PascalCase https://vuejs.org/v2/guide/components-registration.html#Name-Casing

I will fix this with a separate PR for all cases where we use camel case instead of kebab case

frontend/src/components/ShootDetails/ShootLifecycleCard.vue

holgerkoser · 2021-01-08T08:41:44Z

frontend/src/components/ConstraintWarning.vue

+    value: {
+      type: Boolean
+    },
+    constraintType: {


The name of the component ist ConstraintWarning. Why do we repeat the word constraint but warning in the property names. I would use type and message. Or better use a default slot for the message.

frontend/src/components/ConstraintWarning.vue

frontend/src/components/ShootDetails/ShootLifecycleCard.vue

holgerkoser

/lgtm

petersutter

/lgtm

# Conflicts: # frontend/src/components/ShootListRow.vue

Evaluate MaintenancePreconditionsSatisfied constraint

4720575

grolu requested review from holgerkoser and petersutter as code owners December 18, 2020 16:12

gardener-robot added needs/review Needs review size/xs Size of pull request is tiny (see gardener-robot robot/bots/size.py) labels Dec 18, 2020

gardener-robot-ci-2 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 Dec 18, 2020

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 Dec 18, 2020

holgerkoser approved these changes Dec 18, 2020

View reviewed changes

gardener-robot added reviewed/lgtm Has approval for merging and removed needs/review Needs review labels Dec 18, 2020

Show warning icon on cluster details and cluster list for maintenance…

92618c1

… and hibernation constraint issues

gardener-robot added size/m Size of pull request is medium (see gardener-robot robot/bots/size.py) and removed size/xs Size of pull request is tiny (see gardener-robot robot/bots/size.py) labels Dec 22, 2020

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 Dec 22, 2020

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 Dec 22, 2020

grolu requested a review from holgerkoser December 22, 2020 17:29

grolu marked this pull request as draft January 4, 2021 09:47

holgerkoser removed the reviewed/lgtm Has approval for merging label Jan 4, 2021

grolu marked this pull request as ready for review January 4, 2021 11:18

holgerkoser reviewed Jan 5, 2021

View reviewed changes

frontend/src/components/ShootDetails/ShootLifecycleCard.vue Outdated Show resolved Hide resolved

holgerkoser reviewed Jan 5, 2021

View reviewed changes

frontend/src/components/ShootDetails/ShootLifecycleCard.vue Outdated Show resolved Hide resolved

merged constraint warning components

058608a

gardener-robot-ci-2 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 Jan 7, 2021

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 Jan 7, 2021

holgerkoser reviewed Jan 8, 2021

View reviewed changes

frontend/src/components/ConstraintWarning.vue Outdated Show resolved Hide resolved

holgerkoser reviewed Jan 8, 2021

View reviewed changes

frontend/src/components/ShootDetails/ShootLifecycleCard.vue Outdated Show resolved Hide resolved

Merge branch 'master' into enh/MaintenancePreconditionsSatisfied

c60c16d

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 Jan 11, 2021

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 Jan 11, 2021

PR Feedback

1edba2d

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 Jan 11, 2021

holgerkoser approved these changes Jan 12, 2021

View reviewed changes

gardener-robot added the reviewed/lgtm Has approval for merging label Jan 12, 2021

petersutter approved these changes Jan 12, 2021

View reviewed changes

Merge branch 'master' into enh/MaintenancePreconditionsSatisfied

610390d

# Conflicts: # frontend/src/components/ShootListRow.vue

gardener-robot added needs/rebase Needs git rebase and removed reviewed/lgtm Has approval for merging labels Jan 19, 2021

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 Jan 19, 2021

grolu merged commit e2e926e into master Jan 19, 2021

petersutter deleted the enh/MaintenancePreconditionsSatisfied branch January 28, 2021 12:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Evaluate MaintenancePreconditionsSatisfied constraint #918

Evaluate MaintenancePreconditionsSatisfied constraint #918

grolu commented Dec 18, 2020 •

edited

petersutter commented Dec 18, 2020

rfranzke commented Dec 18, 2020

grolu commented Dec 21, 2020

grolu commented Dec 22, 2020

holgerkoser Jan 5, 2021 •

edited

holgerkoser Jan 5, 2021

grolu Jan 11, 2021

holgerkoser Jan 5, 2021 •

edited

holgerkoser Jan 5, 2021

grolu Jan 11, 2021

holgerkoser Jan 5, 2021

holgerkoser Jan 5, 2021

grolu Jan 11, 2021

holgerkoser Jan 5, 2021

holgerkoser Jan 5, 2021

grolu Jan 11, 2021

holgerkoser Jan 8, 2021

holgerkoser left a comment

petersutter left a comment

	name: 'maintenanceConstraintWarning',
	name: 'MaintenanceConstraintWarning',

	name: 'hibernationConstraintWarning',
	name: 'HibernationConstraintWarning',

Evaluate MaintenancePreconditionsSatisfied constraint #918

Evaluate MaintenancePreconditionsSatisfied constraint #918

Conversation

grolu commented Dec 18, 2020 • edited

petersutter commented Dec 18, 2020

rfranzke commented Dec 18, 2020

grolu commented Dec 21, 2020

grolu commented Dec 22, 2020

holgerkoser Jan 5, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

holgerkoser Jan 5, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

holgerkoser left a comment

Choose a reason for hiding this comment

petersutter left a comment

Choose a reason for hiding this comment

grolu commented Dec 18, 2020 •

edited

holgerkoser Jan 5, 2021 •

edited

holgerkoser Jan 5, 2021 •

edited