From 6bdb3fe040060256e7aa45772b73f00c2cfe8453 Mon Sep 17 00:00:00 2001 From: Phil Renaud Date: Thu, 2 May 2024 16:32:27 -0400 Subject: [PATCH] Correctly using followUpEvaluation --- ui/app/components/job-row.js | 91 ++----------------------- ui/app/models/allocation.js | 6 +- ui/app/routes/jobs/index.js | 1 + ui/app/serializers/job.js | 9 ++- ui/app/templates/components/job-row.hbs | 3 +- 5 files changed, 18 insertions(+), 92 deletions(-) diff --git a/ui/app/components/job-row.js b/ui/app/components/job-row.js index 716e5fac3fe9..9afb43a5550e 100644 --- a/ui/app/components/job-row.js +++ b/ui/app/components/job-row.js @@ -8,45 +8,13 @@ import Component from '@glimmer/component'; import { action } from '@ember/object'; import { inject as service } from '@ember/service'; -import { tracked } from '@glimmer/tracking'; import { task } from 'ember-concurrency'; -import { computed } from '@ember/object'; export default class JobRow extends Component { @service router; @service store; @service system; - // @tracked fullActiveDeploymentObject = {}; - - // /** - // * If our job has an activeDeploymentID, as determined by the statuses endpoint, - // * we check if this component's fullActiveDeploymentObject has the same ID. - // * If it does, we don't need to do any fetching: we can simply check this.fullActiveDeploymentObject.requiresPromotion - // * If it doesn't, we need to fetch the deployment with the activeDeploymentID - // * and set it to this.fullActiveDeploymentObject, then check this.fullActiveDeploymentObject.requiresPromotion. - // */ - // get requiresPromotion() { - // if (!this.args.job.hasActiveCanaries || !this.args.job.activeDeploymentID) { - // return false; - // } - - // if (this.fullActiveDeploymentObject && this.fullActiveDeploymentObject.id === this.args.job.activeDeploymentID) { - // return this.fullActiveDeploymentObject.requiresPromotion; - // } - - // this.fetchActiveDeployment(); - // return false; - // } - - // @action - // async fetchActiveDeployment() { - // if (this.args.job.hasActiveCanaries && this.args.job.activeDeploymentID) { - // let deployment = await this.store.findRecord('deployment', this.args.job.activeDeploymentID); - // this.fullActiveDeploymentObject = deployment; - // } - // } - /** * Promotion of a deployment will error if the canary allocations are not of status "Healthy"; * this function will check for that and disable the promote button if necessary. @@ -63,45 +31,14 @@ export default class JobRow extends Component { } /** - * Similar to the below, but cares if any non-old canaries have failed, regardless of their rescheduled status. + * Used to inform the user that an allocation has entered into a perment state of failure: + * That is, it has exhausted its restarts and its reschedules and is in a terminal state. */ - get someCanariesHaveRescheduled() { - // TODO: Weird thing where alloc.isUnhealthy right away, because alloc.DeploymentStatus.Healthy is false. - // But that doesn't seem right: health check in that state should be unknown or null, perhaps. - const relevantAllocs = this.args.job.allocations.filter( - (a) => !a.isOld && a.isCanary - ); - console.log( - 'relevantAllocs', - relevantAllocs, - relevantAllocs.map((a) => a.jobVersion), - relevantAllocs.map((a) => a.clientStatus), - relevantAllocs.map((a) => a.isUnhealthy) - ); - - return relevantAllocs.some( - (a) => - a.clientStatus === 'failed' || - a.clientStatus === 'lost' || - a.isUnhealthy - ); - } - get someCanariesHaveFailedAndWontReschedule() { - let availableSlotsToFill = this.args.job.expectedRunningAllocCount; - let runningOrPendingCanaries = this.args.job.allocations.filter( - (a) => !a.isOld && a.isCanary && !a.hasBeenRescheduled - ); const relevantAllocs = this.args.job.allocations.filter( (a) => !a.isOld && a.isCanary && !a.hasBeenRescheduled ); - console.log( - 'relevantAllocs', - relevantAllocs, - relevantAllocs.map((a) => a.jobVersion), - relevantAllocs.map((a) => a.clientStatus), - relevantAllocs.map((a) => a.isUnhealthy) - ); + return relevantAllocs.some( (a) => a.clientStatus === 'failed' || @@ -111,15 +48,6 @@ export default class JobRow extends Component { } @task(function* () { - // ID: jobDeployments[0]?.id, - // IsActive: jobDeployments[0]?.status === 'running', - // // IsActive: true, - // JobVersion: jobDeployments[0]?.versionNumber, - // Status: jobDeployments[0]?.status, - // StatusDescription: jobDeployments[0]?.statusDescription, - // AllAutoPromote: false, - // RequiresPromotion: true, // TODO: lever - /** * @typedef DeploymentSummary * @property {string} id @@ -135,12 +63,6 @@ export default class JobRow extends Component { */ let latestDeploymentSummary = this.args.job.latestDeploymentSummary; - // console.log( - // 'checking if requries promotion', - // this.args.job.name, - // latestDeploymentSummary, - // this.args.job.hasActiveCanaries - // ); // Early return false if we don't have an active deployment if (!latestDeploymentSummary.isActive) { return false; @@ -158,11 +80,8 @@ export default class JobRow extends Component { console.log('canaries are healthy.'); return 'canary-promote'; } - // if (this.someCanariesHaveFailedAndWontReschedule) { - if (this.someCanariesHaveRescheduled) { - // TODO: I'm uncertain about when to alert the user here. It seems like it might be important - // enough to let them know when ANY canary has to be rescheduled, but there's an argument to be - // made that we oughtn't bother them until it's un-reschedulable. + + if (this.someCanariesHaveFailedAndWontReschedule) { console.log('some canaries have failed.'); return 'canary-failure'; } diff --git a/ui/app/models/allocation.js b/ui/app/models/allocation.js index 4d83507a6ff4..d1a0cec7f9dd 100644 --- a/ui/app/models/allocation.js +++ b/ui/app/models/allocation.js @@ -71,12 +71,12 @@ export default class Allocation extends Model { return ( this.willNotRestart && !this.get('nextAllocation.content') && - !this.get('followUpEvaluation.content') + !this.belongsTo('followUpEvaluation').id() ); } get hasBeenRescheduled() { - return this.get('followUpEvaluation.content'); + return Boolean(this.belongsTo('followUpEvaluation').id()); } get hasBeenRestarted() { @@ -131,7 +131,7 @@ export default class Allocation extends Model { preemptedByAllocation; @attr('boolean') wasPreempted; - @belongsTo('evaluation') followUpEvaluation; + @belongsTo('evaluation', { async: true }) followUpEvaluation; @computed('clientStatus') get statusClass() { diff --git a/ui/app/routes/jobs/index.js b/ui/app/routes/jobs/index.js index fdb0def121cd..94add2a491f0 100644 --- a/ui/app/routes/jobs/index.js +++ b/ui/app/routes/jobs/index.js @@ -112,6 +112,7 @@ export default class IndexRoute extends Route.extend( * @returns {Object} */ handleErrors(error) { + console.log('handling error', error); error.errors.forEach((err) => { this.notifications.add({ title: err.title, diff --git a/ui/app/serializers/job.js b/ui/app/serializers/job.js index a0435641a8bb..6662d6e78115 100644 --- a/ui/app/serializers/job.js +++ b/ui/app/serializers/job.js @@ -179,7 +179,6 @@ export default class JobSerializer extends ApplicationSerializer { data: { id: alloc.ID, type: 'allocation', - // TODO: This is too much manual pushing! I should take attributes as they come in. attributes: { clientStatus: alloc.ClientStatus, deploymentStatus: { @@ -189,6 +188,14 @@ export default class JobSerializer extends ApplicationSerializer { jobVersion: alloc.JobVersion, nodeID: alloc.NodeID, }, + relationships: { + followUpEvaluation: alloc.FollowupEvalID && { + data: { + id: alloc.FollowupEvalID, + type: 'evaluation', + }, + }, + }, }, }); }); diff --git a/ui/app/templates/components/job-row.hbs b/ui/app/templates/components/job-row.hbs index 1dc06efbc7fa..e4a217a53fc5 100644 --- a/ui/app/templates/components/job-row.hbs +++ b/ui/app/templates/components/job-row.hbs @@ -81,8 +81,7 @@