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

docker stack deploy failed if volume name with special character like "@" used in the compose file #33529

Closed
lipingxue opened this issue Jun 5, 2017 · 24 comments

Comments

@lipingxue
Copy link

Description

I am trying to use vSphere Docker Volume Service with "docker stack deploy" command to deploy a stack with the following compose file.

version: '3'

services:
   db:
     image: mariadb
     deploy:
      restart_policy:
        condition: on-failure
        delay: 5s
        max_attempts: 3
        window: 120s
      placement:
        constraints:
          - node.role == worker
     environment:
       MYSQL_ROOT_PASSWORD: rootpasswd
       MYSQL_USER: wp
       MYSQL_PASSWORD: wppasswd
       MYSQL_DATABASE: wp
     volumes:
       - mariadb@sharedVmfs-0:/var/lib/mysql

   web:
     image: wordpress:latest
     deploy:
      restart_policy:
        condition: on-failure
        delay: 5s
        max_attempts: 3
        window: 120s
     depends_on:
       - db
     ports:
       - "8080:80"
     environment:
       WORDPRESS_DB_USER: wp
       WORDPRESS_DB_PASSWORD: wppasswd
       WORDPRESS_DB_HOST: db:3306
       WORDPRESS_DB_NAME: wp

volumes:
   mariadb@sharedVmfs-0:
     driver: vsphere
     driver_opts:
       size: 1Gb
       diskformat: zeroedthick
                                                            

In that compose file, the volume is created by vSphere Docker Volume Service, and has the format like vol_name@datastore (mariadb@sharedVmfs-0 in the compose file). docker stack deploy failed with error like:

"mariadb@sharedVmfs-0 Additional property mariadb@sharedVmfs-0 is not allowed".

Steps to reproduce the issue:

  1. setup a swarm cluster with three VMs(1 master + 2 worker), each VMs install the vSphere Docker Volume Service
  2. run docker stack deploy -c docker-compose-vsphere.yml wordpress in the master node to deploy the stack
  3. Actually, this problem is not related to the specific volume plugin, and should be reproducible if the name of the volume used in the compose file includes special character like "@"

Describe the results you received:

root@sc-rdops-vm02-dhcp-52-237:~# docker stack deploy -c docker-compose-vsphere.yml wordpress
mariadb@sharedVmfs-0 Additional property mariadb@sharedVmfs-0 is not allowed

It looks to me that volume name including special characters like "@" cannot be used in the compose file.

Describe the results you expected:
I expect docker stack deploy works when the name of volume includes special character like "@".

I know one of the workaround is to pre-create the volume mariadb@sharedVmfs-0 , and then mark the volume as "external" in the compose file like this:

volumes:
   mariadb:
     external:
        name:  mariadb@sharedVmfs-0

It seems that most docker API can support volume name with special characters for volumes created by volume plugin, for example, I can create the volume mariadb@sharedVmfs-0 - using the volume plugin:

root@sc-rdops-vm02-dhcp-52-237:~# docker volume create --driver=vsphere --name=mariadb@sharedVmfs-0 -o size=1gb
mariadb@sharedVmfs-0
root@sc-rdops-vm02-dhcp-52-237:~# 
root@sc-rdops-vm02-dhcp-52-237:~# 
root@sc-rdops-vm02-dhcp-52-237:~# docker volume ls
DRIVER              VOLUME NAME
vsphere:latest      mariadb@sharedVmfs-0

So I think docker stack deploy should also work with volume name which includes special character like "@".

Additional information you deem important (e.g. issue happens only occasionally):

Output of docker version:

root@sc-rdops-vm02-dhcp-52-237:~# docker version
Client:
 Version:      17.03.1-ce
 API version:  1.27
 Go version:   go1.7.5
 Git commit:   c6d412e
 Built:        Fri Mar 24 00:40:33 2017
 OS/Arch:      linux/amd64

Server:
 Version:      17.03.1-ce
 API version:  1.27 (minimum version 1.12)
 Go version:   go1.7.5
 Git commit:   c6d412e
 Built:        Fri Mar 24 00:40:33 2017
 OS/Arch:      linux/amd64
 Experimental: false

Output of docker info:

root@sc-rdops-vm02-dhcp-52-237:~# docker info
Containers: 0
 Running: 0
 Paused: 0
 Stopped: 0
Images: 4
Server Version: 17.03.1-ce
Storage Driver: aufs
 Root Dir: /var/lib/docker/aufs
 Backing Filesystem: extfs
 Dirs: 25
 Dirperm1 Supported: true
