feat(container): update, restart y stats

[gedera]

Causa raíz

DockerSwarm::Container no implementa update, restart ni stats, y la ausencia no se nota — lib/docker_swarm/base.rb:158-160:

elsif !method_str.end_with?("?")
  _define_dynamic_accessor(method_str)
  nil

Cualquier método desconocido que no termine en ? define un accessor y devuelve nil, sin NoMethodError. Medido:

c.start    respond_to?=true   → implementado
c.stop     respond_to?=true   → implementado
c.logs     respond_to?=true   → implementado

c.update   respond_to?=true   → devolvió nil   (NO levantó)
c.restart  respond_to?=true   → devolvió nil   (NO levantó)
c.stats    respond_to?=true   → devolvió nil   (NO levantó)

respond_to? también miente, así que un consumidor defensivo que chequee antes de llamar recibe lo mismo. Aguas abajo, el containers_controller del BCM serializa ese nil y responde 200 OK con cuerpo nulo (sequre/box_cluster_manager#43).

Cuatro decisiones, y ninguna es copiar al hermano

① No se incluye Concerns::Updatable. Está escrito para services de Swarm: manda query_params: { version: current_version } (updatable.rb:26) para el control de concurrencia optimista. El POST /containers/{id}/update no tiene version — es otro endpoint, con otra semántica (límites de recursos). Incluirlo mandaría un query param que el Engine ignora en silencio.

② update absorbe kwargs a propósito, y esto es lo menos obvio del PR. Concerns::Creatable#save:32 hace:

return update(registry_auth: registry_auth) if persisted?

Con la firma natural update(new_attributes = {}), ese kwarg se convierte en Hash posicional en Ruby 3 — medido:

def update(new_attributes = {})          →  update(registry_auth: nil)  da  {registry_auth: nil}
def update(new_attributes = {}, **opts)  →  lo separa:  [{}, {registry_auth: nil}]

O sea que save sobre un container persistido postearía {"registry_auth": null} al Engine, sin error. Basura silenciosa: el mismo tipo de bug que este PR viene a cerrar, introducido al cerrarlo. Los dos de registry se descartan porque son cosa de Service.

Y update devuelve el cuerpo, no un booleano. Docker responde {"Warnings": [...]} y ahí avisa cuando un límite no se pudo aplicar; colapsarlo a true se comería justamente esa señal.

③ restart no copia a Service#restart. El hermano simula el restart incrementando ForceUpdate (service.rb:40-43) porque los services de Swarm no tienen endpoint de restart. Los containers sí. Copiarlo sería arrastrar un workaround que acá no hace falta.

④ stats fuerza stream: false, y por eso el método vuelve. Medido contra Engine 29.7.2:

GET /containers/{id}/stats                  exit 28 (timeout) · 6 objetos en 6s · la conexión NO cierra
GET /containers/{id}/stats?stream=false     exit 0 · 1 objeto · 1s · cierra sola

En una llamada RPC el default cuelga — y el modo de falla no es un error, es una espera. Mismo patrón que ADR-027: el default de Docker no es el que sirve.

El parámetro se mergea en vez de reemplazarse, a diferencia de Loggable#logs. Ahí un caller que pasa su propio hash sólo cambia qué streams lee; acá lo dejaría colgado. Se puede pisar a propósito (stats(stream: true)), pero no por accidente.

Evidencia — contra un Engine real, con efecto semántico

No alcanza con el 200: se verificó que hicieran lo que prometen.

Método	HTTP	Query enviada	Devolvió	Efecto verificado
stats	200	{stream: false}	Hash con memory_stats.usage, en 1.0s	no colgó
restart	204	{t: 1}	true	StartedAt 13:36:06.844 → 13:36:08.255
update	200	{}	{"Warnings" => nil}	Memory 67108864 → 134217728

ruby -c        Syntax OK  (los 2 archivos)
rubocop        2 files inspected, no offenses detected

Qué NO prueba

• La suite de la gema todavía no corrió, ni hay tests nuevos: este PR sale con la solución sola para revisar la forma. Van en el commit siguiente.
• Un solo Engine (29.7.2, arm64-darwin). El comportamiento de stats sin stream podría diferir en otras versiones; lo que sí es estable es que el default del spec es stream=true.
• update se probó con Memory/MemorySwap. No se ejercitaron los demás campos (NanoCpus, RestartPolicy, …) ni el caso donde el Engine sí devuelve warnings.

Alcance

• lib/docker_swarm/api.rb — 3 rutas nuevas en containers:.
• lib/docker_swarm/models/container.rb — los 3 métodos.

Qué NO hace: no toca Base, ni el method_missing que es la causa de fondo del silencio (eso es superficie compartida por los 11 modelos y merece su propia decisión); no trae tests ni incremento de doc todavía; y no incluye el release ni los bumps, que son #40.

Corrección de un dato del issue de origen: decía que Api::ENDPOINTS[:containers] declaraba 5 rutas. Son 7 — omitía index y show. No cambia el trabajo.

Closes #39
Part of #30

[gedera]

Review multi-modelo — 5 revisaron, 4 findings, los 4 CONFIRMED y los 4 aplicados

claude/sonnet · codex · opencode-bigpickle · opencode-glm · agy. Con la evidencia contra el Engine adjunta y cinco archivos que el PR no toca incluidos como lectura (base.rb, creatable.rb, updatable.rb, loggable.rb, service.rb) — sin ellos las cuatro decisiones del PR son irrevisables.

Los cuatro hallazgos son de este PR, y el grave muestra algo incómodo: al hacer que update tolerara el kwarg de save, abrí un agujero nuevo.

	Finding	Acuerdo	Verify	Qué se hizo
F1 🔴	save sobre persistido postea {} y pierde los cambios locales en silencio	3/5	✅	levanta ArgumentError
F2 🟠	save devolvía el Hash del Engine, rompiendo su contrato booleano	3/5	✅	se resuelve por F1
F3 🟠	update dejaba el estado local stale	2/5	✅	reload tras el 200
F4 🟡	el except filtraba sólo opts → una credencial posicional viajaba al payload y al log	1/5	✅	el except va sobre el merge

F1 — el que más duele, porque lo introduje al arreglar otra cosa

Creatable#save:32 llama update(registry_auth:) sin atributos. Tras el except el payload quedaba en {}, y el Engine acepta el no-op. Verificado, porque dos revisores lo marcaron como supuesto:

POST /containers/{id}/update  con body {}
  → http=200 · {"Warnings":null} · Memory sin cambiar (67108864 antes y después)

Así que c.Memory = X; c.save perdía el cambio sin un solo error. Antes de este PR devolvía nil por method_missing — no es regresión, pero tuve la oportunidad de cerrarlo y en cambio lo hice parecer que funcionaba, que es peor.

El save genérico no se soporta, a propósito. POST /containers/{id}/update no es "guardar el objeto": es un endpoint angosto de límites de recursos. Derivar el payload de los atributos locales —la otra opción que propuso un revisor— exigiría una whitelist de campos que driftea contra la API. Mejor decirlo que fingirlo, y el mensaje del error redirige a update("Memory" => …).

(Verificado antes de elegir: nadie llama .save sobre containers — ni los consumidores ni los specs de la gema. Hacerlo fallar fuerte no rompe a nadie.)

F3 — donde me aparté de lo que proponían los revisores

Sugerían assign_attributes, que es lo que hace Concerns::Updatable. Acá sería incorrecto: el payload de update es plano (Memory) y el objeto lo tiene anidado (HostConfig.Memory), así que asignarlo crearía un atributo fantasma en vez de actualizar el real. Se usa reload, que trae la forma correcta del Engine y es el mismo cierre que ya usa save al crear.

Evidencia de los cuatro, contra Engine 29.7.2

save sobre persistido    → ArgumentError con el mensaje que redirige   (antes: 200 OK silencioso)
credencial posicional    → filtrada por el except del merge, no viajó
update camino feliz      → HostConfig.Memory 67108864 → 134217728, sin reload manual
                           (el GET del reload se ve en el log, justo tras el POST)
rubocop                  → no offenses

Lo que quedó abierto y declarado

• Base#method_missing sigue intacto. Es la causa de fondo del silencio y la comparten los 11 modelos: Network, Volume y Secret también incluyen Creatable sin Updatable, así que su save sobre persistido cae en el mismo nil. Merece decisión propia, no un arreglo de contrabando en este PR.
• Un supuesto que ningún revisor pudo cerrar: cómo serializa Api.request las claves String en query_params. Si las pasa tal cual, stats("stream" => true) duplicaría el parámetro junto al default. No lo verifiqué.
• Sigue sin tests y sin incremento de doc — van después del OK humano.

[gedera]

por standard esto? no deberia de estar en ingles?

[gedera]

Medí la convención antes de cambiar, porque no la tenía clara: de los 5 mensajes de error de la gema, cuatro están en inglés (assign_attributes expects a Hash…, Docker socket error:…, HTTP #{status}:…, image pull failed:…) y uno en español — registry_auth y registry_auth_from son mutuamente excluyentes, que entró en 0.8.0. Copié al outlier.

Tenés razón: corregido a inglés en afeec1b.

La distinción que me queda, y la aplico de acá en adelante: prosa explicativa en español —los comentarios de container.rb, updatable.rb y el CHANGELOG lo son— pero superficie pública en inglés. Un mensaje de excepción es superficie pública: lo lee quien consume la gema, no quien la mantiene.

Verificado cómo sale:

Container#update requires at least one attribute. An empty payload gets a 200 OK from the
Engine and applies nothing. Coming from `save`? Containers do not support the generic save:
use `update("Memory" => ...)` with explicit resource limits.

No toqué el outlier de 0.8.0 — es preexistente y de otro cambio.