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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[IMPROVEMENT] Inconsistent name convention across volume backup restore and system backup restore #5066

Closed
khushboo-rancher opened this issue Dec 14, 2022 · 8 comments
Assignees
Labels
area/ui UI related like UI or CLI kind/improvement Request for improvement of existing function severity/4 Function working but has a minor issue (a minor incident with low impact)
Milestone

Comments

@khushboo-rancher
Copy link
Contributor

Is your improvement request related to a feature? Please describe (馃憤 if you like this request)

User can restore a volume backup with restore-1volume-1backup-2 but can't restore system backup with the same name.
We can follow same naming convention across the product.

Screenshot 2022-12-14 at 12 45 10 AM (2)

@khushboo-rancher khushboo-rancher added severity/4 Function working but has a minor issue (a minor incident with low impact) kind/improvement Request for improvement of existing function labels Dec 14, 2022
@innobead innobead added this to the v1.4.0 milestone Dec 14, 2022
@innobead innobead added the component/longhorn-manager Longhorn manager (control plane) label Dec 14, 2022
@c3y1huang
Copy link
Contributor

Hi @khushboo-rancher , I am not able to reproduce this.
Screenshot-2022-12-15-10:41:57

I've also asked @chriscchien to try and seems not reproducible.

Can you provide the support bundle?

@khushboo-rancher
Copy link
Contributor Author

I have the support bundle but I've done lots of other operations on the cluster.

supportbundle_53cae9b1-ec77-40ba-aa16-a9171b760e3e_2022-12-15T02-57-59Z.zip

Time around - 2022-12-14 08:00 AM in UTC

@c3y1huang
Copy link
Contributor

I can't tell from the support bundle.

We do not set a naming convention for SystemBackup/Restore. For now, I am not able to reproduce.

@innobead innobead modified the milestones: v1.4.0, v1.5.0 Dec 20, 2022
@khushboo-rancher
Copy link
Contributor Author

I encountered this again with v1.4.0-rc-2

[longhorn-manager-6vgg7 longhorn-manager] time="2022-12-22T20:20:31Z" level=warning msg="HTTP handling error failed to create SystemRestore restore-1`: SystemRestore.longhorn.io \"restore-1`\" is invalid: metadata.name: Invalid value: \"restore-1`\": a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')"

@innobead innobead modified the milestones: v1.5.0, v1.4.0 Dec 22, 2022
@c3y1huang
Copy link
Contributor

c3y1huang commented Dec 23, 2022

@khushboo-rancher , possibly a typo in the name? It seems to include a backtick (`).

And after looking at the issue description, it seems restore-1volume-1backup-2 is prefixed with a space.

@weizhe0422
Copy link
Contributor

Following the steps below to test on master-head and 1.4.x-head, I can reproduce the error message mentioned in this comment because there is a space in the string prefix.

  1. Install Longhorn and set up backup target path
  2. Create or restore a system backup with name restore-1volume-1backup-2
    image
    image

@innobead innobead modified the milestones: v1.4.0, v1.5.0 Dec 27, 2022
@innobead
Copy link
Member

innobead commented Dec 27, 2022

Ok, this is inconsistent at UI. When restoring a volume, UI will do the trim to remove the prefix empty characters, so the payload sent is actually w/o an invalid empty character anymore.

image

https://github.com/longhorn/longhorn-ui/blob/20dbdf331fd0efe299523341058a7c90f62d6f1f/src/routes/backup/RestoreBackup.js#L46-L48

For the backend, we don't have any pre-cleanup for users, so if users create SystemRestore CR directly, they will still encounter this kind of issue. Personally, I think we don't need to do any specific behavior for that because that kind of error can teach users how to use Longhorn or K8s.

So, this is more about the Longhorn UI experience.

@roger-ryao
Copy link

Verified on master-head 20221227

Verified on v1.4.x-head 20221227

The test steps

  1. Create the system backup with the prefix empty characters name
  2. Restore System backup with the prefix empty characters name
    e.g. restore-1volume-1backup-2

Result Passed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ui UI related like UI or CLI kind/improvement Request for improvement of existing function severity/4 Function working but has a minor issue (a minor incident with low impact)
Projects
None yet
Development

No branches or pull requests

6 participants