Skip to content

Add ENGINE column driven by label - #13959

Open
nicksieger wants to merge 1 commit into
docker:mainfrom
nicksieger:ps-engine-column
Open

Add ENGINE column driven by label#13959
nicksieger wants to merge 1 commit into
docker:mainfrom
nicksieger:ps-engine-column

Conversation

@nicksieger

@nicksieger nicksieger commented Jul 21, 2026

Copy link
Copy Markdown
Member

What I did

Show an ENGINE column in compose ps default table output when the com.docker.compose.engine label is present in any containers. Column is omitted otherwise. Expose {{.Engine}} for custom --format templates.

Supports future multi-engine coordinators and workloads.

Related issue

(not mandatory) A picture of a cute animal, if possible in relation to what you did

@nicksieger
nicksieger requested review from a team as code owners July 21, 2026 21:09
@nicksieger
nicksieger requested review from glours and ndeloof July 21, 2026 21:10

@docker-agent docker-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assessment: 🟢 APPROVE

The ENGINE column feature is clean and correct. The implementation:

  • Correctly guards against nil Labels maps in the new Engine() method — no panic risk for containers without labels
  • Only shows the ENGINE column when at least one container carries the com.docker.compose.engine label
  • The hasEngine detection loop correctly reflects the filtered container set
  • Format string construction order (ENGINE before SIZE) is internally consistent
  • New test covers the happy-path scenario

No bugs were found in the added code.

@codecov

codecov Bot commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@glours glours left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hasEngine scans the filtered container set (cmd/compose/ps.go), so column presence depends on which services you name. Say svc1 carries the com.docker.compose.engine label and svc2 doesn't:

  • compose ps → ENGINE column shown (svc2 gets an empty cell)
  • compose ps svc2no ENGINE column, even though svc2's row is identical to the one just shown
  • compose ps svc1 → shown again

Same rows, different schema. That breaks table-parsing and diverges from docker ps, where columns are never data-gated.

As this is a very specific display, could we gate it on the coordinator context (the same context-metadata detection as the project-config work, living under internal/) instead of the data? Presence then stays stable across ps / ps <subset>.

Nit: api.EngineLabel (com.docker.compose.engine) collides by name with desktop.EngineLabel (com.docker.desktop.address), can we consider a more specific name?

@glours
glours requested a review from docker-agent July 22, 2026 15:36

@docker-agent docker-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assessment: 🟢 APPROVE

The ENGINE column feature is well-structured. The new internal/coordinator package correctly gates the feature on Docker context metadata, the label constant matches the documented name (com.docker.compose.engine), and error paths (HTTP timeouts, non-200 responses, missing coordinator) are handled. Test coverage is present for the new coordinator logic, the formatter, and compose ps.

No high- or medium-severity bugs were found in the changed code.

@glours glours left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, maybe will need some refactoring depending of pending comments on #13958

Comment thread cmd/formatter/container_test.go Outdated
assert.Assert(t, strings.Contains(out.String(), "moby"), out.String())
})

t.Run("engine column hidden when no label present", func(t *testing.T) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
t.Run("engine column hidden when no label present", func(t *testing.T) {
t.Run("engine column hidden when coordinateur disabled", func(t *testing.T) {

@nicksieger nicksieger changed the title Add ENGINE column driven by com.docker.compose.engine label Add ENGINE column driven by coordinator context Jul 28, 2026
ndeloof
ndeloof previously approved these changes Aug 10, 2026
Show an ENGINE column in `compose ps` default table output when the
com.docker.compose.engine label is present. Expose {{.Engine}} for custom
--format templates.

Rename api.EngineLabel to api.ContainerEngineLabel to avoid the
name collision with desktop.EngineLabel (com.docker.desktop.address).

Signed-off-by: Nick Sieger <nick@nicksieger.com>
@nicksieger nicksieger changed the title Add ENGINE column driven by coordinator context Add ENGINE column driven by label Aug 14, 2026
@thaJeztah

Copy link
Copy Markdown
Member

Non-blocking, because compose is already heavily depending on labels to store state, but we should start looking if we could transition to using Annotations, especially for state that shouldn't be persisted (doing a docker commit on a container, or running a container from an image that already has labels set could probably still provide "interesting" results);
https://github.com/moby/moby/blob/7e1098f068c282ef9346c861fba0d7fc6b90c4a0/api/types/container/hostconfig.go#L415-L430

// HostConfig the non-portable Config structure of a container.
// Here, "non-portable" means "dependent of the host we are running on".
// Portable information *should* appear in Config.
type HostConfig struct {
	// Applicable to all platforms
	Binds           []string          // List of volume bindings for this container
	ContainerIDFile string            // File (path) where the containerId is written
	LogConfig       LogConfig         // Configuration of the logs for this container
	NetworkMode     NetworkMode       // Network mode to use for the container
	PortBindings    network.PortMap   // Port mapping between the exposed port (container) and the host
	RestartPolicy   RestartPolicy     // Restart policy to be used for the container
	AutoRemove      bool              // Automatically remove container when it exits
	VolumeDriver    string            // Name of the volume driver used to mount volumes
	VolumesFrom     []string          // List of volumes to take from other container
	ConsoleSize     [2]uint           // Initial console size (height,width)
	Annotations     map[string]string `json:",omitempty"` // Arbitrary non-identifying metadata attached to container and provided to the runtime

We may have to look for some parts w.r.t. filtering though, especially for the compose project the object is associated with (which, may not always be "runtime state", so maybe labels are more OK for that); for project we should definitely look if containerd namespaces would be a viable option (and add namespaces to other objects)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants