Skip to content
This repository was archived by the owner on Jan 19, 2023. It is now read-only.

Commit 11a006b

Browse files
scothisscothis
committed
UX refinement and polish
- in the Service/Configuration listing, add 'Revision' suffix to latest X column headers - in the Service/Configuration listing, convert revision names into links - notify user when deleting/editing a controlled resource - swap status and reason columns in conditions table - sort conditions table, promoting Ready condition - 'Route Traffic' -> 'Traffic Policy' Signed-off-by: Scott Andrews <andrewssc@vmware.com>
1 parent 496656c commit 11a006b

File tree

6 files changed

+72
-53
lines changed

6 files changed

+72
-53
lines changed

‎src/components/conditions.ts‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -129,22 +129,31 @@ export class ConditionListFactory implements ComponentFactory<any> {
129129
const table = new h.TableFactoryBuilder([], [], void 0, void 0, void 0, void 0, this.factoryMetadata);
130130
table.columns = [
131131
columns.type,
132-
columns.reason,
133132
columns.status,
133+
columns.reason,
134134
columns.message,
135135
columns.lastTransition,
136136
];
137137
table.emptyContent = "There are no conditions!";
138138
table.loading = false;
139+
140+
const conditions = this.conditions.slice();
141+
// sort by type
142+
conditions.sort((a, b) => (a.type || '').localeCompare(b.type || ''));
143+
const conditionPrimaryIndex = conditions.findIndex(c => c.type == 'Ready' || c.type == 'Succeeded');
144+
if (conditionPrimaryIndex > 0) {
145+
// move the primary condition to the start, otherwise preserving the order
146+
conditions.splice(0, 0, conditions.splice(conditionPrimaryIndex, 1)[0]);
147+
}
139148

140-
for (const condition of this.conditions) {
149+
for (const condition of conditions) {
141150
const row = new h.TableRow(
142151
{
143152
[columns.type]: new TextFactory({ value: condition.type }),
153+
[columns.status]: new TextFactory({ value: condition.status || ConditionStatus.Unknown }),
144154
[columns.reason]: condition.reason ?
145155
new TextFactory({ value: condition.reason }) :
146156
new TextFactory({ value: '*unknown*', options: { isMarkdown: true } }),
147-
[columns.status]: new TextFactory({ value: condition.status || ConditionStatus.Unknown }),
148157
[columns.message]: condition.message ?
149158
new TextFactory({ value: condition.message }) :
150159
new TextFactory({ value: '*empty*', options: { isMarkdown: true } }),

‎src/components/grid-actions.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ interface GridAction {
1717
}
1818

1919
export function deleteGridAction(obj: RuntimeObject): GridAction {
20+
const controlledBy = obj.metadata.ownerReferences?.find(r => r.controller);
21+
const controlledByMessage = controlledBy ? `This resource is controlled by *${controlledBy.kind}* **${controlledBy.name}**, deleting it may not have the intended effect.\n\n` : ''
22+
2023
return {
2124
name: "Delete",
2225
actionPath: "action.octant.dev/deleteObject",
@@ -28,7 +31,7 @@ export function deleteGridAction(obj: RuntimeObject): GridAction {
2831
},
2932
confirmation: {
3033
title: `Delete ${obj.kind}`,
31-
body: `Are you sure you want to delete *${obj.kind}* **${obj.metadata.name}**? This action is permanent and cannot be recovered.`,
34+
body: `${controlledByMessage}Are you sure you want to delete *${obj.kind}* **${obj.metadata.name}**? This action is permanent and cannot be recovered.`,
3235
},
3336
type: "danger",
3437
};

‎src/knative.ts‎

Lines changed: 46 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ export default class MyPlugin implements octant.Plugin {
246246
return handler.call(this, Object.assign({}, params, request));
247247
} catch (e) {
248248
// TODO handle errors other than not found
249-
console.error(`Error rendering Knative plugin content path "${contentPath}": ${JSON.stringify(e)}`);
249+
console.error(`Error rendering Knative plugin content path "${contentPath}": `, e);
250250
return notFoundContentResponse({ contentPath });
251251
}
252252
}
@@ -347,13 +347,21 @@ export default class MyPlugin implements octant.Plugin {
347347

348348
serviceDetailHandler(params: any): octant.ContentResponse {
349349
const name: string = params.serviceName;
350+
const service: Service = this.dashboardClient.Get({
351+
apiVersion: ServingV1,
352+
kind: ServingV1Service,
353+
namespace: this.namespace,
354+
name: name,
355+
});
350356
const title = [
351357
new LinkFactory({ value: "Knative", ref: "/knative" }),
352358
new LinkFactory({ value: "Serving", ref: this.linker({ apiVersion: ServingV1 }) }),
353359
new LinkFactory({ value: "Services", ref: this.linker({ apiVersion: ServingV1, kind: ServingV1Service }) }),
354360
new TextFactory({ value: name }),
355361
];
356-
const body = this.serviceDetail(name);
362+
const body = this.serviceDetail(service);
363+
const controlledBy = service.metadata.ownerReferences?.find(r => r.controller);
364+
const controlledByMessage = controlledBy ? `This resource is controlled by *${controlledBy.kind}* **${controlledBy.name}**, deleting it may not have the intended effect.\n\n` : '';
357365
const buttonGroup = new ButtonGroupFactory({
358366
buttons: [
359367
{
@@ -367,7 +375,7 @@ export default class MyPlugin implements octant.Plugin {
367375
},
368376
confirmation: {
369377
title: "Delete Service",
370-
body: `Are you sure you want to delete *Service* **${name}**? This action is permanent and cannot be recovered.`,
378+
body: `${controlledByMessage}Are you sure you want to delete *Service* **${name}**? This action is permanent and cannot be recovered.`,
371379
},
372380
},
373381
],
@@ -396,13 +404,21 @@ export default class MyPlugin implements octant.Plugin {
396404

397405
configurationDetailHandler(params: any): octant.ContentResponse {
398406
const name: string = params.configurationName;
407+
const configuration: Configuration = this.dashboardClient.Get({
408+
apiVersion: ServingV1,
409+
kind: ServingV1Configuration,
410+
namespace: this.namespace,
411+
name: name,
412+
});
399413
const title = [
400414
new LinkFactory({ value: "Knative", ref: "/knative" }),
401415
new LinkFactory({ value: "Serving", ref: this.linker({ apiVersion: ServingV1 }) }),
402416
new LinkFactory({ value: "Configurations", ref: this.linker({ apiVersion: ServingV1, kind: ServingV1Configuration }) }),
403417
new TextFactory({ value: name }),
404418
];
405-
const body = this.configurationDetail(name);
419+
const body = this.configurationDetail(configuration);
420+
const controlledBy = configuration.metadata.ownerReferences?.find(r => r.controller);
421+
const controlledByMessage = controlledBy ? `This resource is controlled by *${controlledBy.kind}* **${controlledBy.name}**, deleting it may not have the intended effect.\n\n` : '';
406422
const buttonGroup = new ButtonGroupFactory({
407423
buttons: [
408424
{
@@ -416,7 +432,7 @@ export default class MyPlugin implements octant.Plugin {
416432
},
417433
confirmation: {
418434
title: "Delete Configuration",
419-
body: `Are you sure you want to delete *Configuration* **${name}**? This action is permanent and cannot be recovered.`,
435+
body: `${controlledByMessage}Are you sure you want to delete *Configuration* **${name}**? This action is permanent and cannot be recovered.`,
420436
},
421437
},
422438
],
@@ -441,6 +457,12 @@ export default class MyPlugin implements octant.Plugin {
441457

442458
revisionDetailHandler(params: any): octant.ContentResponse {
443459
const name: string = params.revisionName;
460+
const revision: Revision = this.dashboardClient.Get({
461+
apiVersion: ServingV1,
462+
kind: ServingV1Revision,
463+
namespace: this.namespace,
464+
name: name,
465+
});
444466
const title = [];
445467
if (params.serviceName) {
446468
title.push(
@@ -462,7 +484,9 @@ export default class MyPlugin implements octant.Plugin {
462484
);
463485
}
464486

465-
const body = this.revisionDetail(name);
487+
const body = this.revisionDetail(revision);
488+
const controlledBy = revision.metadata.ownerReferences?.find(r => r.controller);
489+
const controlledByMessage = controlledBy ? `This resource is controlled by *${controlledBy.kind}* **${controlledBy.name}**, deleting it may not have the intended effect.\n\n` : '';
466490
const buttonGroup = new ButtonGroupFactory({
467491
buttons: [
468492
{
@@ -476,7 +500,7 @@ export default class MyPlugin implements octant.Plugin {
476500
},
477501
confirmation: {
478502
title: "Delete Revision",
479-
body: `Are you sure you want to delete *Revision* **${name}**? This action is permanent and cannot be recovered.`,
503+
body: `${controlledByMessage}Are you sure you want to delete *Revision* **${name}**? This action is permanent and cannot be recovered.`,
480504
},
481505
},
482506
],
@@ -505,13 +529,21 @@ export default class MyPlugin implements octant.Plugin {
505529

506530
routeDetailHandler(params: any): octant.ContentResponse {
507531
const name: string = params.routeName;
532+
const route: Route = this.dashboardClient.Get({
533+
apiVersion: ServingV1,
534+
kind: ServingV1Route,
535+
namespace: this.namespace,
536+
name: name,
537+
});
508538
const title = [
509539
new LinkFactory({ value: "Knative", ref: "/knative" }),
510540
new LinkFactory({ value: "Serving", ref: this.linker({ apiVersion: ServingV1 }) }),
511541
new LinkFactory({ value: "Routes", ref: this.linker({ apiVersion: ServingV1, kind: ServingV1Route }) }),
512542
new TextFactory({ value: name }),
513543
];
514-
const body = this.routeDetail(name);
544+
const body = this.routeDetail(route);
545+
const controlledBy = route.metadata.ownerReferences?.find(r => r.controller);
546+
const controlledByMessage = controlledBy ? `This resource is controlled by *${controlledBy.kind}* **${controlledBy.name}**, deleting it may not have the intended effect.\n\n` : '';
515547
const buttonGroup = new ButtonGroupFactory({
516548
buttons: [
517549
{
@@ -525,7 +557,7 @@ export default class MyPlugin implements octant.Plugin {
525557
},
526558
confirmation: {
527559
title: "Delete Route",
528-
body: `Are you sure you want to delete *Route* **${name}**? This action is permanent and cannot be recovered.`,
560+
body: `${controlledByMessage}Are you sure you want to delete *Route* **${name}**? This action is permanent and cannot be recovered.`,
529561
},
530562
},
531563
],
@@ -564,13 +596,7 @@ export default class MyPlugin implements octant.Plugin {
564596
return [form];
565597
}
566598

567-
serviceDetail(name: string): ComponentFactory<any>[] {
568-
const service: Service = this.dashboardClient.Get({
569-
apiVersion: ServingV1,
570-
kind: ServingV1Service,
571-
namespace: this.namespace,
572-
name: name,
573-
});
599+
serviceDetail(service: Service): ComponentFactory<any>[] {
574600
const routes: Route[] = this.dashboardClient.List({
575601
apiVersion: ServingV1,
576602
kind: ServingV1Route,
@@ -683,13 +709,7 @@ export default class MyPlugin implements octant.Plugin {
683709
return new ConfigurationListFactory({ configurations, linker: this.linker, factoryMetadata });
684710
}
685711

686-
configurationDetail(name: string): ComponentFactory<any>[] {
687-
const configuration: Configuration = this.dashboardClient.Get({
688-
apiVersion: ServingV1,
689-
kind: ServingV1Configuration,
690-
namespace: this.namespace,
691-
name: name,
692-
});
712+
configurationDetail(configuration: Configuration): ComponentFactory<any>[] {
693713
const revisions: Revision[] = this.dashboardClient.List({
694714
apiVersion: ServingV1,
695715
kind: ServingV1Revision,
@@ -763,13 +783,7 @@ export default class MyPlugin implements octant.Plugin {
763783
];
764784
}
765785

766-
revisionDetail(name: string): ComponentFactory<any>[] {
767-
const revision: Revision = this.dashboardClient.Get({
768-
apiVersion: ServingV1,
769-
kind: ServingV1Revision,
770-
namespace: this.namespace,
771-
name: name,
772-
});
786+
revisionDetail(revision: Revision): ComponentFactory<any>[] {
773787
const childDeployment: V1Deployment | undefined = this.dashboardClient.List({
774788
apiVersion: 'apps/v1',
775789
kind: 'Deployment',
@@ -785,7 +799,7 @@ export default class MyPlugin implements octant.Plugin {
785799
kind: 'Pod',
786800
namespace: this.namespace,
787801
selector: {
788-
'serving.knative.dev/revision': name,
802+
'serving.knative.dev/revision': revision.metadata.name || '_',
789803
},
790804
});
791805
pods.sort((a, b) => (a.metadata?.name || '').localeCompare(b.metadata?.name || ''));
@@ -838,14 +852,7 @@ export default class MyPlugin implements octant.Plugin {
838852
return new RouteListFactory({ routes, linker: this.linker, factoryMetadata });
839853
}
840854

841-
routeDetail(name: string): ComponentFactory<any>[] {
842-
const route: Route = this.dashboardClient.Get({
843-
apiVersion: ServingV1,
844-
kind: ServingV1Route,
845-
namespace: this.namespace,
846-
name: name,
847-
});
848-
855+
routeDetail(route: Route): ComponentFactory<any>[] {
849856
return [
850857
new RouteSummaryFactory({
851858
route,

‎src/serving/configuration.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import { TextFactory } from "@project-octant/plugin/components/text";
2424
import { TimestampFactory } from "@project-octant/plugin/components/timestamp";
2525

2626
import { V1Deployment } from "@kubernetes/client-node";
27-
import { ServingV1, ServingV1Configuration, Configuration, Revision, Route } from "./api";
27+
import { ServingV1, ServingV1Configuration, Configuration, Revision, Route, ServingV1Revision } from "./api";
2828
import { RevisionListFactory } from "./revision";
2929

3030
interface ConfigurationListParameters {
@@ -47,8 +47,8 @@ export class ConfigurationListFactory implements ComponentFactory<any> {
4747
toComponent(): Component<any> {
4848
const columns = {
4949
name: 'Name',
50-
latestCreated: 'Latest Created',
51-
latestReady: 'Latest Ready',
50+
latestCreated: 'Latest Created Revision',
51+
latestReady: 'Latest Ready Revision',
5252
age: 'Age',
5353
};
5454
const table = new h.TableFactoryBuilder([], [], void 0, void 0, void 0, void 0, this.factoryMetadata);
@@ -80,10 +80,10 @@ export class ConfigurationListFactory implements ComponentFactory<any> {
8080
},
8181
}),
8282
[columns.latestCreated]: status.latestCreatedRevisionName
83-
? new TextFactory({ value: status.latestCreatedRevisionName })
83+
? new LinkFactory({ value: status.latestCreatedRevisionName, ref: this.linker({ apiVersion: ServingV1, kind: ServingV1Revision, name: status.latestCreatedRevisionName }, { apiVersion: ServingV1, kind: ServingV1Configuration, name: metadata.name }) })
8484
: notFound,
8585
[columns.latestReady]: status.latestReadyRevisionName
86-
? new TextFactory({ value: status.latestReadyRevisionName })
86+
? new LinkFactory({ value: status.latestReadyRevisionName, ref: this.linker({ apiVersion: ServingV1, kind: ServingV1Revision, name: status.latestReadyRevisionName }, { apiVersion: ServingV1, kind: ServingV1Configuration, name: metadata.name }) })
8787
: notFound,
8888
[columns.age]: new TimestampFactory({ timestamp: Math.floor(new Date(metadata.creationTimestamp || 0).getTime() / 1000) }),
8989
},

‎src/serving/revision.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ export class RevisionListFactory implements ComponentFactory<any> {
6262
const columns = {
6363
name: 'Name',
6464
generation: 'Generation',
65-
traffic: 'Route Traffic',
65+
traffic: 'Traffic Policies',
6666
replicas: 'Replicas',
6767
age: 'Age',
6868
};

‎src/serving/service.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -140,8 +140,8 @@ export class ServiceListFactory implements ComponentFactory<any> {
140140
const columns = {
141141
name: 'Name',
142142
url: 'URL',
143-
latestCreated: 'Latest Created',
144-
latestReady: 'Latest Ready',
143+
latestCreated: 'Latest Created Revision',
144+
latestReady: 'Latest Ready Revision',
145145
age: 'Age',
146146
};
147147
const table = new h.TableFactoryBuilder([], [], void 0, void 0, void 0, void 0, this.factoryMetadata);
@@ -178,10 +178,10 @@ export class ServiceListFactory implements ComponentFactory<any> {
178178
? new LinkFactory({ value: status.url, ref: status.url })
179179
: notFound,
180180
[columns.latestCreated]: status.latestCreatedRevisionName
181-
? new TextFactory({ value: status.latestCreatedRevisionName })
181+
? new LinkFactory({ value: status.latestCreatedRevisionName, ref: this.linker({ apiVersion: ServingV1, kind: ServingV1Revision, name: status.latestCreatedRevisionName }, { apiVersion: ServingV1, kind: ServingV1Service, name: metadata.name }) })
182182
: notFound,
183183
[columns.latestReady]: status.latestReadyRevisionName
184-
? new TextFactory({ value: status.latestReadyRevisionName })
184+
? new LinkFactory({ value: status.latestReadyRevisionName, ref: this.linker({ apiVersion: ServingV1, kind: ServingV1Revision, name: status.latestReadyRevisionName }, { apiVersion: ServingV1, kind: ServingV1Service, name: metadata.name }) })
185185
: notFound,
186186
[columns.age]: new TimestampFactory({ timestamp: Math.floor(new Date(metadata.creationTimestamp || 0).getTime() / 1000) }),
187187
},

0 commit comments

Comments
 (0)