Logging Driver: json-file
Cgroup Driver: cgroupfs
Plugins: 
 Volume: local
 Network: bridge host macvlan null overlay
Swarm: active
 NodeID: jov3lt7f7nd6h646zhu08or77
 Is Manager: true
 ClusterID: rzpzvmjzmzfrzy570hc7wwpe6
 Managers: 1
 Nodes: 3
 Orchestration:
  Task History Retention Limit: 5
 Raft:
  Snapshot Interval: 10000
  Number of Old Snapshots to Retain: 0
  Heartbeat Tick: 1
  Election Tick: 3
 Dispatcher:
  Heartbeat Period: 5 seconds
 CA Configuration:
  Expiry Duration: 3 months
 Node Address: 10.192.88.178
 Manager Addresses:
  10.192.88.178:2377
Runtimes: runc
Default Runtime: runc
Init Binary: docker-init
containerd version: 4ab9917febca54791c5f071a9d1f404867857fcc
runc version: 54296cf40ad8143b62dbcaa1d90e520a2136ddfe
init version: 949e6fa
Security Options:
 apparmor
Kernel Version: 4.2.0-27-generic
Operating System: Ubuntu 14.04.4 LTS
OSType: linux
Architecture: x86_64
CPUs: 2
Total Memory: 3.86 GiB
Name: sc-rdops-vm02-dhcp-52-237
ID: NINK:HV2F:IRM2:6JJU:XBVA:DW7J:CV5S:OARB:J4UL:O7S3:4LBA:7AVE
Docker Root Dir: /var/lib/docker
Debug Mode (client): false
Debug Mode (server): false
Registry: https://index.docker.io/v1/
WARNING: No swap limit support
Experimental: false
Insecure Registries:
 127.0.0.0/8
Live Restore Enabled: false

Additional environment details (AWS, VirtualBox, physical, etc.):
Ubuntu VMs running on ESX

@lipingxue
Copy link
Author

@GordonTheTurtle, is there someone already looked into this issue? I can help to implement the fix. Can you point me someone who can shed some light on how to fix this issue? Thanks!

@AkihiroSuda
Copy link
Member

This behavior is expected

$ docker volume create a@b
Error response from daemon: create a@b: "a@b" includes invalid characters for a local volume name, only "[a-zA-Z0-9][a-zA-Z0-9_.-]" are allowed. If you intended to pass a host directory, use absolute path

Also, @GordonTheTurtle is a bot 🐢

@thaJeztah
Copy link
Member

@AkihiroSuda unfortunately it's not that simple; the a-zA-Z0-9 naming restriction applies only to the "local" volume driver, but not to plugins. So on a standalone daemon, volume names including such characters are allowed for volumes if they are using a plugin (the plugin then determines the accepted characters).

However, I know that for Swarm-mode resources some additional restrictions apply (e.g. overlay networks with a . in their name are not accepted in swarm-mode, because the . is reserved for future namespacing of resources).

I'm not sure if the same applies to volumes in swarm-mode, so will have to check.

ping @cpuguy83 @stevvooe PTAL

@cpuguy83
Copy link
Member

cpuguy83 commented Jun 8, 2017

Yes, it looks like the yaml schema definition has a restriction on the volume name.
This should not be restricted in the yaml.

@lipingxue
Copy link
Author

@cpuguy83 Could you point me where is the yaml schema definition in the moby repo? I cannot find it. I can only find it in old docker repo https://github.com/docker/cli/blob/3d58c3feaccc71512e692f27c18c109d7b262281/cli/compose/schema/data/config_schema_v3.3.json#L33-L42

@cpuguy83
Copy link
Member

cpuguy83 commented Jun 8, 2017

@lipingxue That is the correct place. The CLI lives in the docker/cli repo.

@lipingxue
Copy link
Author

