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

Create backup script on control nodes, add metadata for backupninja #19

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

avlasov-mos-de
Copy link

List of related PRs coming later

fs_excludes:
- /var/backups/cassandra/OPENCONTRAIL_CONTROL_DB/*
fs_includes:
- /var/backups/cassandra/OPENCONTRAIL_CONTROL_DB/{{ system.name }}.{{ system.domain }}/
Copy link
Member

Choose a reason for hiding this comment

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

Use grains instead or pillar that comes for these states.

NODETOOL=$(which nodetool)

TODAY_DATE=$(date +%F)
NODENAME=$(salt-call pillar.get linux:network:fqdn --out=newline_values_only)
Copy link
Member

@fpytloun fpytloun May 15, 2017

Choose a reason for hiding this comment

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

Use grains instead or better just hostname -f and rely on correctly set hostname. Dependency on other formulas is not necessary.

SNAPSHOT_NAME=snp-$(date +%F-%H%M-%S)
DATE_SCHEMA=$(date +%F-%H%M-%S)

DB_BIND_HOST=$(salt-call pillar.get opencontrail:database:bind:host --out=newline_values_only)
Copy link
Member

Choose a reason for hiding this comment

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

You are rendering this file as jinja template so you can reference {{ opencontrail.database.bind.host }} directly without need to call salt.

@@ -0,0 +1,7 @@
/usr/bin/cassandra_backup:
Copy link
Member

@fpytloun fpytloun May 15, 2017

Choose a reason for hiding this comment

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

Use /usr/local/bin instead. It's also better to use dash (I would name it contrail-cassandra-backup) than underscore.

declare -a KEYSPACE_LIST=( $( ${CQL_SH} -e "DESC KEYSPACES" | awk '{RS="\\s+"; if(NF>0){print}}' | sort ) )

# Make sure backup Directory exists
mkdir -p "${BACKUP_SCHEMA_DIR}"
Copy link
Member

Choose a reason for hiding this comment

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

You shouldn't fail if it already exists, use this instead:

[ -d "${BACKUP_SCHEMA_DIR" ] || mkdir -p "${BACKUP_SCHEMA_DIR}"

Copy link
Author

Choose a reason for hiding this comment

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

mkdir won't fail if -p key is given

Copy link
Member

Choose a reason for hiding this comment

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

I would rather manage that dirs by backup.sls in opencontrail formula. (rights/content, etc..)

Copy link
Member

@fpytloun fpytloun left a comment

Choose a reason for hiding this comment

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

See my comments above.

@slimakcz
Copy link

Hi Filip,
I think avlasov rework code due to your comments. Can you please check again and merge it?
Thank you so much.

@@ -0,0 +1,58 @@
{% from "opencontrail/map.jinja" import database with context %}
Copy link
Member

Choose a reason for hiding this comment

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

You should strip empty lines by using {%- and -%}

Copy link
Member

Choose a reason for hiding this comment

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

And this file doesn't need to be a jinja template at all.

@@ -0,0 +1,7 @@
/usr/local/bin/contrail-cassandra-backup:
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer not introducing new state (cassandra_backup.sls) but having this as a part of opencontrail.control

NODETOOL=$(which nodetool)

TODAY_DATE=$(date +%F)
NODENAME="{{ node_fqdn }}"
Copy link
Member

Choose a reason for hiding this comment

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

Use $(hostname -f) instead

Copy link
Member

@fpytloun fpytloun left a comment

Choose a reason for hiding this comment

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

See 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.

4 participants