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

Added data dir parameter while creating etcd container #645

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

bhagyashree-sarawate
Copy link
Contributor

Hi Sneha/William,
Added the ETCD_DATA_DIR parameter while creating the etcd container
Please review.
Verified whether the ETCD_DATA_DIR is set after running the playbook as follows:

[root@csimbe06-b13 ansible_3par_docker_plugin]# docker ps -a
CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES
6888fc2fa394 hpestorage/legacyvolumeplugin:3.2-prerelease "/bin/sh -c ./pluginâ¦" 6 minutes ago Up 6 minutes plugin_container
b04e082ca756 quay.io/coreos/etcd:v2.2.0 "/etcd" 7 minutes ago Up 7 minutes 2379-2380/tcp, 4001/tcp, 0.0.0.0:23790->23790/tcp, 0.0.0.0:23800->23800/tcp, 7001/tcp, 0.0.0.0:40010->40010/tcp etcd
809a2091a15a hello-world "/hello" 5 weeks ago Exited (0) 5 weeks ago epic_feynman

[root@csimbe06-b13 ansible_3par_docker_plugin]# docker exec -it b04e082ca756 /etcd
2019-06-10 09:25:01.639397 I | flags: recognized and used environment variable ETCD_ADVERTISE_CLIENT_URLS=http://10.50.0.160:23790,http://10.50.0.160:40010
2019-06-10 09:25:01.639466 I | flags: recognized and used environment variable ETCD_DATA_DIR=/var/lib/etcd
2019-06-10 09:25:01.639491 I | flags: recognized and used environment variable ETCD_INITIAL_ADVERTISE_PEER_URLS=http://10.50.0.160:23800
2019-06-10 09:25:01.639502 I | flags: recognized and used environment variable ETCD_INITIAL_CLUSTER=etcd0=http://10.50.0.160:23800
2019-06-10 09:25:01.639539 I | flags: recognized and used environment variable ETCD_INITIAL_CLUSTER_STATE=new
2019-06-10 09:25:01.639580 I | flags: recognized and used environment variable ETCD_INITIAL_CLUSTER_TOKEN=etcd-cluster-1
2019-06-10 09:25:01.639598 I | flags: recognized and used environment variable ETCD_LISTEN_CLIENT_URLS=http://0.0.0.0:23790,http://0.0.0.0:40010
2019-06-10 09:25:01.639610 I | flags: recognized and used environment variable ETCD_LISTEN_PEER_URLS=http://0.0.0.0:23800
2019-06-10 09:25:01.639632 I | flags: recognized and used environment variable ETCD_NAME=etcd0
2019-06-10 09:25:01.639724 I | etcdmain: etcd Version: 2.2.0
2019-06-10 09:25:01.639739 I | etcdmain: Git SHA: e4561dd
2019-06-10 09:25:01.639747 I | etcdmain: Go Version: go1.5
2019-06-10 09:25:01.639756 I | etcdmain: Go OS/Arch: linux/amd64
2019-06-10 09:25:01.639765 I | etcdmain: setting maximum number of CPUs to 32, total number of available CPUs is 32
2019-06-10 09:25:01.639850 N | etcdmain: the server is already initialized as member before, starting as etcd member...
2019-06-10 09:25:01.640081 C | etcdmain: listen tcp 0.0.0.0:23800: bind: address already in use

Copy link
Collaborator

@wdurairaj wdurairaj left a comment

Choose a reason for hiding this comment

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

Sorry. I noticed this late.

There should be a volume mapping like

docker run \
  -p 2379:2379 \
  -p 2380:2380 \
  --volume=${DATA_DIR}:/etcd-data \
  --name etcd ${REGISTRY}:latest \
  /usr/local/bin/etcd \
  --data-dir=/etcd-data --name node1 \

Your code change, only changes the --data-dir using the environment variable, meaning inside the etcd container the data of cluster is preserved.. but we need the backup on the etcd host, so a volume mapping will be required which is --volume=${DATA_DIR}:/etcd-data \

Request to add a volume mapping .

@wdurairaj
Copy link
Collaborator

Request to reference issue number in git commit message, so there will be an update on the corresponding bug as well

Eg.
git commit -m "Issue #611 -- added --data-dir as part of etcd container creation"

Copy link
Collaborator

@wdurairaj wdurairaj left a comment

Choose a reason for hiding this comment

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

Looks good for me. Let me see if Sneha has any comments.

@prablr79
Copy link
Contributor

@sneharai4 can you quickly check and merge this change ?

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.

3 participants