Skip to content

Commit b4dbe43

Browse files
committed
Review: Address comments
* Add test for failing to get pod-list from k8s * Replace deprecated NewSimpleClientset() with NewClientset()
1 parent 7757474 commit b4dbe43

File tree

2 files changed

+62
-1
lines changed

2 files changed

+62
-1
lines changed

pkg/jobs/common_job_list_test.go

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,22 @@
11
package jobs
22

33
import (
4+
"bytes"
45
"context"
6+
"fmt"
7+
"log"
58
"path/filepath"
69
"strings"
710
"testing"
811
"time"
912

13+
"github.com/nginxinc/nginx-k8s-supportpkg/pkg/data_collector"
1014
"github.com/nginxinc/nginx-k8s-supportpkg/pkg/mock"
15+
"github.com/stretchr/testify/assert"
16+
"go.uber.org/mock/gomock"
17+
"k8s.io/apimachinery/pkg/runtime"
18+
"k8s.io/client-go/kubernetes/fake"
19+
k8stesting "k8s.io/client-go/testing" // Add this line
1120
)
1221

1322
func TestCommonJobList_SelectedJobsProduceFiles(t *testing.T) {
@@ -81,3 +90,55 @@ func TestCommonJobList_PodListJSONKeyPresence(t *testing.T) {
8190
}
8291
}
8392
}
93+
94+
func TestCommonJobList_PodListError(t *testing.T) {
95+
ctrl := gomock.NewController(t)
96+
defer ctrl.Finish()
97+
98+
// Create mock clients
99+
mockClient := fake.NewSimpleClientset()
100+
101+
// Add a reactor that returns an error for pod list operations
102+
mockClient.PrependReactor("list", "pods", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) {
103+
return true, nil, fmt.Errorf("mock API error: pods not available")
104+
})
105+
106+
// Setup data collector with error-prone client
107+
tmpDir := t.TempDir()
108+
var logOutput bytes.Buffer
109+
110+
dc := &data_collector.DataCollector{
111+
BaseDir: tmpDir,
112+
Namespaces: []string{"default", "test-namespace"},
113+
Logger: log.New(&logOutput, "", 0),
114+
K8sCoreClientSet: mockClient,
115+
}
116+
117+
// Execute the pod-list job
118+
var podJob []Job
119+
jobs := CommonJobList()
120+
for _, job := range jobs {
121+
if job.Name == "pod-list" || job.Name == "collect-pods-logs" {
122+
podJob = append(podJob, job)
123+
}
124+
}
125+
// podJob := jobs[0] // First job is pod-list
126+
127+
ctx := context.Background()
128+
ch := make(chan JobResult, 1)
129+
for _, job := range podJob {
130+
job.Execute(dc, ctx, ch)
131+
132+
result := <-ch
133+
134+
// Assertions
135+
logContent := logOutput.String()
136+
assert.Contains(t, logContent, "Could not retrieve pod list for namespace default")
137+
assert.Contains(t, logContent, "Could not retrieve pod list for namespace test-namespace")
138+
assert.Contains(t, logContent, "mock API error: pods not available")
139+
140+
// No files should be created since API calls failed
141+
assert.Empty(t, result.Files)
142+
assert.Nil(t, result.Error) // The job itself doesn't fail, just logs errors
143+
}
144+
}

pkg/mock/mock_data_collector.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ func SetupMockDataCollector(t *testing.T) *data_collector.DataCollector {
6868
t.Fatalf("Failed to load test objects: %v", err)
6969
}
7070

71-
client := fake.NewSimpleClientset(objs...)
71+
client := fake.NewClientset(objs...)
7272

7373
// Mock rest.Config
7474
restConfig := &rest.Config{

0 commit comments

Comments
 (0)