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.

Sign up for GitHub

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

Jump to bottom

Feature backup / restore #1359

Merged
merged 32 commits into from
Jun 29, 2020
Merged

Feature backup / restore #1359

merged 32 commits into from
Jun 29, 2020

Conversation

sk4zuzu
Copy link
Contributor

@sk4zuzu sk4zuzu commented Jun 16, 2020

No description provided.

@sk4zuzu sk4zuzu marked this pull request as draft June 16, 2020 14:26
This was linked to issues Jun 16, 2020
Backup: Elasticsearch and Kibana #884
Closed
Epicli backup implementation #1185
Closed
Backup: RabbitMQ Configuration #1187
Closed
Backup: HAProxy #1188
Closed
Restore: Elasticsearch and Kibana #1197
Closed
Restore: Prometheus #1198
Closed
Epicli restore implementation #1199
Closed
Restore: HAProxy #1200
Closed
Restore: RabbitMQ Configuration #1285
Closed
@plirglo plirglo linked an issue Jun 16, 2020 that may be closed by this pull request
Restore: PostgreSQL #1195
Closed
@sk4zuzu sk4zuzu added this to the Backup/Restore milestone Jun 16, 2020
@sk4zuzu sk4zuzu marked this pull request as ready for review June 18, 2020 08:44
@seriva
Copy link
Collaborator

seriva commented Jun 18, 2020

Instead of passing in the complete cluster yml with added backup/restore configuration for the epicli recovery/backup commands I would suggest only to use the backup/restore configuration and get the rest of the cluster definition from the manifest.yml which is can be loaded from the build directory input.

This way its better seperated and its easier to use the same backup/restore configuration for many clusters.

seriva
seriva reviewed Jun 19, 2020
View reviewed changes
core/src/epicli/cli/engine/PatchEngine.py Outdated Show resolved Hide resolved
core/src/epicli/cli/engine/PatchEngine.py Outdated Show resolved Hide resolved
seriva
seriva reviewed Jun 19, 2020
View reviewed changes
core/src/epicli/data/common/defaults/configuration/prometheus.yml
@@ -15,6 +15,7 @@ specification:
- "--web.console.libraries=/etc/prometheus/console_libraries" # Directory should be the same as "config_directory"
- "--web.console.templates=/etc/prometheus/consoles" # Directory should be the same as "config_directory"
- "--web.listen-address=0.0.0.0:9090" # Address that Prometheus console will be available
- "--web.enable-admin-api" # Enables administrative HTTP API
Copy link
Collaborator

Choose a reason for hiding this comment

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

We need to enable this permanently?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not really I guess. The problem is that doing this "on-demand" requires modification of the systemd unit and in result full stop / start cycle. I wanted to avoid restarts...

mkyc
mkyc previously requested changes Jun 19, 2020
View reviewed changes
Copy link
Contributor

@mkyc mkyc left a comment

Choose a reason for hiding this comment

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

This kind of PR is impossible to review. My opinion is that this should get divided into 6 separated PR's each per one component. Also isn't some documentation missing?

core/src/epicli/data/common/ansible/playbooks/roles/recovery/tasks/kubernetes.yml Outdated Show resolved Hide resolved
core/src/epicli/data/common/defaults/configuration/backup.yml Outdated Show resolved Hide resolved
core/src/epicli/data/common/defaults/configuration/recovery.yml Outdated Show resolved Hide resolved
core/src/epicli/cli/helpers/doc_list_helpers.py Show resolved Hide resolved
core/src/epicli/data/common/ansible/playbooks/roles/recovery/tasks/common/upload_via_ssh.yml Outdated Show resolved Hide resolved
@sk4zuzu
Copy link
Contributor Author

sk4zuzu commented Jun 19, 2020

I believe documentation is being prepared by @plirglo, am I correct?

@sk4zuzu
Copy link
Contributor Author

sk4zuzu commented Jun 19, 2020 •
edited
Loading

@mkyc, I've removed some unneeded code in last few commits, I hope it helps with the review. 👍

@plirglo
Copy link
Contributor

plirglo commented Jun 22, 2020

@sk4zuzu Yes, it's in progress

mkyc
mkyc reviewed Jun 22, 2020
View reviewed changes
Copy link
Contributor

@mkyc mkyc left a comment

Choose a reason for hiding this comment

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

@sk4zuzu thanks for those cleanups.

