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

Bug 1795235: DR: Backup should keep data and keys separate #1398

Merged

openshift-merge-robot merged 1 commit into openshift:master from retroflexer:backup-should-keep-data-and-keys-separate

Jan 24, 2020

Contributor

retroflexer commented Jan 21, 2020

- What I did
Backup should save static pod resources of kube-apiserver separate from the snapshot of etcd database. When etcd is encrypted, it is important to keep the data and keys separatedly for security.

Similarly, restore can be invoked on a backup directory containing files for the static pod resources along with the etcd database.
- How to verify it
Follow DR documentation to back up and restore. The cluster should functional after the same backup is restored on all masters.
- Description for the changelog

Save and restore kube apiserver's static pod resources separatedly from the etcd database.

openshift-ci-robot added the size/M label

openshift-ci-robot requested review from kikisdeliveryservice and yuqi-zhang

January 21, 2020 19:36

Contributor

kikisdeliveryservice commented Jan 21, 2020

/assign @hexfusion

openshift-ci-robot assigned hexfusion

Contributor Author

retroflexer commented Jan 21, 2020

/test e2e-aws-disruptive

sttts reviewed

View reviewed changes

templates/master/00-master/_base/files/usr-local-bin-etcd-snapshot-restore-sh.yaml Outdated Show resolved Hide resolved

Contributor

sttts commented Jan 22, 2020

Only some quotation nits. Otherwise, it looks good.

retroflexer force-pushed the backup-should-keep-data-and-keys-separate branch from e4d4344 to eb2474e Compare

January 22, 2020 09:58

Contributor Author

retroflexer commented Jan 22, 2020

@sttts Added quotation marks and braces.

sttts reviewed

View reviewed changes

templates/master/00-master/_base/files/usr-local-bin-etcd-snapshot-restore-sh.yaml Outdated Show resolved Hide resolved

retroflexer force-pushed the backup-should-keep-data-and-keys-separate branch from eb2474e to 158d174 Compare

January 22, 2020 10:12

Contributor

sttts commented Jan 22, 2020

/approve
/lgtm

openshift-ci-robot assigned sttts

openshift-ci-robot added the lgtm label

Contributor

hexfusion commented Jan 22, 2020 •

edited

level=fatal msg="failed to fetch Cluster: failed to generate asset "Cluster": failed to create cluster: >failed to apply using Terraform"

CI flake

/test e2e-aws

Contributor Author

retroflexer commented Jan 22, 2020

/test e2e-aws

1 similar comment

Contributor Author

retroflexer commented Jan 22, 2020

/test e2e-aws

Contributor Author

retroflexer commented Jan 22, 2020

/cherrypick release-4.3

openshift-cherrypick-robot commented Jan 22, 2020

@retroflexer: once the present PR merges, I will cherry-pick it on top of release-4.3 in a new PR and assign it to you.

In response to this:

/cherrypick release-4.3

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

bergerhoffer mentioned this pull request

Updating docs for changes to backup and restore scripts openshift/openshift-docs#19223

Merged

Contributor Author

retroflexer commented Jan 22, 2020

/test e2e-aws

2 similar comments

Contributor Author

retroflexer commented Jan 23, 2020

/test e2e-aws

Contributor Author

retroflexer commented Jan 23, 2020

/test e2e-aws


          Backup should keep data and keys separate

5fdfe58

retroflexer force-pushed the backup-should-keep-data-and-keys-separate branch from 158d174 to 5fdfe58 Compare

January 23, 2020 10:52

openshift-ci-robot removed the lgtm label

retroflexer commented

View reviewed changes

templates/master/00-master/_base/files/usr-local-bin-etcd-snapshot-restore-sh.yaml Show resolved Hide resolved

Contributor Author

retroflexer commented Jan 23, 2020

/test e2e-aws

Contributor

hexfusion commented Jan 23, 2020

/lgtm

openshift-ci-robot added the lgtm label

Contributor

hexfusion commented Jan 23, 2020

/test e2e-gcp-op

sttts reviewed

View reviewed changes

templates/master/00-master/_base/files/usr-local-bin-etcd-snapshot-restore-sh.yaml

