Skip to content

Commit e2d45f0

Browse files
committed
fix(logservice): eliminate RunJob false-success and harden CreatePVC
- RunJob: timeout and empty-pod-list paths used errors.Wrapf(err, ...) which returns nil when err is nil, causing RunJob to return a nil error (false success) when job polling timed out with the last GetJob succeeding. This made LogService bootstrap falsely report "succeeded" while the job was still failing. Use errors.Errorf for the timeout path and split the pod-list condition so nil err is not wrapped. - CreatePVC: snapshot StoreStorage/LogStorage into locals right after the nil checks and use them in PVC construction, defending against an intermittent nil-deref observed at the LogStorage.Size use despite the pre-existing nil checks.
1 parent 457d8b1 commit e2d45f0

2 files changed

Lines changed: 19 additions & 6 deletions

File tree

internal/resource/oblogservicenode/oblogservicenode_task.go

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,13 @@ func CreatePVC(m *OBLogServiceNodeManager) tasktypes.TaskError {
104104
return errors.New("storage.logStorage is required but was nil")
105105
}
106106

107+
// Snapshot the storage fields into locals after the nil checks so that any
108+
// hypothetical concurrent mutation of m.Resource.Spec.Storage between the
109+
// checks and the PVC construction cannot turn a non-nil check into a nil
110+
// deref (observed as an intermittent panic at the LogStorage.Size use).
111+
storeStorage := m.Resource.Spec.Storage.StoreStorage
112+
logStorage := m.Resource.Spec.Storage.LogStorage
113+
107114
podName := m.Resource.Name
108115
storePvcName := fmt.Sprintf("%s-%s", podName, oceanbaseconst.LogServiceStoreVolumeSuffix)
109116
logPvcName := fmt.Sprintf("%s-%s", podName, oceanbaseconst.LogServiceLogVolumeSuffix)
@@ -127,10 +134,10 @@ func CreatePVC(m *OBLogServiceNodeManager) tasktypes.TaskError {
127134
AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce},
128135
Resources: corev1.VolumeResourceRequirements{
129136
Requests: corev1.ResourceList{
130-
"storage": m.Resource.Spec.Storage.StoreStorage.Size,
137+
"storage": storeStorage.Size,
131138
},
132139
},
133-
StorageClassName: &m.Resource.Spec.Storage.StoreStorage.StorageClass,
140+
StorageClassName: &storeStorage.StorageClass,
134141
},
135142
}
136143
if err := m.Client.Create(m.Ctx, storePvc); err != nil && !kubeerrors.IsAlreadyExists(err) {
@@ -147,10 +154,10 @@ func CreatePVC(m *OBLogServiceNodeManager) tasktypes.TaskError {
147154
AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce},
148155
Resources: corev1.VolumeResourceRequirements{
149156
Requests: corev1.ResourceList{
150-
"storage": m.Resource.Spec.Storage.LogStorage.Size,
157+
"storage": logStorage.Size,
151158
},
152159
},
153-
StorageClassName: &m.Resource.Spec.Storage.LogStorage.StorageClass,
160+
StorageClassName: &logStorage.StorageClass,
154161
},
155162
}
156163
if err := m.Client.Create(m.Ctx, logPvc); err != nil && !kubeerrors.IsAlreadyExists(err) {

internal/resource/utils/jobs.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,14 +120,20 @@ func RunJob(ctx context.Context, c client.Client, logger *logr.Logger, namespace
120120
time.Sleep(time.Second * time.Duration(obcfg.GetConfig().Time.CheckJobInterval))
121121
}
122122
if !finished {
123-
return "", int32(cmdconst.ExitCodeNotExecuted), errors.Wrapf(err, "Run job %s timeout", fullJobName)
123+
// NOTE: do not use errors.Wrapf here — when the last GetJob succeeded
124+
// (job still running), err is nil and Wrapf(nil, ...) returns nil,
125+
// which would make RunJob falsely report success.
126+
return "", int32(cmdconst.ExitCodeNotExecuted), errors.Errorf("Run job %s timeout", fullJobName)
124127
}
125128
clientSet := k8sclient.GetClient()
126129
podList, err := clientSet.ClientSet.CoreV1().Pods(namespace).List(ctx, metav1.ListOptions{
127130
LabelSelector: fmt.Sprintf("job-name=%s", fullJobName),
128131
})
129132
if err != nil || len(podList.Items) == 0 {
130-
return "", int32(cmdconst.ExitCodeNotExecuted), errors.Wrapf(err, "failed to get pods of job %s", jobName)
133+
if err != nil {
134+
return "", int32(cmdconst.ExitCodeNotExecuted), errors.Wrapf(err, "failed to get pods of job %s", jobName)
135+
}
136+
return "", int32(cmdconst.ExitCodeNotExecuted), errors.Errorf("failed to get pods of job %s: no pods found", jobName)
131137
}
132138
var outputBuffer bytes.Buffer
133139
podLogOpts := corev1.PodLogOptions{}

0 commit comments

Comments
 (0)