Skip to content

Commit 15b7a96

Browse files
author
Arthur Granado
committed
fix: trace workloads with missing properties
Address TODO comments, either removing it or improve code, once it was clear which action we should take about each one. Basically the TODOs are in two kinds of functions, the handlers and the readers: - For the handlers, we think it's ok we just remove the TODO and do nothing, we're already doing it in other handlers, and these handlers basically remove the workload from Homebase once it is removed from the cluster. If we have any problem, this workload would be removed from Homebase in 4 days anyway. - For the readers, it is worth including an info log with the workload name and namespace. Just because, in the future, if a customer has any problem, we will have some logs that will help us to investigate it.
1 parent 5f54557 commit 15b7a96

File tree

8 files changed

+19
-14
lines changed

8 files changed

+19
-14
lines changed

src/supervisor/watchers/handlers/cron-job.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import { FALSY_WORKLOAD_NAME_MARKER } from './types';
66
export async function cronJobWatchHandler(cronJob: V1beta1CronJob): Promise<void> {
77
if (!cronJob.metadata || !cronJob.spec || !cronJob.spec.jobTemplate.spec ||
88
!cronJob.spec.jobTemplate.metadata || !cronJob.spec.jobTemplate.spec.template.spec) {
9-
// TODO(ivanstanev): possibly log this. It shouldn't happen but we should track it!
109
return;
1110
}
1211

src/supervisor/watchers/handlers/daemon-set.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import { FALSY_WORKLOAD_NAME_MARKER } from './types';
66
export async function daemonSetWatchHandler(daemonSet: V1DaemonSet): Promise<void> {
77
if (!daemonSet.metadata || !daemonSet.spec || !daemonSet.spec.template.metadata ||
88
!daemonSet.spec.template.spec || !daemonSet.status) {
9-
// TODO(ivanstanev): possibly log this. It shouldn't happen but we should track it!
109
return;
1110
}
1211

src/supervisor/watchers/handlers/deployment.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import { FALSY_WORKLOAD_NAME_MARKER } from './types';
66
export async function deploymentWatchHandler(deployment: V1Deployment): Promise<void> {
77
if (!deployment.metadata || !deployment.spec || !deployment.spec.template.metadata ||
88
!deployment.spec.template.spec || !deployment.status) {
9-
// TODO(ivanstanev): possibly log this. It shouldn't happen but we should track it!
109
return;
1110
}
1211

src/supervisor/watchers/handlers/job.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import { FALSY_WORKLOAD_NAME_MARKER } from './types';
55

66
export async function jobWatchHandler(job: V1Job): Promise<void> {
77
if (!job.metadata || !job.spec || !job.spec.template.metadata || !job.spec.template.spec) {
8-
// TODO(ivanstanev): possibly log this. It shouldn't happen but we should track it!
98
return;
109
}
1110

src/supervisor/watchers/handlers/replica-set.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import { FALSY_WORKLOAD_NAME_MARKER } from './types';
66
export async function replicaSetWatchHandler(replicaSet: V1ReplicaSet): Promise<void> {
77
if (!replicaSet.metadata || !replicaSet.spec || !replicaSet.spec.template ||
88
!replicaSet.spec.template.metadata || !replicaSet.spec.template.spec || !replicaSet.status) {
9-
// TODO(ivanstanev): possibly log this. It shouldn't happen but we should track it!
109
return;
1110
}
1211

src/supervisor/watchers/handlers/replication-controller.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ export async function replicationControllerWatchHandler(replicationController: V
77
if (!replicationController.metadata || !replicationController.spec || !replicationController.spec.template ||
88
!replicationController.spec.template.metadata || !replicationController.spec.template.spec ||
99
!replicationController.status) {
10-
// TODO(ivanstanev): possibly log this. It shouldn't happen but we should track it!
1110
return;
1211
}
1312

src/supervisor/watchers/handlers/stateful-set.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import { FALSY_WORKLOAD_NAME_MARKER } from './types';
66
export async function statefulSetWatchHandler(statefulSet: V1StatefulSet): Promise<void> {
77
if (!statefulSet.metadata || !statefulSet.spec || !statefulSet.spec.template.metadata ||
88
!statefulSet.spec.template.spec || !statefulSet.status) {
9-
// TODO(ivanstanev): possibly log this. It shouldn't happen but we should track it!
109
return;
1110
}
1211

src/supervisor/workload-reader.ts

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { V1OwnerReference } from '@kubernetes/client-node';
33
import * as kubernetesApiWrappers from './kuberenetes-api-wrappers';
44
import { k8sApi } from './cluster';
55
import { IKubeObjectMetadata, WorkloadKind } from './types';
6+
import logger = require('../common/logger');
67

78
type IWorkloadReaderFunc = (
89
workloadName: string,
@@ -16,7 +17,8 @@ const deploymentReader: IWorkloadReaderFunc = async (workloadName, namespace) =>
1617

1718
if (!deployment.metadata || !deployment.spec || !deployment.spec.template.metadata ||
1819
!deployment.spec.template.spec || !deployment.status) {
19-
// TODO(ivanstanev): add logging to know when/if it happens!
20+
logIncompleteWorkload(workloadName, namespace);
21+
2022
return undefined;
2123
}
2224

@@ -37,7 +39,8 @@ const replicaSetReader: IWorkloadReaderFunc = async (workloadName, namespace) =>
3739

3840
if (!replicaSet.metadata || !replicaSet.spec || !replicaSet.spec.template ||
3941
!replicaSet.spec.template.metadata || !replicaSet.spec.template.spec || !replicaSet.status) {
40-
// TODO(ivanstanev): add logging to know when/if it happens!
42+
logIncompleteWorkload(workloadName, namespace);
43+
4144
return undefined;
4245
}
4346

@@ -58,7 +61,8 @@ const statefulSetReader: IWorkloadReaderFunc = async (workloadName, namespace) =
5861

5962
if (!statefulSet.metadata || !statefulSet.spec || !statefulSet.spec.template.metadata ||
6063
!statefulSet.spec.template.spec || !statefulSet.status) {
61-
// TODO(ivanstanev): add logging to know when/if it happens!
64+
logIncompleteWorkload(workloadName, namespace);
65+
6266
return undefined;
6367
}
6468

@@ -79,7 +83,8 @@ const daemonSetReader: IWorkloadReaderFunc = async (workloadName, namespace) =>
7983

8084
if (!daemonSet.metadata || !daemonSet.spec || !daemonSet.spec.template.spec ||
8185
!daemonSet.spec.template.metadata || !daemonSet.status) {
82-
// TODO(ivanstanev): add logging to know when/if it happens!
86+
logIncompleteWorkload(workloadName, namespace);
87+
8388
return undefined;
8489
}
8590

@@ -99,7 +104,8 @@ const jobReader: IWorkloadReaderFunc = async (workloadName, namespace) => {
99104
const job = jobResult.body;
100105

101106
if (!job.metadata || !job.spec || !job.spec.template.spec || !job.spec.template.metadata) {
102-
// TODO(ivanstanev): add logging to know when/if it happens!
107+
logIncompleteWorkload(workloadName, namespace);
108+
103109
return undefined;
104110
}
105111

@@ -122,7 +128,8 @@ const cronJobReader: IWorkloadReaderFunc = async (workloadName, namespace) => {
122128

123129
if (!cronJob.metadata || !cronJob.spec || !cronJob.spec.jobTemplate.metadata ||
124130
!cronJob.spec.jobTemplate.spec || !cronJob.spec.jobTemplate.spec.template.spec) {
125-
// TODO(ivanstanev): add logging to know when/if it happens!
131+
logIncompleteWorkload(workloadName, namespace);
132+
126133
return undefined;
127134
}
128135

@@ -143,7 +150,8 @@ const replicationControllerReader: IWorkloadReaderFunc = async (workloadName, na
143150
if (!replicationController.metadata || !replicationController.spec || !replicationController.spec.template ||
144151
!replicationController.spec.template.metadata || !replicationController.spec.template.spec ||
145152
!replicationController.status) {
146-
// TODO(ivanstanev): add logging to know when/if it happens!
153+
logIncompleteWorkload(workloadName, namespace);
154+
147155
return undefined;
148156
}
149157

@@ -157,6 +165,10 @@ const replicationControllerReader: IWorkloadReaderFunc = async (workloadName, na
157165
};
158166
};
159167

168+
function logIncompleteWorkload(workloadName: string, namespace: string): void {
169+
logger.info({ workloadName, namespace }, 'kubernetes api could not return workload');
170+
}
171+
160172
// Here we are using the "kind" property of a k8s object as a key to map it to a reader.
161173
// This gives us a quick look up table where we can abstract away the internal implementation of reading a resource
162174
// and just grab a generic handler/reader that does that for us (based on the "kind").

0 commit comments

Comments
 (0)