Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

info: add Features field to show feature-configuration #43880

Closed
wants to merge 1 commit into from

Conversation

thaJeztah
Copy link
Member

This adds a new Features field for API v1.43 and up, which contains the current
configuration of features that have been either enabled or disabled in the daemon
configuration.

With this patch:

mkdir -p /etc/docker
echo '{"features":{"containerd-snapshotter":true}}' > /etc/docker/daemon.json
dockerd &

curl -s  --unix-socket /var/run/docker.sock http://localhost/info | jq .Features
[
  "containerd-snapshotter=enabled"
]

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

This adds a new Features field for API v1.43 and up, which contains the current
configuration of features that have been either enabled or disabled in the daemon
configuration.

With this patch:

```bash
mkdir -p /etc/docker
echo '{"features":{"containerd-snapshotter":true}}' > /etc/docker/daemon.json
dockerd &

curl -s  --unix-socket /var/run/docker.sock http://localhost/info | jq .Features
[
  "containerd-snapshotter=enabled"
]
```

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Copy link
Member Author

I should add that this shows the configuration currently; wondering if we would need to change this to show the current status (e.g. for features that are enabled by default)

@thaJeztah
Copy link
Member Author

We discussed this in the maintainers meeting;

  • the only two options currently available are buildkit and containerd-snapshotter
  • the buildkit option will become less (non) relevant soon (also see api: set default "Builder-Version" to "2" (BuildKit) on Linux #43657), but also is already exposed through the API (as builder-version)
  • for the containerd-snapshotter, it might be more relevant to show;
    • the default snapshotter used for containerd (which basically can replace the GraphDriver field)
    • the list of available / supported snapshotter plugins (as detected by containerd), which may be more relevant to users
  • having a list of features therefore currently has no real use-case, other than for the test-case in emptyfs: Use newer image layout rumpl/moby#31, which can also be addressed by the GraphDriver field if we would propagate that.

Moving this back to draft, and I'll have a look at exposing the information as GraphDriver and/or GraphDriver metadata instead

@thaJeztah thaJeztah marked this pull request as draft July 28, 2022 20:08
@AkihiroSuda
Copy link
Member

Just having labels (with a prefix like com.docker.engine.features/) might be enough for this?

@rumpl
Copy link
Member

rumpl commented Aug 1, 2022

having a list of features therefore currently has no real use-case, other than for the test-case in

One real use case for the features list is for buildx: it can use this information to know if its docker driver can support multi-platform images

//
// If a certain feature doesn't appear in this list then it's unset
// (i.e. neither true nor false).
Features []string `json:"Features,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

Any reason this is a []string with a micro-format instead of a map[string]bool?

Copy link
Contributor

Choose a reason for hiding this comment

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

either map[string]bool or map[string]string (for status, enabled| disabled | ..)
but please don't use yet another string microformat within a JSON API

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, this was a 5 minute, quick implementation for discussion in the maintainers call; we did discuss using a map instead (possibly with custom marshaling to return a consistent order in the API response)

But then we also discussed if we really wanted this, because

  • these options don't reflect the current state of these features (only if they're overridden)
  • for the builder version; this is already exposed through other ways
  • for the snapshotters, it'll be more useful to know what snapshotter is used (and what other snapshotters are available); both of those would effectively replace what "graphdriver" is currently showing, so the preference was to explore using that instead.

w.r.t. buildx; I wonder if that's information that should be part of the buildkit API 🤔 (/cc @crazy-max )

@thaJeztah
Copy link
Member Author

closing in favour of #43983

@thaJeztah thaJeztah closed this Aug 22, 2022
containerd integration automation moved this from To do to Done Aug 22, 2022
@thaJeztah thaJeztah deleted the daemon_info_features branch August 22, 2022 17:56
@thaJeztah thaJeztah removed this from the v-next milestone Aug 24, 2022
@thaJeztah thaJeztah removed this from Done in containerd integration Aug 24, 2022
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.

None yet

4 participants