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

Replication Primary Dashboard: handle errors #8845

Merged
merged 24 commits into from
Apr 28, 2020
Merged

Replication Primary Dashboard: handle errors #8845

Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
74a7fbe
use h3 instead of code elements
andaley Apr 20, 2020
b23c1b4
use correct property names for StateDisplay
andaley Apr 21, 2020
4553a30
WIP
andaley Apr 21, 2020
0e39b9c
remove todo
andaley Apr 21, 2020
3ddb24b
move cluster states into a map; make status menu icon match cluster s…
andaley Apr 22, 2020
b64d2e9
show error in state card using the same state map in the cluster model
andaley Apr 22, 2020
83c50ad
whitespace
andaley Apr 22, 2020
ae02971
move cluster-states into a helper and update usage
andaley Apr 22, 2020
92fb513
use circle success icon for stream-wals because that is the ideal state
andaley Apr 22, 2020
b25ce17
more refactoring of cluster state display
andaley Apr 22, 2020
927c508
use new cluster-states helper
andaley Apr 22, 2020
448b1f0
whitespace
andaley Apr 22, 2020
502064c
use clusterStates helper in replication secondary card
andaley Apr 23, 2020
def71a1
remove extra import
andaley Apr 24, 2020
f672ae3
add default values for when state isn't recognized
andaley Apr 24, 2020
246fd51
make sure that state exists before getting state details from cluster…
andaley Apr 24, 2020
acb3d06
be more strict when state cannot be found
andaley Apr 24, 2020
d8ddb95
use brace expansion to fix linting error
andaley Apr 24, 2020
d5b07f3
add tests for error states
andaley Apr 24, 2020
83e3003
fix text wrapping issue on secondary cards; make titles match mocks
andaley Apr 24, 2020
11842b1
Merge branch 'ui/replication-status-discoverability' of https://githu…
andaley Apr 24, 2020
3e496da
use unknown if metric isn't foudn
andaley Apr 27, 2020
98f7527
remove extra border on selectable card when there is an error
andaley Apr 27, 2020
d20029a
use outline square in status menu for error
andaley Apr 27, 2020
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
43 changes: 3 additions & 40 deletions ui/app/models/cluster.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,46 +49,9 @@ export default DS.Model.extend({

anyReplicationEnabled: or('{dr,performance}.replicationEnabled'),

stateDisplay(state) {
if (!state) {
return null;
}
const defaultDisp = 'Synced';
const displays = {
'stream-wals': 'Streaming',
'merkle-diff': 'Determining sync status',
'merkle-sync': 'Syncing',
};

return displays[state] || defaultDisp;
},

drStateDisplay: computed('dr.state', function() {
return this.stateDisplay(this.get('dr.state'));
}),

performanceStateDisplay: computed('performance.state', function() {
return this.stateDisplay(this.get('performance.state'));
}),

stateGlyph(state) {
const glyph = 'check-circle-outline';

const glyphs = {
'stream-wals': 'check-circle-outline',
'merkle-diff': 'android-sync',
'merkle-sync': 'android-sync',
};

return glyphs[state] || glyph;
},

drStateGlyph: computed('dr.state', function() {
return this.stateGlyph(this.get('dr.state'));
}),

performanceStateGlyph: computed('performance.state', function() {
return this.stateGlyph(this.get('performance.state'));
modeState: computed('dr.{mode,state}', 'performance.{mode,state}', 'replicationMode', function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Since only replicationMode is accessed for the value, I don't think dr{...} and performance{...} are necessary. Also I've never seen that thing.{foo,bar} pattern before 🤯

Copy link
Contributor Author

Choose a reason for hiding this comment

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

glad you brought this up! i think we still need all of these properties here because modeState is dependent on all of their values. that is, when the dr.state or performance.state changes, we want to update our cache so that the next time modeState is accessed in the template, the new computed value is shown. i know it's wonky since we're only using replicationMode on line 54, but behind the scenes we want the drOrPerf.mode to affect this property. i found this blurb on the ember guides that talks about this process:

The first time you access the fullName property, the function will be called and the results will be cached. Subsequent access of fullName will read from the cache without calling the function. Changing any of the dependent properties causes the cache to invalidate, so that the computed function runs again on the next access.

that being said i am not 100% positive about this, so if you think i've misunderstood definitely let me know!

regarding the thing.{stuff, yay} syntax, my linter showed me that! there's more about using object properties as dependent keys and using that syntax here:
https://guides.emberjs.com/v3.8.0/object-model/computed-properties-and-aggregate-data/

const mode = this.replicationMode;
return this.get(`${mode}.state`);
}),

dr: fragment('replication-attributes'),
Expand Down
13 changes: 6 additions & 7 deletions ui/app/styles/components/replication-dashboard.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,16 @@
&:hover {
box-shadow: 0 0 0 1px rgba($grey-dark, 0.3);
}

.title-number {
font-size: $size-3;
align-self: end;
}
}

.helper-text {
font-weight: $font-weight-normal;
}

.title.is-6 {
margin-bottom: $spacing-xs;
}

.selectable-card-container {
display: grid;
grid-template-columns: repeat(auto-fit, minmax(400px, 1fr));
Expand All @@ -24,9 +23,10 @@

.card-container {
display: grid;
grid-gap: 2rem;
grid-template-columns: 1fr 1fr;
grid-template-rows: 1fr 1fr 0.8fr;
padding: $spacing-l 0 18px $spacing-l;
padding: $spacing-l;
line-height: 1.5;

height: 230px;
Expand All @@ -38,7 +38,6 @@

.grid-item-title {
grid-column: 1 / span 2;

display: flex;
}
.grid-item-left {
Expand Down
6 changes: 3 additions & 3 deletions ui/app/templates/vault/cluster/replication-dr-promote/details.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<section class="section">
<div class="container is-widescreen">
<ReplicationPage @model={{model}} as |Page|>
<Page.header
<Page.header
@showTabs={{false}}
/>
{{#if Page.isDisabled}}
Expand Down Expand Up @@ -29,7 +29,7 @@
</EmptyState>
{{else}}
<Page.toggle />
<Page.dashboard
<Page.dashboard
{{!-- passing in component to render so that the yielded components are flexible based on the dashboard --}}
@componentToRender='replication-secondary-card' as |Dashboard|>
<Dashboard.card
Expand All @@ -39,7 +39,7 @@
@title="Write-Ahead Logs (WALs)"
/>
{{#if Dashboard.isSyncing }}
<AlertBanner
<AlertBanner
@title="Syncing in progress"
@type="info"
@secondIconType="loading"
Expand Down
15 changes: 10 additions & 5 deletions ui/lib/core/addon/components/replication-dashboard.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,20 @@
import Component from '@ember/component';
import { computed } from '@ember/object';
import { clusterStates } from 'core/helpers/cluster-states';
import layout from '../templates/components/replication-dashboard';

const MERKLE_STATES = { sync: 'merkle-sync', diff: 'merkle-diff' };

export default Component.extend({
layout,
data: null,
isSyncing: computed('data', function() {
if (this.dr.state === MERKLE_STATES.sync || this.dr.state === MERKLE_STATES.diff) {
return true;
dr: computed('data', function() {
let dr = this.data.dr;
if (!dr) {
return false;
}
return dr;
}),
isSyncing: computed('dr', function() {
const { state } = this.dr;
return state && clusterStates([state]).isSyncing;
}),
});
3 changes: 3 additions & 0 deletions ui/lib/core/addon/components/replication-mode-summary.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,4 +48,7 @@ export default Component.extend({
clusterIdDisplay: replicationAttr('clusterIdDisplay'),
mode: null,
cluster: null,
modeState: computed('cluster', function() {
return this.cluster.modeState;
}),
});
31 changes: 13 additions & 18 deletions ui/lib/core/addon/components/replication-secondary-card.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,7 @@
import Component from '@ember/component';
import { computed } from '@ember/object';
import layout from '../templates/components/replication-secondary-card';

const STATES = {
streamWals: 'stream-wals',
idle: 'idle',
transientFailure: 'transient_failure',
shutdown: 'shutdown',
};
import { clusterStates } from 'core/helpers/cluster-states';

export default Component.extend({
layout,
Expand All @@ -28,19 +22,20 @@ export default Component.extend({
return Math.abs(this.get('lastWAL') - this.get('lastRemoteWAL'));
}),
inSyncState: computed('state', function() {
if (this.state === STATES.streamWals) {
return true;
}
// if our definition of what is considered 'synced' changes,
// we should use the clusterStates helper instead
return this.state === 'stream-wals';
}),

hasErrorClass: computed('data', 'title', 'state', 'connection', function() {
if (this.title === 'States') {
if (
this.state === STATES.idle ||
this.connection === STATES.transientFailure ||
this.connection === STATES.shutdown
) {
return true;
}
const { title, state, connection } = this;

// only show errors on the state card
if (title === 'States') {
const currentClusterisOk = clusterStates([state]).isOk;
const primaryIsOk = clusterStates([connection]).isOk;
return !(currentClusterisOk && primaryIsOk);
}
return false;
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like you're missing the red border around the selectable card when they're in an error state.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ooh good catch, thanks!

}),
});
1 change: 1 addition & 0 deletions ui/lib/core/addon/components/toggle.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export default Component.extend({
tagName: '',
checked: false,
disabled: false,
name: '',
andaley marked this conversation as resolved.
Show resolved Hide resolved
size: 'normal',
status: 'normal',
safeId: computed('name', function() {
Expand Down
69 changes: 69 additions & 0 deletions ui/lib/core/addon/helpers/cluster-states.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
import { helper as buildHelper } from '@ember/component/helper';

// A hash of cluster states to ensure that the status menu and replication dashboards
// display states and glyphs consistently
// this includes states for the primary vault cluster and the connection_state

export const CLUSTER_STATES = {
running: {
glyph: 'check-circle-outline',
isOk: true,
isSyncing: false,
},
ready: {
glyph: 'check-circle-outline',
isOk: true,
isSyncing: false,
},
'stream-wals': {
glyph: 'check-circle-outline',
display: 'Streaming',
isOk: true,
isSyncing: false,
},
'merkle-diff': {
glyph: 'android-sync',
display: 'Determining sync status',
isOk: true,
isSyncing: true,
},
connecting: {
glyph: 'android-sync',
display: 'Streaming',
isOk: true,
isSyncing: true,
},
'merkle-sync': {
glyph: 'android-sync',
display: 'Syncing',
isOk: true,
isSyncing: true,
},
idle: {
glyph: 'cancel-circle-fill',
isOk: false,
isSyncing: false,
},
transient_failure: {
glyph: 'cancel-circle-fill',
isOk: false,
isSyncing: false,
},
shutdown: {
glyph: 'cancel-circle-fill',
isOk: false,
isSyncing: false,
},
};

export function clusterStates([state]) {
const defaultDisplay = {
glyph: '',
display: '',
isOk: null,
isSyncing: null,
};
return CLUSTER_STATES[state] || defaultDisplay;
}

export default buildHelper(clusterStates);
12 changes: 5 additions & 7 deletions ui/lib/core/addon/templates/components/replication-mode-summary.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@
</div>
<div class="level-right">
{{#if replicationEnabled}}
{{#if (get cluster (concat mode 'StateGlyph'))}}
<span class="has-text-success">
{{#if (cluster-states modeState)}}
<span class="{{if (get (cluster-states modeState) "isOk") "has-text-success" "has-text-danger"}}">
<Icon
aria-hidden="true"
@glyph={{get cluster (concat mode 'StateGlyph')}}
@glyph={{get (cluster-states modeState) "glyph"}}
/>
</span>
{{else if syncProgress}}
Expand Down Expand Up @@ -104,10 +104,8 @@
{{#link-to "vault.cluster.replication.mode.index" cluster.name mode class="button is-primary"}}
Enable
{{/link-to}}
{{else if (eq mode 'dr')}}
{{cluster.drReplicationStateDisplay}}
{{else if (eq mode 'performance')}}
{{cluster.perfReplicationStateDisplay}}
{{else}}
{{get (cluster-states modeState) "display"}}
{{/if}}
</div>
{{/if}}
Loading