+                      SNAPSHOT_FILE="$1"
+                    elif [[ "$1" =~ \.tar\.gz$ ]]; then
+                      BACKUP_FILE="$1"
+                      tar xzf ${BACKUP_FILE} -C ${ASSET_DIR}/tmp/ snapshot.db

Contributor

sttts Jan 23, 2020

quotes.

tar xzf works? no dash?

Contributor Author

retroflexer Jan 23, 2020 •

edited

Quotes are not needed as we have already checked if it is null.

Yes, tar xzf works. The current version on master uses the same option.

https://github.com/openshift/machine-config-operator/blob/master/templates/master/00-master/_base/files/usr-local-bin-etcd-snapshot-restore-sh.yaml#L54

Contributor

sttts Jan 23, 2020

no way for spaces? Everything is hardcoded and not dependent on user input?

Contributor Author

retroflexer commented Jan 23, 2020

/test e2e-aws

Member

runcom commented Jan 23, 2020

/approve
/lgtm

openshift-ci-robot assigned runcom

Contributor

openshift-ci-robot commented Jan 23, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hexfusion, retroflexer, runcom, sttts

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [runcom]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci-robot added the approved label

Contributor

openshift-bot commented Jan 23, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

7 similar comments

Contributor

openshift-bot commented Jan 23, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

Contributor

openshift-bot commented Jan 23, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

Contributor

openshift-bot commented Jan 23, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

Contributor

openshift-bot commented Jan 23, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

Contributor

openshift-bot commented Jan 24, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

Contributor

openshift-bot commented Jan 24, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

Contributor

openshift-bot commented Jan 24, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

Contributor

kikisdeliveryservice commented Jan 24, 2020

/skip

openshift-merge-robot merged commit ae69155 into openshift:master

openshift-cherrypick-robot commented Jan 24, 2020

@retroflexer: #1398 failed to apply on top of branch "release-4.3":

error: Failed to merge in the changes.
Using index info to reconstruct a base tree...
M	templates/master/00-master/_base/files/usr-local-bin-etcd-snapshot-backup-sh.yaml
M	templates/master/00-master/_base/files/usr-local-bin-etcd-snapshot-restore-sh.yaml
M	templates/master/00-master/_base/files/usr-local-bin-openshift-recovery-tools-sh.yaml
Falling back to patching base and 3-way merge...
Auto-merging templates/master/00-master/_base/files/usr-local-bin-openshift-recovery-tools-sh.yaml
CONFLICT (content): Merge conflict in templates/master/00-master/_base/files/usr-local-bin-openshift-recovery-tools-sh.yaml
Auto-merging templates/master/00-master/_base/files/usr-local-bin-etcd-snapshot-restore-sh.yaml
CONFLICT (content): Merge conflict in templates/master/00-master/_base/files/usr-local-bin-etcd-snapshot-restore-sh.yaml
Auto-merging templates/master/00-master/_base/files/usr-local-bin-etcd-snapshot-backup-sh.yaml
CONFLICT (content): Merge conflict in templates/master/00-master/_base/files/usr-local-bin-etcd-snapshot-backup-sh.yaml
Patch failed at 0001 Backup should keep data and keys separate

In response to this:

/cherrypick release-4.3

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Contributor

openshift-ci-robot commented Jan 24, 2020

@retroflexer: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/e2e-aws-scaleup-rhel7	`5fdfe58`	link	`/test e2e-aws-scaleup-rhel7`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

retroflexer deleted the backup-should-keep-data-and-keys-separate branch

January 24, 2020 11:08

retroflexer changed the title ~~DR: Backup should keep data and keys separate~~ Bug 1795235: DR: Backup should keep data and keys separate

Contributor

openshift-ci-robot commented Jan 30, 2020

@retroflexer: All pull requests linked via external trackers have merged. Bugzilla bug 1795235 has been moved to the MODIFIED state.

In response to this:

Bug 1795235: DR: Backup should keep data and keys separate

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Contributor Author

retroflexer commented Jan 30, 2020

/test e2e-aws-scaleup-rhel7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment