Skip to content

Commit

Permalink
Correctly using followUpEvaluation
Browse files Browse the repository at this point in the history
  • Loading branch information
philrenaud committed May 2, 2024
1 parent d1045e7 commit 6bdb3fe
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 92 deletions.
91 changes: 5 additions & 86 deletions ui/app/components/job-row.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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' ||
Expand All @@ -111,15 +48,6 @@ export default class JobRow extends Component {
}

@task(function* () {

Check failure on line 50 in ui/app/components/job-row.js

View workflow job for this annotation

GitHub Actions / pre-test

This generator function does not have 'yield'
// 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
Expand All @@ -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;
Expand All @@ -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';
}
Expand Down
6 changes: 3 additions & 3 deletions ui/app/models/allocation.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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() {
Expand Down
1 change: 1 addition & 0 deletions ui/app/routes/jobs/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
9 changes: 8 additions & 1 deletion ui/app/serializers/job.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand All @@ -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',
},
},
},
},
});
});
Expand Down
3 changes: 1 addition & 2 deletions ui/app/templates/components/job-row.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,7 @@
<JobStatus::AllocationStatusRow
@allocBlocks={{@job.allocBlocks}}
@steady={{true}}
{{!-- TODO: compact temporarily removed for testing --}}
{{!-- @compact={{true}} --}}
@compact={{true}}
@runningAllocs={{@job.allocBlocks.running.healthy.nonCanary.length}}
@groupCountSum={{@job.expectedRunningAllocCount}}
/>
Expand Down

0 comments on commit 6bdb3fe

Please sign in to comment.