@cpuguy83 So the fix should be done on the docker/cli repo, right?
Should I reopen the issue I filed with docker/compose(docker/compose#4886) or use this issue when I implement the fix? Thanks!

@cpuguy83
Copy link
Member

cpuguy83 commented Jun 8, 2017 via email

@lipingxue
Copy link
Author

lipingxue commented Jun 21, 2017

@cpuguy83 I have a question about this fix. After removing the restriction in the YAML file, https://github.com/docker/cli/blob/3d58c3feaccc71512e692f27c18c109d7b262281/cli/compose/schema/data/config_schema_v3.3.json#L33-L42, I also want to apply the check to make sure when user uses local driver to create volume in YAML file, the volume name can only be "[a-zA-Z0-9][a-zA-Z0-9_.-]". So I need to add check if the driver is local, and the volume name specified by user is not "[a-zA-Z0-9][a-zA-Z0-9_.-]", an error need to be returned.

I mean some check like this

if driver == local && volume_name is not [a-zA-Z0-9][a-zA-Z0-9_.-] {
    return error("create %s includes invalid characters for a local volume name, only "[a-zA-Z0-9][a-zA-Z0-9_.-]" are allowed.", volume_name)
}

My question is where is the right place to put this check?

  1. I have never submit a fix in docker/cli repo, so I need someone to help me with the review/checkin process, such as
  • how to build and install the docker CLI with the fix, and then I can verify in my local setup
  • what is a required test for a PR

Who is the better person from docker/cli maintainer to contact for this issue?

@cpuguy83
Copy link
Member

@lipingxue The YAML does not need to restrict names here, IMO.

@lipingxue
Copy link
Author

@dnephin I search the PR history in docker/cli repo, and it seems that you have worked on volume spec parse, which is may related to this issue that I plan to work on. So could you shed some lights on the fix of this issue? Thanks!

@dnephin
Copy link
Member

dnephin commented Jun 21, 2017

The problem is that these names are internal references to volumes within the compose file, not just volume names.

What's wrong with using an external volume?

@lipingxue
Copy link
Author

lipingxue commented Jun 21, 2017

@dnephin Thanks for your quick response.
The problem is our customer who used vDVS along with swarm does not want to pre-create the volume and then use the volume as the external volume in the compose file. For detail, please refer this issue vmware-archive/vsphere-storage-for-docker#1315

Our customer wants to specify the volume name to be created in the YAML file and let the swarm service create the volume.

AFAIK, docker volume create command does not have limitation on the volume name which is created by volume plugin (plugin will decide whether the name is a valid one or not).
I just want to understand why in the compose file, it has the limitation on the volume name. In my opinion, the limitation should only apply to volume created by local driver (which is in sync with docker volume create ). Is there any specific reason behind that? Is it possible to make the fix to make sure the limitation only apply to volume created by local driver?

@lipingxue
Copy link
Author

@thaJeztah Could you shed some lights on this issue? I know using external volume in the compose file is a workaround, but it just a workaround. Other docker command such as docker volume create does not have limitations on the name of volume created by volume plugin, why docker compose put those limitation, which does not make sense to me. Thanks!

@thaJeztah
Copy link
Member

thaJeztah commented Jun 23, 2017

Hm, so looking at the issue, the problem indeed seems to be that the volumes section has the internal docker-compose names (i.e. references in the stack), these references have a restriction. The problem is a bit that the same reference also duplicate (by default) as part of the actual name of the object that's generated (i.e., having a volume reference named foo, the actual volume that's created will be mystackname_foo)

In this case, the actual volume that's created would thus become mystackname_mariadb@sharedVmfs-0 (I'm not sure that's your intent?)

So, possibly the name should be made customizable, and allowing users to specify a hard-coded name for a volume; there's one pitfall though, and that is that running multiple instances of the stack would lead to the same volume being shared by all instances (similar to having it defined as "external").

This would look something like this;

version: "3.2"
services:
  web:
    image: "nginx:alpine"
    volumes:
      - foo:/some/path
volumes:
  foo:
    name: mariadb@sharedVmfs-0
    driver: vsphere
    driver_opts:
      size: 1Gb
      diskformat: zeroedthick

@dnephin do you think that would make sense?

For now, there's actually a limitation/issue/bug that volumes defined as "external" are actually created if they do not exist when the stack is deployed (see the bug report here: #29976). So the following compose file actually creates a volume named i-am-external when the stack is deployed;

version: "3.2"
services:
  web:
    image: "nginx:alpine"
    volumes:
      - foo:/some/path
volumes:
  foo:
    external:
      name: i-am-external

This may help you (for now), but may change in future so should not be relied on.

@lipingxue
Copy link
Author

lipingxue commented Jun 26, 2017

@thaJeztah @dnephin Yes, I would like to have some way that users are able to specify a hard-coded volume, like the following.

version: "3.2"
services:
  web:
    image: "nginx:alpine"
    volumes:
      - foo:/some/path
volumes:
  foo:
    name: mariadb@sharedVmfs-0
    driver: vsphere
    driver_opts:
      size: 1Gb
      diskformat: zeroedthick

Just want to confirm with you, the above YAML file does not work with the current docker stack deploy, right? I tested with the following YAML file, and I got the error like