sk4zuzu and others added 9 commits June 29, 2020 13:09
@sk4zuzu
backup/recovery: uncommenting and refactoring legacy code (#1241)
f0fbd03
@sk4zuzu
backup/recovery: adding ansible code for syncing files between hosts (#…
296535a 
…1257)

* backup/recovery: adding ansible code for syncing files between hosts

* backup/recovery: adding rsync to package requirements

* backup/recovery: fixing rsync code + adding random name for private key

* backup/recovery: adding ssh code (transfer via pipeline)

* backup/recovery/prometheus: enabling admin API + initial implementation
@sk4zuzu
backup/recovery: adding initial code for restoring monitoring snapsho…
02d4b55 
…ts (#1259)

* backup/recovery: moving sync code to roles + cleanups

* backup/recovery: adding initial code for restoring monitoring snapshots

* backup/recovery: adding kubernetes backup sync (copy-only)

* backup/recovery: checking if snapshots are available first
@sk4zuzu
backup/recovery: adding on-the-fly sha1 calculation (#1262)
f1ce62a 
* backup/recovery: adding on-the-fly sha1 calculation in the download_via_ssh copy method

* backup/recovery: fixing on-the-fly sha1 calculation

- monitoring: reverting invalid tar argument order
- download_via_ssh: making the inline_script fail properly
@plirglo @sk4zuzu
Postgres Backup Initial
b8ce249 
* postgres backup

* Add all config files to backup
@sk4zuzu
backup/recovery: monitoring refactor (#1270)
8d67110 
- switching from ssh to rsync
- adding grafana backup/restore
- adding prometheus etc backup/restore
@sk4zuzu
backup/recovery: adding loadbalancer (#1279)
cbf2f00
@sk4zuzu
backup/recovery: logging (WIP) (#1277)
e26dad8 
- work in progress, does not work with elasticsearch clusters yet
@sk4zuzu
backup/recovery: adding rabbitmq (without messages) (#1291)
dc6d8d4 
* backup/recovery: adding rabbitmq (without messages)

* backup/recovery: adding rabbitmq (archive fix)
sk4zuzu and others added 3 commits June 29, 2020 13:10
@sk4zuzu
Hotfix for the elasticsearch recovery procedure (#1381)
4eed450 
* backup/recovery: logging/elasticsearch snapshot restore fix

- ensuring kibana is always down
- deleting all indices prior to restore

* backup/recovery: logging/elasticsearch snapshot restore fix

- ensuring all filebeat instances are always down

* backup/recovery: logging/elasticsearch snapshot restore fix

- ensuring kibana and filebeat instances will not be started (via reboot) during restore
@sk4zuzu
backup/recovery: fixing load_balancer in RedHat (#1390)
3575a13
@plirglo @sk4zuzu
Fix database check (#1391)
d5a36ef
@sk4zuzu sk4zuzu force-pushed the feature/backup-restore branch from 70760a5 to d5a36ef Compare June 29, 2020 11:18
@sk4zuzu sk4zuzu dismissed mkyc’s stale review June 29, 2020 11:23

Incomplete review.

jetalone85
jetalone85 reviewed Jun 29, 2020
View reviewed changes
core/src/epicli/cli/engine/BackupRecoveryEngineBase.py
Comment on lines +67 to +89
"""Populate input yaml documents with additional required ad-hoc data."""

# Seed the self.configuration_docs
self.configuration_docs = copy.deepcopy(self.input_docs)

# Please notice using DefaultMerger is not needed here, since it is done already at this point.
# We just check if documents are missing and insert default ones without the unneeded merge operation.
for kind in {'configuration/backup', 'configuration/recovery'}:
try:
# Check if the required document is in user inputs
document = select_single(self.configuration_docs, lambda x: x.kind == kind)
except ExpectedSingleResultException:
# If there is no document provided by the user, then fallback to defaults
document = load_yaml_obj(data_types.DEFAULT, 'common', kind)
# Inject the required "version" attribute
document['version'] = VERSION
# Copy the "provider" value from the cluster model
document['provider'] = self.cluster_model.provider
# Save the document for later use
self.configuration_docs.append(document)
finally:
# Copy the "provider" value to the specification as well
document.specification['provider'] = document['provider']
Copy link
Contributor

Choose a reason for hiding this comment

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

Why we left so many comments in ready to use code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think comparing to other pojects we don't have enough comments 🤔. Which ones do you want me to remove?

Copy link
Contributor

Choose a reason for hiding this comment

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

No one. This is only generally question. I don't see many places commented like this. I suspect it's a new quality.

@rafzei rafzei self-requested a review June 29, 2020 11:51
@plirglo plirglo merged commit 6c4d140 into develop Jun 29, 2020
This was referenced Jun 29, 2020
Closed
Closed
@plirglo plirglo mentioned this pull request Jun 29, 2020
Closed
@toszo toszo mentioned this pull request Jun 29, 2020
Closed
This was referenced Jun 29, 2020
Closed
Closed
Closed
Closed
Closed
Closed
This was referenced Jun 29, 2020
Closed
Closed
@plirglo plirglo mentioned this pull request Jun 29, 2020
Closed
@sk4zuzu sk4zuzu deleted the feature/backup-restore branch October 9, 2020 10:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mkyc mkyc mkyc left review comments

@seriva seriva seriva left review comments

@jetalone85 jetalone85 jetalone85 approved these changes

@rafzei rafzei rafzei approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Backup/Restore
6 participants
@sk4zuzu @seriva @plirglo @jetalone85 @rafzei @mkyc