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

Don't loose additional metadata fields when read/writing contexts metadata #2572

Merged

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