root@sc-rdops-vm02-dhcp-52-237:~# docker  stack deploy -c /root/postgres.yml postgres
name Additional property name is not allowed

root@sc-rdops-vm02-dhcp-52-237:~# cat postgres.yml 
version: "3"
 
services:
 
  postgres:
    image: postgres
    ports:
      - "5432:5432"
    volumes:
      - postgres_vol:/var/lib/data
    environment:
      - "POSTGRES_PASSWORD=secretpass"
      - "PGDATA=/var/lib/data/db"
    deploy:
      restart_policy:
        condition: on-failure
      placement:
        constraints:
          - node.role == worker
volumes:
   postgres_vol:
      name: postgres_vol@vsanDatastore
      driver: "vsphere"
      driver_opts:
        size: "1GB"

@lipingxue
Copy link
Author

I also tried to make the change in compose schema json file.
I changed the regex under "patternProperties" from "^[a-zA-Z0-9.-]+$" to "^[a-zA-Z0-9.-@]+$"

"volumes": {
      "id": "#/properties/volumes",
      "type": "object",
      "patternProperties": {
        "^[a-zA-Z0-9._-]+$": {
          "$ref": "#/definitions/volume"
        }
      },
      "additionalProperties": false
    },

After making the change to regex, I think compose should be able to parse volume name like "postgres_vol@vsanDatastore". But when I try docker stack deploy, I still get the errors like the following.

root@sc-rdops-vm02-dhcp-52-237:~# ./tmp_with_fix/docker-linux-amd64 stack deploy -c /root/postgres.yml postgres
postgres_vol@vsanDatastore Additional property postgres_vol@vsanDatastore is not allowed

Why it complains "postgres_vol@vsanDatastore" is additonal property? Could you shed some lights?

@thaJeztah
Copy link
Member

Why it complains "postgres_vol@vsanDatastore" is additonal property? Could you shed some lights?

The format/parser currently doesn't expect a name: property, so will complain about that. Also any changes to the schema also need to be re-generated (the code itself does not use the .json file at runtime, but a variable that's included in the Go source code; https://github.com/docker/cli/blob/e574286ba2b5ff602912410fddb7324c50c208ec/cli/compose/schema/bindata.go#L134

@lipingxue
Copy link
Author

lipingxue commented Jun 26, 2017

@thaJeztah Thanks for the quick response.
What do you mean the "schema also need to be re-generated"? How can I make it "re-generated"? In theory, after I making the regex to "^[a-zA-Z0-9.-@]+$", it should be able to specify volume name which include "@", right?

Do you think the change I made is the right one, and I just need some way to "re-generated" after making the change in schema or the approach I take is totally wrong?

What I need is to let user be able to specify volume name like "postgres_vol@vsanDatastore" in the YAML file and they can use this YAML file with docker stack deploy command.

@thaJeztah
Copy link
Member

The "postgres_vol@vsanDatastore" most likely cannot be used as key / identifier in the YAML file, that's why I suggested accepting a name: parameter for "non-external" volumes.

@lipingxue
Copy link
Author

@thaJeztah Why "postgres_vol@vsanDatastore" cannot be used as key/identifier in the YAML file? Any specific reason?
I will try to explore the option you point out which accept a name: parameter for "non-external" volume.

Another question is say if I modify the schema file, how can I "re-generated" the variable which included in the Go source code? Do I need to use any specific tool to do that?
If my understanding is correct, after making changes in the schema, I need use some tools to generate the new value of variable var _dataConfig_schema_v33Json, and then manually replace the value of this varaible in binddata.go, right? Thanks!

cc @dnephin @cpuguy83

@thaJeztah
Copy link
Member

Another question is say if I modify the schema file, how can I "re-generated" the variable which included in the Go source code? Do I need to use any specific tool to do that?

If I recall correctly; in the docker/cli repository, run;

go generate github.com/docker/cli/cli/compose/schema

@dnephin
Copy link
Member

dnephin commented Jun 28, 2017

+1 for the suggestion from @thaJeztah to move name: out of external and allow it to be set for non-external as well. We should deprecate the name under external and warn when it's set.

To re-generate the schema:

# start an interactive container
make -f docker.Makefile shell
# In the container 
make cli/compose/schema/bindata.go

go generate github.com/docker/cli/cli/compose/schema in the container will also work (it's the same thing).

@dnephin
Copy link
Member

dnephin commented Jun 30, 2017

Thanks. I created docker/cli#274 to add support for a custom name for non-external volumes.

@dnephin dnephin closed this as completed Jun 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants