Skip to content

Conversation

@simonferquel
Copy link
Contributor

- What I did

Make the cli accept additional (untyped) fields in its contexts metadata. Primary goal is to unlock interoperability with our Docker experience for Azure ACI work, which needs to add some metadata into docker contexts to work properly.

The fact that additional fields where not supported previously is actually a bug I introduced when introducing "typed endpoints and contexts metadata".

- How I did it

DockerContext struct now has a custom implementation of json.Marshaler and json.Unmarshaler, keeping addition fields in a map.

- How to verify it

  • The PR comes with a synthetic test
  • Create a context with docker context create test --from=default
  • Modify the metadata file (~/.docker/contexts/meta/9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08/meta.json) to add fields in the "metadata" object such as {"Name":"test","Metadata":{"StackOrchestrator":"swarm","AdditionalKey":"Value"},"Endpoints":{"docker":{"Host":"unix:///var/run/docker.sock","SkipTLSVerify":false}}}
  • docker context inspect test with the modification should have the AdditionalKey field in its output.
  • docker context update test --description=foo should not drop the AdditionalKey field anymore.

- Description for the changelog

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

@simonferquel
Copy link
Contributor Author

cc @rumpl @gtardif

Copy link
Member

@rumpl rumpl left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@silvin-lubecki silvin-lubecki left a comment

Choose a reason for hiding this comment

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

LGTM

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants