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

Abierto
#6,780 0 comentarios 0 reacciones 0 asignados Ver en GitHub

Los mantenedores suelen responder en 2 días

Nadie ha tomado este issue todavía.

  • #6921 de @AruneshDwivedi — cerrado sin fusionar

Evaluación

Dificultad
4/5
Tiempo estimado
3-5 días
Aptitud para principiantes
48/100
Tipo de issue
Error
Claridad
Bastante claro
Estado de actividad
Tranquilo
Stack tecnológico
go

Línea de trabajo

Empieza con ListAppHeadDeployments en pkg/app/pipedv1/apistore/deploymentstore/store.go:125-134 y la implementación correspondiente en pkg/app/piped/apistore/deploymentstore/store.go. Sigue dónde se consume el método para determinar si el head debe ser el deployment no completado más antiguo o más reciente, y después añade la prueba de regresión con dos deployments descrita en el issue para ambos stores. Se considera terminado cuando la implementación y el docstring de Lister coinciden.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

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
Lenguaje dominante
Go
Estrellas
1.4k
Forks
391
Merge medio
4 d 1 h
PR fusionados (30 d)
58

Preparar el entorno

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de pipe-cd/pipecd

Todos los issues de pipe-cd/pipecd

Issues similares

Más issues de Go

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.