Skip to content

Commit 76817ec

Browse files
authored
fix(podlogs): follow mode exit bug and test improvements (cloudnative-pg#9599)
The test "should catch extra logs if given the follow option" was flaky in CI because it tested implementation details (counting loop iterations with tight timing) rather than actual behavior. Redesigned the test to verify what Follow=true actually does: it keeps streaming until the context is cancelled. The test now uses Eventually/Consistently patterns that handle timing variations gracefully, making it robust across different environments. The improved test exposed a bug in the production code: the streaming function would exit when all current log streams completed, even when Follow=true was set. This caused premature exit instead of continuing to poll for new or restarted pods. Fixed by changing the exit condition to only apply when Follow=false. Signed-off-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>
1 parent 59db511 commit 76817ec

File tree

2 files changed

+27
-18
lines changed

2 files changed

+27
-18
lines changed

pkg/podlogs/cluster_writer.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ func (csr *ClusterWriter) SingleStream(ctx context.Context, writer io.Writer) er
220220
}
221221
}
222222
}
223-
if streamSet.isZero() {
223+
if !csr.Options.Follow && streamSet.isZero() {
224224
return nil
225225
}
226226

pkg/podlogs/cluster_writer_test.go

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ package podlogs
2222
import (
2323
"bytes"
2424
"context"
25-
"strings"
2625
"sync"
2726
"time"
2827

@@ -153,40 +152,50 @@ var _ = Describe("Cluster logging tests", func() {
153152
Expect(logBuffer.String()).To(BeEquivalentTo("fake logs\nfake logs\n"))
154153
})
155154

156-
It("should catch extra logs if given the follow option", func(ctx context.Context) {
155+
It("should continue streaming when follow option is enabled", func(ctx context.Context) {
157156
client := fake.NewClientset(pod)
158-
159-
wg := sync.WaitGroup{}
157+
var wg sync.WaitGroup
160158
wg.Add(1)
161159
var logBuffer syncBuffer
160+
errChan := make(chan error, 1)
161+
streamCtx, cancel := context.WithCancel(ctx)
162162

163-
// let's set a short follow-wait, and keep the cluster streaming for two
164-
// cycles
165-
followWaiting := 150 * time.Millisecond
166-
ctx2, cancel := context.WithTimeout(ctx, 300*time.Millisecond)
167163
go func() {
168-
// we always invoke done no matter what happens
169164
defer wg.Done()
170165
defer GinkgoRecover()
171166
streamClusterLogs := ClusterWriter{
172167
Cluster: cluster,
173168
Options: &corev1.PodLogOptions{
174169
Follow: true,
175170
},
176-
FollowWaiting: followWaiting,
171+
FollowWaiting: 50 * time.Millisecond, // Short interval for test speed
177172
Client: client,
178173
}
179-
err := streamClusterLogs.SingleStream(ctx2, &logBuffer)
180-
// we cannot reliably now if we will close the function before the context
181-
// deadline, so we accept both nil and context.DeadlineExceeded
182-
Expect(err).To(Or(BeNil(), Equal(context.DeadlineExceeded)))
174+
err := streamClusterLogs.SingleStream(streamCtx, &logBuffer)
175+
errChan <- err
183176
}()
184177

185-
time.Sleep(350 * time.Millisecond)
178+
Eventually(func() bool {
179+
return len(logBuffer.String()) > 0
180+
}, 2*time.Second, 10*time.Millisecond).Should(BeTrue(),
181+
"streaming should capture logs")
182+
183+
Consistently(func() bool {
184+
select {
185+
case <-errChan:
186+
return false
187+
default:
188+
return true
189+
}
190+
}, 200*time.Millisecond, 50*time.Millisecond).Should(BeTrue(),
191+
"streaming should continue until cancelled")
192+
186193
cancel()
187194
wg.Wait()
188195

189-
fakeLogCount := strings.Count(logBuffer.String(), "fake logs\n")
190-
Expect(fakeLogCount).To(BeNumerically(">=", 2))
196+
var streamErr error
197+
Eventually(errChan, time.Second).Should(Receive(&streamErr))
198+
Expect(streamErr).To(Equal(context.Canceled))
199+
Expect(logBuffer.String()).To(ContainSubstring("fake logs"))
191200
})
192201
})

0 commit comments

Comments
 (0)