WIP - ES Troubleshooting #1020
WIP - ES Troubleshooting #1020
Conversation
| }, | ||
| canTroubleshoot(node) { | ||
| // Manager-type nodes only | ||
| return ['so-manager', 'so-managersearch', 'so-standalone'].indexOf(node.role) != -1; |
There was a problem hiding this comment.
I would think there would be more node types here, like eval, import, etc.
| <span class="filter label">{{ i18n.eventstoreStatus }}:</span> | ||
| <span id="node_eventstoreStatus" :class="areMetricsCurrent(item) ? 'filter value text-' + colorNodeStatus(item.eventstoreStatus) : 'filter value stale'"> | ||
| {{ $root.localizeMessage(item.eventstoreStatus) }} | ||
| <v-icon v-if="canTroubleshoot(item)" style="font-size: 10px; vertical-align: middle;" :title="i18n.troubleshootElasticsearch" @click="showTroubleshootDialog(item)" class="theme-icon cursor-pointer ml-1" data-aid="grid_node_troubleshoot">fa-circle-info</v-icon> |
There was a problem hiding this comment.
I try to avoid inline styles. Was the icon too big by default? If so, the vuetify v-icon component should have a size property.
| </v-alert> | ||
| <div v-else> | ||
| <!-- Issues Section - Show problems first --> | ||
| <div v-if="troubleshootData.elasticsearchStatus?.healthReport?.nonGreenCount > 0 && troubleshootData.elasticsearchStatus?.healthReport?.indicators?.length"> |
There was a problem hiding this comment.
These inline scripts are getting complex. Would be cleaner and testable to move them to grid.js functions. Ex: hasEsTroubleshootIssues(). Similar comment for others in this file.
| <template v-if="diag"> | ||
| <p class="mb-2 text-caption">{{ diag.cause }}</p> | ||
| <p v-if="diag.affectedNodes?.length > 0" class="mb-2 text-caption"> | ||
| Affected node{{ diag.affectedNodes.length > 1 ? 's' : '' }}: {{ diag.affectedNodes.length <= 5 ? diag.affectedNodes.join(', ') : diag.affectedNodes.slice(0, 5).join(', ') + ' and ' + (diag.affectedNodes.length - 5) + ' more' }} |
There was a problem hiding this comment.
This looks like it could use some help.
- "Affected node" should be localized
- Problem need to move that logic/join statement into a helper function. Ex:
getEsTroubleshootEffectedNodes()
Several other English phrases in this PR also need localized.
| {{ diag.affectedIndices.length }} affected indices | ||
| </v-expansion-panel-title> | ||
| <v-expansion-panel-text> | ||
| <div style="max-height: 150px; overflow-y: auto;" class="text-caption"> |
There was a problem hiding this comment.
Another example where you should try to stick to vuetify style classes instead of inlining styles. Matthew can assist here if this is going to be too time consuming to get ramped up on.
| // Manager-type nodes only | ||
| return ['so-manager', 'so-managersearch', 'so-standalone'].indexOf(node.role) != -1; | ||
| }, | ||
| showTroubleshootDialog(node) { |
There was a problem hiding this comment.
These new JS functions need unit tests.
| return | ||
| } | ||
|
|
||
| // Handle Elasticsearch troubleshoot operation separately since it returns output |
There was a problem hiding this comment.
Currently you have a mixture of reusing the manage member handler but then branching out to a new gridMemberStore function. There are two ways to handle this:
- Go with the new
Diagnose()salt-store function which would require this logic to move to a new postDiagnose() API handler.
- or -
- Remove this
iflogic here and pass in the new op into the existingManageMember()function, and have it do the branching, like test/restart ops.
| ManageMember(ctx context.Context, operation string, id string) error | ||
| SendFile(ctx context.Context, node string, from string, to string, cleanup bool) error | ||
| Import(ctx context.Context, node string, file string, importer string) (*string, error) | ||
| RunTroubleshoot(ctx context.Context, node string, script string) (string, error) |
There was a problem hiding this comment.
This would go away in favor us either reusing the existing ManageMember func or creating a new Diagnose func.
| @@ -1164,6 +1164,28 @@ func (store *Saltstore) Import(ctx context.Context, node string, file string, im | |||
| return &output, err | |||
| } | |||
|
|
|||
| func (store *Saltstore) RunTroubleshoot(ctx context.Context, node string, script string) (string, error) { | |||
There was a problem hiding this comment.
Consider renaming this to Diagnose() and pass in an enum like elastic or storage. The have a switch determine which script to use for that incoming enum. Or have a single bash script called so-diagnose that accepts the enum and the bash script can branch out to different diagnostic routines.
But first read the other comments below.
2 participants
No description provided.