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

Module/docker #1646

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

Module/docker #1646

wants to merge 7 commits into from

Conversation

akarzim
Copy link
Contributor

@akarzim akarzim commented Nov 28, 2018

Proposed Changes on Docker aliases

Added

  • new alias to send kill -s HUP signal to a running container
  • new aliases to fetch and follow the logs of a container
  • new alias to pull all tagged images

Fixed

  • force terminal geometry on dkE alias

Copy link
Collaborator

@indrajitr indrajitr left a comment

Choose a reason for hiding this comment

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

Mostly looks good. @akarzim, would you consider the suggestions inlined?

modules/docker/README.md Outdated Show resolved Hide resolved
modules/docker/README.md Outdated Show resolved Hide resolved
modules/docker/README.md Outdated Show resolved Hide resolved
modules/docker/alias.zsh Show resolved Hide resolved
@@ -16,13 +16,15 @@ alias dkb='docker build'
alias dkd='docker diff'
alias dkdf='docker system df'
alias dke='docker exec'
alias dkE='docker exec -it'
alias dkE='docker exec -e COLUMNS=`tput cols` -e LINES=`tput lines` -it'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why was this modified? I understand what it does, but this shouldn't be needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My coworkers and me often encounter weird behaviour on interactive terminal, lines wrap at 80 characters and overlaps themselves. This workaround fix this issue.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Does it do the same thing when using the docker-compose binds below? Would it be worth adding there as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

docker-compose exec doesn't allow interactive terminal, but docker container exec does. As well as docker run and docker container run. I'll add the environment variables there too.

Copy link
Collaborator

@belak belak Jan 3, 2019

Choose a reason for hiding this comment

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

I think you're mistaken. I just double checked and docker-compose exec is interactive and has a pseudo-tty allocated by default. It would be nice to have those variables there as well.

Otherwise, it looks good to me!

~ % cat docker-compose.yml
version: '2'
services:
  mysql:
    image: mysql:5.7
    ports:
     - "3306:3306"
    environment:
      MYSQL_ROOT_PASSWORD: "secret"
      MYSQL_DATABASE: "homestead"
      MYSQL_USER: "homestead"
      MYSQL_PASSWORD: "secret"
    volumes:
     - mysqldata:/var/lib/mysql
volumes:
  mysqldata:
~ % docker-compose up -d
belak_mysql_1 is up-to-date
~ % docker-compose exec mysql bash
root@3012bf149158:/# ls
bin  boot  dev	docker-entrypoint-initdb.d  entrypoint.sh  etc	home  lib  lib64  media  mnt  opt  proc  root  run  sbin  srv  sys  tmp  usr  var
root@3012bf149158:/# exit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're totally right ! Though, you may need to include the following lines in your docker-compose.yml to enable interactive mode in some environments:

stdin_open: true
tty: true

The first corresponds to -i in docker run and the second to -t.

source: https://stackoverflow.com/questions/36249744/interactive-shell-using-docker-compose#39150040

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants