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

Migrate from ubuntu to alpine:3.6 #5

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

Conversation

Siassei
Copy link

@Siassei Siassei commented Nov 18, 2017

  • add build.sh
  • migrate to alpine linux, fixed version to 3.6
  • fix latest version in shell script
  • both, docker container and shell script is migrated to alpine linux
  • all changes are tested with a fresh docker installation and an project that contains two named volumes

request was started at #4

add bug fix for line

volumes=($(docker volume ls -f name=$project_name | awk '{if (NR > 1) print $2}'))

the output of an container with two volumes was

# volumes=($(docker volume ls -f name=nextcloud | awk '{if (NR > 1) print $2}'))
# echo $volumes
nextcloud_nextcloud-postgres-data

but there are two volumes

# docker volume ls -f name=nextcloud | awk '{if (NR > 0) print $2}'
VOLUME
nextcloud_nextcloud-postgres-data
nextcloud_postgres-data

after the fix

# declare -a volumes=()
# readarray -t volumes < <(docker volume ls -f name=$project_name | awk '{if (NR > 1) print $2}')
# echo $volumes
nextcloud_nextcloud-postgres-data
nextcloud_postgres-data

@Siassei
Copy link
Author

Siassei commented Nov 19, 2017

add a extended script docker_full_backup.sh for an full backup. For an fast and safe restore the containers should also saved.

  • actually backup/restore all containers and volumes
  • compressed tar files are ended with .tar.gz
  • /backup contains subfolder for containers and volumes
  • new script does not collide with old script/data

@Siassei
Copy link
Author

Siassei commented Nov 20, 2017

so, all things should work fine. Please let my know if you found something :)

@kiview
Copy link
Owner

kiview commented Nov 28, 2017

Cool, thanks :)
I'll look into it this week.

RUN apk add py-pip
RUN apk add bash
RUN apk add gzip
RUN rm -rf /var/cache/apk/*
Copy link
Owner

Choose a reason for hiding this comment

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

We should squash these layers into a single layer (so only one RUN statement). I know it wasn't done in the original version, but that's how it should be done ;)

BACKUP_DIR=/tmp/mybackups
DATE=xxxx-xx-xx

#export EXCLUDE_CONTAINER=1
Copy link
Owner

Choose a reason for hiding this comment

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

Should not be commented in example.

./docker_volume_backup.sh /home/kiview/Gitlab/docker-compose.yml gitlab $(pwd)/backup restore 2016-10-19
```

### docker_full_backup.sh

Copy link
Owner

Choose a reason for hiding this comment

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

Missing introductory text which explains, that the following code should reside in its own bash script.

you can use it like this:

```bash
PROJECT_DIR # path to directory that contains the docker files, e.g. docker-compose.yml, Dockerfile, ...
Copy link
Owner

Choose a reason for hiding this comment

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

Same as above, this will only work if the example is in a bash file. The old examples could be run directly from the command line.

echo " container are excluded"
else
echo " enter container images"
#declare -a containers=()
Copy link
Owner

Choose a reason for hiding this comment

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

Please remove commented code.

echo " volumes are excluded"
else
echo " mounting volumes and performing backup/restore..."
#declare -a volumes=()
Copy link
Owner

Choose a reason for hiding this comment

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

Please remove commented code.

volumes=($(docker volume ls -f name=$project_name | awk '{if (NR > 1) print $2}'))
for v in "${volumes[@]}"
do
#declare -a volumes=()
Copy link
Owner

Choose a reason for hiding this comment

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

Please remove commented code.

@kiview
Copy link
Owner

kiview commented Nov 28, 2017

Would it be possible to have two different PRs? The first one for switching to alpine and another for adding the full backup functionality?

And since a lot of code of docker_volume_backup.sh seems to be duplicated in docker_full_backup.sh, it might be better to merge the files.

Also please explain to me the use case of docker_full_backup.sh 🙂

@Siassei
Copy link
Author

Siassei commented Nov 30, 2017

sorry, I'm very busy. I will fix and explain that in a week.
But thanks for the comments!

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.

2 participants