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

Improve etcd backup filenames #10143

Merged
merged 2 commits into from Jun 24, 2022

Conversation

xrstf
Copy link
Contributor

@xrstf xrstf commented Jun 22, 2022

What does this PR do / Why do we need it:
Not all S3 backends can handle files with colons (:) in them, apparently. Also it's hard to use the files because no file extension is present and you're not even sure if it's a tar.gz of the etcd or not.

This PR addresses both points. Existing backups should be untouched, but new ones should be more compatible.

Does this PR close any issues?:
Fixes #7337

Does this PR introduce a user-facing change?:

etcd backup files are named differently (`foo-YYYY-MM-DDThh:mm:ss` to `foo-YYYY-MM-DDThhmmss.db`) to improve compatibility with different storage solutions

@kubermatic-bot kubermatic-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: yes Denotes that all commits in the pull request have the valid DCO signoff message. sig/cluster-management Denotes a PR or issue as being assigned to SIG Cluster Management. labels Jun 22, 2022
@kubermatic-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: xrstf

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:

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

@kubermatic-bot kubermatic-bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 22, 2022
@xrstf xrstf requested a review from embik June 22, 2022 11:09
@xrstf
Copy link
Contributor Author

xrstf commented Jun 22, 2022

/retest

1 similar comment
@xrstf
Copy link
Contributor Author

xrstf commented Jun 22, 2022

/retest

Copy link
Member

@embik embik left a comment

Choose a reason for hiding this comment

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

Q

pkg/storeuploader/storeuploader.go Show resolved Hide resolved
@kubermatic-bot kubermatic-bot added the lgtm Indicates that a PR is ready to be merged. label Jun 24, 2022
@kubermatic-bot
Copy link
Contributor

LGTM label has been added.

Git tree hash: a8f558c2a6edc7bb6ddb8bcff4c126868619d9fc

@xrstf
Copy link
Contributor Author

xrstf commented Jun 24, 2022

/retest

@kubermatic-bot kubermatic-bot merged commit 3e417d8 into kubermatic:master Jun 24, 2022
@xrstf xrstf deleted the more-compatible-backup-names branch June 24, 2022 17:29
@dharapvj
Copy link
Contributor

/cherry-pick release/v2.19
/cherry-pick release/v2.20

@kubermatic-bot
Copy link
Contributor

@dharapvj: #10143 failed to apply on top of branch "release/v2.19":

Applying: do not use colons in legacy backup file names to increase compatibility with some S3 backends
Using index info to reconstruct a base tree...
M	pkg/storeuploader/storeuploader.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/storeuploader/storeuploader.go
CONFLICT (content): Merge conflict in pkg/storeuploader/storeuploader.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 do not use colons in legacy backup file names to increase compatibility with some S3 backends
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick release/v2.19
/cherry-pick release/v2.20

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.

@dharapvj
Copy link
Contributor

/cherry-pick release/v2.20

@kubermatic-bot
Copy link
Contributor

@dharapvj: #10143 failed to apply on top of branch "release/v2.20":

Applying: do not use colons in legacy backup file names to increase compatibility with some S3 backends
Using index info to reconstruct a base tree...
M	pkg/storeuploader/storeuploader.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/storeuploader/storeuploader.go
CONFLICT (content): Merge conflict in pkg/storeuploader/storeuploader.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 do not use colons in legacy backup file names to increase compatibility with some S3 backends
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick release/v2.20

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.

xrstf added a commit to xrstf/kubermatic that referenced this pull request Jul 20, 2022
* do not use colons in legacy backup file names to increase compatibility with some S3 backends

* do not create backup files without extension
xrstf added a commit to xrstf/kubermatic that referenced this pull request Jul 20, 2022
kubermatic-bot pushed a commit that referenced this pull request Jul 20, 2022
* do not use colons in legacy backup file names to increase compatibility with some S3 backends

* do not create backup files without extension
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Denotes that all commits in the pull request have the valid DCO signoff message. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/cluster-management Denotes a PR or issue as being assigned to SIG Cluster Management. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

User Cluster Backup Naming not S3 compliant
4 participants