-
Notifications
You must be signed in to change notification settings - Fork 522
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
Docker Command Customization #1596
Comments
@philliphoff thank you for putting this together, great proposal overall. A couple of suggestions:
HTH! |
@karolz-ms Thanks for the feedback!
I intended the tokens to be similar to VS Code's notion of tokens in the
I considered this (and am not opposed to a separate Perhaps we could do a (limited) emulation of VS Code's command's {
"docker.commands.logs": [
{
"label": "Match Redis Containers",
"when": "image == redis",
},
{
"label": "Match Redis-like-named Containers",
"when": "name ~= /redis/"
}
]
} I don't really want to go down the path of supporting complex statements like VS Code, however (e.g. boolean expressions), so I think I'd want to limit it to just one equality/match expression. |
@philliphoff makes sense to do simple substitution on tokens in the command; I see the note on that in the proposal 👍 With regards to matching, the only thing that I would emphasize is the separation of the label and the match property/properties. I do believe it will save us quite a bit of grief down the line. Having a single |
Coming from #1496, @philliphoff, I think that this is an interesting proposal in the good direction, but I'm afraid that it does not face the issues that force me to keep using Docker's CLI. Having some templates defined in Tha main idea in the screencast shown in #1496 is that templates are first-class citizens in the GUI, just as containers, images and registries. This is because of how thoughts come to the user. Sometimes users want to run a specific container and need to guess/try which options to use. However, in other contexts, users know what options they need to use, but they need to guess/try multiple containers. In the current proposal, it feels that templates are hidden in the GUI, as these are defined in the From this point of view, I don't think the syntax for saving the templates is relevant, as long as GUI features are provided to manipulate them easily. Nonetheless, I believe that using arrays instead of a single string might make it easier to work with: {
"label": "Build with Node 12",
"template": [
"docker",
"build",
"-f",
"${dockerfile}",
"-r",
"${tag}",
"--build-arg",
"NODE_VERSION=12.14.1",
"${context}"
]
} BTW, it would make sense to have configurations defined declaratively: {
"label": "Build with Node",
"template": [
"docker",
"build",
"-f",
"${dockerfile}",
"-r",
"${tag}",
"--build-arg",
"NODE_VERSION=${nodeVerion}",
"${context}"
],
"configurations": {
"12": {
"nodeVersion": "12.14.1",
},
"13": {
"nodeVersion": "13.7.0",
}
}
} The "configurations" object can be updated with "latest used configurations" dynamically. However, this strategy might conflict with the
I think that absolute paths should also be supported.
IMHO, this (and any other parameter) should be optionally customizable from the GUI, just before executing the command.
I believe that:
By the same token, Actually, I think that this is the main limiting factor for my use cases. My main requirement is to run images with exposed volumes and ports which are not defined in the Dockefile/image, and which are mostly a one-time setting. The number of shared volumes and ports is not fixed.
As above, I think that Of course, I completely agree with your proposal being a great improvement compared to the current state. Please, take these comments as constructive criticism about where I'd like to go, not as a demand. |
Another issue I forgot to comment is that the main command might not be |
@umarcor Thank you for your thoughts!
|
Thank you for building/maintaining this piece of software! This extension was one of the main reasons that pushed me to start using VSC. It is not as good as I'd like yet, but the layout, and the UX were really good from the begining. It was easy to visualise how it can be non-intrusively extended.
I think this is important, and it should not be forgotten, as you say. Nevertheless, it is not prioriraty at all. Neither for v0.11.0, nor for v0.12.0 or... I'd prefer to have the GUI well thought than quickly done. In case you didn't see it yet, this is a prototype I did: https://user-images.githubusercontent.com/38422348/70940519-cdc30400-204a-11ea-91cd-260f97054cb2.gif I didn't implement it as a PR because I could not build this extension. However, I found my limit with regard to VSC's API and internal feature set. Hence, I'd be glad to somehow contribute to have that reused, should it be reusable at all.
Note that when I said that latest configurations can be updated dynamically, I meant to have them added, not selected. So, if I select a base template/configuration and I choose to customise some field before launching it, that should/could be added to the list of known/existing configurations for that template. Maybe this needs some more elaboration. With customise I mean two possibilities:
In both cases, the interaction would happen through the dropdown menu in the center (I don't know how is it called). Actually, this text-menu based interaction can be a temporal workaround for the lack of a GUI.
Glad to know!
Agree! |
Ah, I think I understand what you mean now. Would it make sense to separate "Customize" from "Execute", i.e. what users do to "Execute" doesn't really change from what there is today, but add a command for "Customize", that could open the config UI or take them to the settings.json, etc.? |
I hit one speed bump during implementation with the compose commands. The extension allows for no compose file to be specified, in which case it just runs |
@bwateratmsft Alternatively we could have two settings/command templates, one used when compose file is specified, the other when it is not |
Another option would be to put the |
Yeah, as I think about it that makes more sense. |
Sure! |
Overview
The Docker extension executes a number of Docker CLI commands on behalf of its users, such as to build images, run containers, attach to containers, and view container logs. Some of these commands have a large number of optional arguments, often used in very specific scenarios, and our users have stated a need be able to augment or override arguments passed to commands executed by the extension.
To date, the ability to customize the commands executed by the extension has been added piecemeal, with the customization often just appended to the command line generated by the extension. While this is often sufficient, it doesn't allow the user to significantly alter the command, for example, to change arguments or flags set by the extension itself, which may be necessary in a number of user scenarios.
This proposal is intended to:
Non-goals
Some Docker interaction is performed via an SDK rather than via the command lines. For example, container start/stop/restart/remove/inspect/prune. Customization of those commands may be considered in a future proposal as it would likely require a different customization scheme, if and when there is demand for it.
Related issues (not an exhaustive list)
#794
#807
#1197
#1274
#1349
#1505
#1590
#1593
General Proposal
For each Docker command (and variant), for example, build, run, attach, and view logs, the extension will:
Define a configuration setting that represents a "template" of the command line to be executed.
The default value of this setting will represent a complete command line for that Docker command (generally corresponding to that currently generated and executed by the extension).
The setting may support a set of "tokens" that represent values generated by the extension, which, if present, are substituted with their representative value at the location of the token within the template.
The set of tokens supported by a template may vary based on the Docker command.
More than one template may be defined for each command; in such cases, each
template
property may be associated with alabel
and/or amatch
property.If a
match
property is specified, those commands supporting heuristic-based selection will use that value to match against values appropriate to the command (for example, the container or image name).If multiple templates specify a
label
property and no template was otherwise selected based on theirmatch
properties, those templates will be included in the list (built from the templates'label
properties) from which the user will be prompted to select a template.If no templates were selected based on their
match
properties and fewer than 2 remaining templates have an associatedlabel
property, the first such template will be selected.If no templates were selected based on their
match
properties and no other templates have been specified, then the default template for that setting will be selected.Settings JSON Schema
VS Code will prompt the user to edit the
settings.json
file when adding or editing these settings. Customizing a command, at a minimum, is just assigning the corresponding setting name a string value that represents the Docker command line to execute. For example, to customize the Docker build command to set the build argumentNODE_VERSION
to12.14.1
:In cases where a single template is not sufficient, multiple templates can be specified by assigning the corresponding setting name an array of template objects. For example, to provide two customizations of the Docker build command, to set the
NODE_VERSION
build argument to differing values:In that scenario, upon invoking the build command, the extension will prompt the user to select a template to use from a list of the template labels.
Suppose the user wants to show all logs for Redis containers but otherwise limit logs for other containers to the last 10 lines. The user could define two templates, one matching
redis
and another template that serves as the default for all others (by not specifying amatch
orlabel
property).Docker Build
Current Behavior
When building an image, the following command is used:
The
<dockerfile>
value is the workspace-relative path of the selectedDockerfile
. The<context>
value is derived from thedocker.imageBuildContextPath
setting or, if not set, defaults to the workspace-relative folder in which theDockerfile
resides.The
<tag>
value is ultimately entered/confirmed by the user when invoking the build command. However, if previously built, it defaults to the previously-entered value for thatDockerfile
. Otherwise, it defaultsto
<folder>:latest
where<folder>
is the parent folder of theDockerfile
.Proposed Behavior
The new behavior will be to offer the following setting:
docker.commands.build
docker build --rm -f "${dockerfile}" -t ${tag} "${context}"
This setting will support the following tokens:
${context}
docker.imageBuildContextPath
setting. Otherwise, the workspace-relative folder in which theDockerfile
resides.${dockerfile}
Dockerfile
.${tag}
Dockerfile
. Otherwise, defaults to<folder>:latest
where<folder>
is the parent folder of theDockerfile
.Docker Run
Current Behavior
When running an image, the following commands are used:
The
<tag>
value is the full tag of the selected image. The<ports>
value is generated from the list of exposed ports in the image (i.e. ultimately from theDockerfile
), where each exposed port is mapped to the same port on the local machine. For example,"EXPOSE 5000 5001"
would generate"-p 5000:5000 -p 5001:5001"
.For the Azure CLI, the
<network>
value is--net=host
on Linux but not set on other platforms. The<volumes>
value is a mapping of volumes in the container to the user's local.azure
,.ssh
, and.kube
folders (so simplify remote connections to Azure resources). For example,"-v /Users/<user>/.azure:/root/.azure -v /Users/<user>/.ssh:/root/.ssh -v /Users/<user>/.kube:/root/.kube"
.Proposed Behavior
The new behavior will be to offer the following settings:
docker.commands.run
docker run --rm -d ${exposedPorts} ${tag}
docker.commands.runInteractive
docker run --rm -it ${exposedPorts} ${tag}
docker.commands.runAzureCli
docker run ${network} ${userVolumes} -it --rm azuresdk/azure-cli-python:latest
These settings will support the following tokens:
${exposedPorts}
Dockerfile
), where each exposed port is mapped to the same port on the local machine. For example,"EXPOSE 5000 5001"
would generate"-p 5000:5000 -p 5001:5001"
.${tag}
${network}
--net=host
. Not set for other platforms.${userVolumes}
.azure
,.ssh
, and.kube
folders (so simplify remote connections to Azure resources). For example,"-v /Users/<user>/.azure:/root/.azure -v /Users/<user>/.ssh:/root/.ssh -v /Users/<user>/.kube:/root/.kube"
.Template Matching
For non-Azure CLI run commands, if there are multiple templates associated with the command, the extension will first look for a template with a
label
property matching the tag of the image being run. If found, that template will be used. If not found, then the user will be prompted to select from the set of templates.Docker Attach
Current Behavior
When attaching to a running container, the following command is used:
The
<shellCommand>
value is derived from the value of either thedocker.attachShellCommand.windowsContainer
ordocker.attachShellCommand.linuxContainer
settings.The defaults for those settings are:
docker.attachShellCommand.linuxContainer
docker.attachShellCommand.windowsContainer
powershell
Proposed Behavior
The new behavior will be to offer the following setting:
docker.commands.attach
docker exec -it ${containerId} ${shellCommand}
This setting will support the following tokens:
${containerId}
${shellCommand}
docker.attachShellCommand.linuxContainer
ordocker.attachShellCommand.windowsContainer
setting, as appropriate.Template Matching
If there are multiple templates associated with the command, the extension will first look for a template with a
label
property matching the name of the container being attached to. If found, that template will be used. If not found, the extension will then look for a template with alabel
property matching the tag of the image of the container being attached to. If not found, then the user will be prompted to select from the set of templates.Docker Logs
Current Behavior
When viewing logs of a container, the following command is used:
Proposed Behavior
The new behavior will be to offer the following setting:
docker.commands.logs
docker logs -f ${containerId}
This setting will support the following tokens:
${containerId}
Template Matching
If there are multiple templates associated with the command, the extension will first look for a template with a
label
property matching the name of the container being attached to. If found, that template will be used. If not found, the extension will then look for a template with alabel
property matching the tag of the image of the container being attached to. If not found, then the user will be prompted to select from the set of templates.Docker Compose Up
Current Behavior
When bringing up a composition, the following command is used:
The
<file>
value is the workspace-relative path to the selected Docker Compose YAML file. The<detached>
value is set to-d
if the settingdocker.dockerComposeDetached
is set totrue
. The<build>
value is set to--build
if the settingdocker.dockerComposeBuild
is set totrue
.Proposed Behavior
The new behavior will be to offer the following setting:
docker.commands.composeUp
docker-compose ${configurationFile} up ${detached} ${build}
This setting will support the following tokens:
${configurationFile}
-f
plus the workspace-relative path to the selected Docker Compose YAML file.${detached}
-d
if the settingdocker.dockerComposeDetached
is set totrue
. Otherwise, set to""
.${build}
--build
if the settingdocker.dockerComposeBuild
is set totrue
. Otherwise, set to""
.Docker Compose Down
Current Behavior
When taking down a composition, the following command is used:
docker-compose -f "<file>" down
Proposed Behavior
The new behavior will be to offer the following setting:
docker.commands.composeDown
docker-compose ${configurationFile} down
This setting will support the following tokens:
${configurationFile}
-f
plus the workspace-relative path to the selected Docker Compose YAML file.The text was updated successfully, but these errors were encountered: