Skip to content

Commit d8a6188

Browse files
fix: respect pipeline config in Kubernetes planner for workload-less apps
When a Kubernetes application contains only ConfigMaps (no Deployments or other workloads), decideStrategy() was immediately returning QUICK_SYNC because findWorkloadManifests() found nothing to inspect. This caused the pipeline defined in app.pipecd.yaml to be silently ignored. Fix: pass pipelineDefined bool to decideStrategy(). When the user has explicitly configured a pipeline and no workloads are found, use progressive sync instead of falling back to QUICK_SYNC. Also added unit tests covering: - ConfigMap-only app with pipeline defined -> progressive sync - ConfigMap-only app without pipeline defined -> QUICK_SYNC (no regression) Fixes: #4799 Signed-off-by: Mihir Dixit <dixitmihir1@gmail.com>
1 parent 8ac9041 commit d8a6188

2 files changed

Lines changed: 76 additions & 3 deletions

File tree

pkg/app/piped/planner/kubernetes/kubernetes.go

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,8 @@ func (p *Planner) Plan(ctx context.Context, in planner.Input) (out planner.Outpu
214214
manifestCache.Put(in.MostRecentSuccessfulCommitHash, oldManifests)
215215
}
216216

217-
progressive, desc := decideStrategy(oldManifests, newManifests, cfg.Workloads, in.Logger)
217+
pipelineDefined := cfg.Pipeline != nil && len(cfg.Pipeline.Stages) > 0
218+
progressive, desc := decideStrategy(oldManifests, newManifests, cfg.Workloads, pipelineDefined, in.Logger)
218219
out.Summary = desc
219220

220221
if progressive {
@@ -230,14 +231,26 @@ func (p *Planner) Plan(ctx context.Context, in planner.Input) (out planner.Outpu
230231

231232
// First up, checks to see if the workload's `spec.template` has been changed,
232233
// and then checks if the configmap/secret's data.
233-
func decideStrategy(olds, news []provider.Manifest, workloadRefs []config.K8sResourceReference, logger *zap.Logger) (progressive bool, desc string) {
234+
// pipelineDefined should be true when the user has explicitly configured a pipeline
235+
// in the application config, which forces progressive sync even when no workloads exist.
236+
func decideStrategy(olds, news []provider.Manifest, workloadRefs []config.K8sResourceReference, pipelineDefined bool, logger *zap.Logger) (progressive bool, desc string) {
234237
oldWorkloads := findWorkloadManifests(olds, workloadRefs)
235238
if len(oldWorkloads) == 0 {
239+
if pipelineDefined {
240+
progressive = true
241+
desc = "Sync progressively because pipeline is defined in the application config"
242+
return
243+
}
236244
desc = "Quick sync by applying all manifests because it was unable to find the currently running workloads"
237245
return
238246
}
239247
newWorkloads := findWorkloadManifests(news, workloadRefs)
240248
if len(newWorkloads) == 0 {
249+
if pipelineDefined {
250+
progressive = true
251+
desc = "Sync progressively because pipeline is defined in the application config"
252+
return
253+
}
241254
desc = "Quick sync by applying all manifests because it was unable to find workloads in the new manifests"
242255
return
243256
}

pkg/app/piped/planner/kubernetes/kubernetes_test.go

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -408,7 +408,67 @@ func TestDecideStrategy(t *testing.T) {
408408
}
409409
for _, tc := range tests {
410410
t.Run(tc.name, func(t *testing.T) {
411-
gotProgressive, gotDesc := decideStrategy(tc.olds, tc.news, tc.workloadRefs, zap.NewNop())
411+
gotProgressive, gotDesc := decideStrategy(tc.olds, tc.news, tc.workloadRefs, false, zap.NewNop())
412+
assert.Equal(t, tc.wantProgressive, gotProgressive)
413+
assert.Equal(t, tc.wantDesc, gotDesc)
414+
})
415+
}
416+
}
417+
418+
func TestDecideStrategyWithPipelineDefined(t *testing.T) {
419+
t.Parallel()
420+
421+
configMapManifest := func(name, data string) provider.Manifest {
422+
return provider.MakeManifest(provider.ResourceKey{
423+
APIVersion: "v1",
424+
Kind: provider.KindConfigMap,
425+
Name: name,
426+
}, &unstructured.Unstructured{
427+
Object: map[string]interface{}{"data": data}},
428+
)
429+
}
430+
431+
tests := []struct {
432+
name string
433+
olds []provider.Manifest
434+
news []provider.Manifest
435+
pipelineDefined bool
436+
wantProgressive bool
437+
wantDesc string
438+
}{
439+
{
440+
// Regression test for: https://github.com/pipe-cd/pipecd/issues/4799
441+
// ConfigMap-only app (no Deployment) with pipeline defined in app.pipecd.yaml
442+
// must use progressive sync, not QUICK_SYNC.
443+
name: "configmap-only app with pipeline defined should be progressive",
444+
olds: []provider.Manifest{
445+
configMapManifest("my-config", "old-value"),
446+
},
447+
news: []provider.Manifest{
448+
configMapManifest("my-config", "new-value"),
449+
},
450+
pipelineDefined: true,
451+
wantProgressive: true,
452+
wantDesc: "Sync progressively because pipeline is defined in the application config",
453+
},
454+
{
455+
// Without a pipeline defined, ConfigMap-only app must still use QUICK_SYNC
456+
// (no regression in the default/non-pipeline path).
457+
name: "configmap-only app without pipeline defined should be quick sync",
458+
olds: []provider.Manifest{
459+
configMapManifest("my-config", "old-value"),
460+
},
461+
news: []provider.Manifest{
462+
configMapManifest("my-config", "new-value"),
463+
},
464+
pipelineDefined: false,
465+
wantProgressive: false,
466+
wantDesc: "Quick sync by applying all manifests because it was unable to find the currently running workloads",
467+
},
468+
}
469+
for _, tc := range tests {
470+
t.Run(tc.name, func(t *testing.T) {
471+
gotProgressive, gotDesc := decideStrategy(tc.olds, tc.news, nil, tc.pipelineDefined, zap.NewNop())
412472
assert.Equal(t, tc.wantProgressive, gotProgressive)
413473
assert.Equal(t, tc.wantDesc, gotDesc)
414474
})

0 commit comments

Comments
 (0)