bug: pipedv1 deployment store returns newest deployment as head, not oldest

Aperta
  • Qualcosa non va?
#6,780 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

I maintainer di solito rispondono entro 2 giorni

Nessuno ha ancora preso questa issue.

  • #6921 di @AruneshDwivedi — chiusa senza merge

Valutazione

Difficoltà
4/5
Tempo stimato
3-5 giorni
Idoneità per principianti
48/100
Tipo di issue
Bug
Chiarezza
Abbastanza chiara
Stato di attività
Tranquilla
Stack tecnologico
go
Ambito
devops, infrastructure

Direzione di ricerca

Inizia da ListAppHeadDeployments in pkg/app/pipedv1/apistore/deploymentstore/store.go:125-134 e dall’implementazione corrispondente in pkg/app/piped/apistore/deploymentstore/store.go. Traccia dove viene usato il metodo per determinare se l’head debba essere il deployment non completato più vecchio o più recente, quindi aggiungi per entrambi gli store il test di regressione con due deployment descritto nell’issue. Il lavoro è completato quando l’implementazione e il docstring di Lister concordano.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

What happened:

While reviewing the deployment store sync logic in pipedv1, the head-deployment selection in pkg/app/pipedv1/apistore/deploymentstore/store.go:125-134 looks inverted relative to the Lister interface's docstring.

The interface promises:

ListAppHeadDeployments returns the map from application ID to its head of deploying deployments.
Head deployment is same with the oldest uncompleted one.

But the implementation writes pendings → planneds → runnings into the same map[string]*model.Deployment, so for an application with both a pending and a running deployment, the running one wins by overwrite order:

for _, d := range pendings { headDeployments[d.ApplicationId] = d }
for _, d := range planneds { headDeployments[d.ApplicationId] = d }
for _, d := range runnings { headDeployments[d.ApplicationId] = d }

A PENDING deployment is logically newer than a RUNNING one for the same application (it was created later, after the running one started), so this code returns the newest uncompleted deployment, not the oldest.

The same inversion exists at the corresponding location in the v0 store (pkg/app/piped/apistore/deploymentstore/store.go).

What you expected to happen:

Either:

  1. The docstring is updated to match the implementation ("head = newest uncompleted"), or
  2. The implementation is corrected to match the docstring — either reverse the write order or pick the head explicitly by created_at.

I don't know which side is correct, since I haven't traced where ListAppHeadDeployments is consumed downstream. Whichever way it ends up, the docstring and the code should agree.

How to reproduce it:

Unit test against the store with two *model.Deployment instances sharing one ApplicationId, one DEPLOYMENT_PENDING and one DEPLOYMENT_RUNNING. Lister.ListAppHeadDeployments()[appID] returns the running one — the opposite of what the docstring promises.

Environment:

  • piped version: master @ 78fdbeb7c (both v0 and v1 affected)
  • control-plane version: same
  • Others: surfaced during self-review of #6727 while writing tests for ListAppHeadDeployments
Lingua principale
Go
Stelle
1.4k
Fork
391
Merge medio
4g 1h
PR unite (30g)
58

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di pipe-cd/pipecd

Tutte le issue di pipe-cd/pipecd

Issue simili

Altre issue su Go

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.