Skip to content

Commit

Permalink
Fix bug: display job as failed if job backoff limit has reached (#3098)
Browse files Browse the repository at this point in the history
* Fix bug: display job as failed if job backoff limit has reached

* Update backend API model. Fix broken unit test
  • Loading branch information
alexmt authored and k8s-ci-robot committed Jul 9, 2018
1 parent 547f5a7 commit d1d99e5
Show file tree
Hide file tree
Showing 7 changed files with 88 additions and 3 deletions.
33 changes: 33 additions & 0 deletions src/app/backend/resource/job/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,24 @@ type JobList struct {
Errors []error `json:"errors"`
}

type JobStatusType string

const (
// JobRunning means the job is still running.
JobStatusRunning JobStatusType = "Running"
// JobComplete means the job has completed its execution.
JobStatusComplete JobStatusType = "Complete"
// JobFailed means the job has failed its execution.
JobStatusFailed JobStatusType = "Failed"
)

type JobStatus struct {
// Short, machine understandable job status code.
Status JobStatusType `json:"status"`
// A human-readable description of the status of related job.
Message string `json:"message"`
}

// Job is a presentation layer view of Kubernetes Job resource. This means it is Job plus additional
// augmented data we can get from other sources
type Job struct {
Expand All @@ -60,6 +78,9 @@ type Job struct {

// number of parallel jobs defined.
Parallelism *int32 `json:"parallelism"`

// JobStatus contains inferred job status based on job conditions
JobStatus JobStatus `json:"jobStatus"`
}

// GetJobList returns a list of all Jobs in the cluster.
Expand Down Expand Up @@ -140,12 +161,24 @@ func ToJobList(jobs []batch.Job, pods []v1.Pod, events []v1.Event, nonCriticalEr
}

func toJob(job *batch.Job, podInfo *common.PodInfo) Job {
jobStatus := JobStatus{Status: JobStatusRunning}
for _, condition := range job.Status.Conditions {
if condition.Type == batch.JobComplete && condition.Status == v1.ConditionTrue {
jobStatus.Status = JobStatusComplete
break
} else if condition.Type == batch.JobFailed && condition.Status == v1.ConditionTrue {
jobStatus.Status = JobStatusFailed
jobStatus.Message = condition.Message
break
}
}
return Job{
ObjectMeta: api.NewObjectMeta(job.ObjectMeta),
TypeMeta: api.NewTypeMeta(api.ResourceKindJob),
ContainerImages: common.GetContainerImages(&job.Spec.Template.Spec),
InitContainerImages: common.GetInitContainerImages(&job.Spec.Template.Spec),
Pods: *podInfo,
JobStatus: jobStatus,
Parallelism: job.Spec.Parallelism,
}
}
10 changes: 10 additions & 0 deletions src/app/backend/resource/job/list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,10 @@ func TestGetJobListFromChannels(t *testing.T) {
},
Status: batch.JobStatus{
Active: 7,
Conditions: []batch.JobCondition{{
Type: batch.JobFailed,
Status: v1.ConditionTrue,
}},
},
},
},
Expand Down Expand Up @@ -165,6 +169,9 @@ func TestGetJobListFromChannels(t *testing.T) {
Failed: 2,
Warnings: []common.Event{},
},
JobStatus: JobStatus{
Status: JobStatusRunning,
},
}, {
ObjectMeta: api.ObjectMeta{
Name: "rs-name",
Expand All @@ -179,6 +186,9 @@ func TestGetJobListFromChannels(t *testing.T) {
Failed: 2,
Warnings: []common.Event{},
},
JobStatus: JobStatus{
Status: JobStatusFailed,
},
}},
Errors: []error{},
},
Expand Down
3 changes: 3 additions & 0 deletions src/app/backend/resource/workload/workload_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,9 @@ func TestGetWorkloadsFromChannels(t *testing.T) {
Warnings: []common.Event{},
Desired: &replicas,
},
JobStatus: job.JobStatus{
Status: job.JobStatusRunning,
},
}},
[]cronjob.CronJob{{
ObjectMeta: api.ObjectMeta{
Expand Down
11 changes: 10 additions & 1 deletion src/app/externs/backendapi.js
Original file line number Diff line number Diff line change
Expand Up @@ -312,14 +312,23 @@ backendApi.ReplicaSetDetail;
*/
backendApi.ReplicaSetList;

/**
* @typedef {{
* status: !string,
* message: string
* }}
*/
backendApi.JobStatus;

/**
* @typedef {{
* objectMeta: !backendApi.ObjectMeta,
* typeMeta: !backendApi.TypeMeta,
* pods: !backendApi.PodInfo,
* containerImages: !Array<string>,
* initContainerImages: !Array<string>,
* parallelism: number
* parallelism: number,
* jobStatus: !backendApi.JobStatus
* }}
*/
backendApi.Job;
Expand Down
6 changes: 6 additions & 0 deletions src/app/frontend/job/list/card.html
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@
<kd-resource-card object-meta="$ctrl.job.objectMeta"
type-meta="$ctrl.job.typeMeta">
<kd-resource-card-status layout="row">
<md-icon class="material-icons kd-error" ng-if="::$ctrl.isFailed()">
error
<md-tooltip md-delay="500" md-autohide>
{{::$ctrl.job.jobStatus.message}}
</md-tooltip>
</md-icon>
<md-icon class="material-icons md-warn"
ng-if="::$ctrl.hasWarnings()">
error
Expand Down
13 changes: 11 additions & 2 deletions src/app/frontend/job/list/card_component.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,16 @@ class JobCardController {
* @export
*/
hasWarnings() {
return this.job.pods.warnings.length > 0;
return this.job.pods.warnings.length > 0 && !this.isFailed();
}

/**
* Checks if job status is failed.
* @return {boolean}
* @export
*/
isFailed() {
return this.job.jobStatus.status === 'Failed';
}

/**
Expand All @@ -82,7 +91,7 @@ class JobCardController {
* @export
*/
isSuccess() {
return !this.isPending() && !this.hasWarnings();
return !this.isPending() && !this.hasWarnings() && !this.isFailed();
}
}

Expand Down
15 changes: 15 additions & 0 deletions src/test/frontend/job/list/card_component_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ describe('Job card', () => {
},
],
},
jobStatus: {
status: 'Running',
},
};

// then
Expand All @@ -73,6 +76,9 @@ describe('Job card', () => {
pods: {
warnings: [],
},
jobStatus: {
status: 'Completed',
},
};

// then
Expand All @@ -88,6 +94,9 @@ describe('Job card', () => {
warnings: [],
pending: 1,
},
jobStatus: {
status: 'Running',
},
};

// then
Expand All @@ -106,6 +115,9 @@ describe('Job card', () => {
},
],
},
jobStatus: {
status: 'Running',
},
};

// then
Expand All @@ -121,6 +133,9 @@ describe('Job card', () => {
warnings: [],
pending: 0,
},
jobStatus: {
status: 'Running',
},
};

// then
Expand Down

0 comments on commit d1d99e5

Please sign in to comment.