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

Add v1.10 velero upgrade doc #5468

Merged
merged 3 commits into from Nov 7, 2022
Merged

Conversation

qiuming-best
Copy link
Contributor

Signed-off-by: Ming mqiu@vmware.com

Thank you for contributing to Velero!

Please add a summary of your change

Does your change fix a particular issue?

Fixes #(issue)

Please indicate you've done the following:

  • Accepted the DCO. Commits without the DCO will delay acceptance.
  • Created a changelog file or added /kind changelog-not-required as a comment on this pull request.
  • Updated the corresponding documentation in site/content/docs/main.

@@ -17,12 +17,15 @@ If you're not yet running at least Velero v1.6, see the following:
- [Upgrading to v1.6][6]
- [Upgrading to v1.7][7]
- [Upgrading to v1.8][8]
- [Upgrading to v1.9][9]
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. Prerequisite is wrong.
  2. I'm not sure if it's necessary to list all the previous minor versions upgrade instructions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What about we just keep 3 or 5 items on the list?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. Prerequisite modified
  2. I've just keep 5 items

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@OrlinVasilev Do you think how many items should we keep for upgrading instructions?

# uploader_type value cloud be restic or kopia
kubectl get deploy -n velero -ojson \
| sed "s#\"image\"\: \"velero\/velero\:v[0-9]*.[0-9]*.[0-9]\"#\"image\"\: \"velero\/velero\:v1.10.0\"#g" \
| sed "s#\"--default-volumes-to-restic=true\"#\"--default-volumes-to-fs-backup=true\",\"--uploader-type=$uploader_type\"#g" \
Copy link
Contributor

Choose a reason for hiding this comment

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

If the user explicitly set --default-volumes-to-restic=false and it will remain, will it cause any problem?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, I'll modify it

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've changed

dsjson=$(kubectl get ds -n velero -ojson)
kubectl delete ds -n velero --all --force --grace-period 0
echo $dsjson | sed "s#\"image\"\: \"velero\/velero\:v[0-9]*.[0-9]*.[0-9]\"#\"image\"\: \"velero\/velero\:v1.10.0\"#g" \
| sed "s#restic#node-agent#g" | kubectl apply -f -
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure restic is not used in the env var or path or other places, if this is for renaming, similar to image, would it be better if we let sed replace only the name attribute?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, currently, restic if is used by label, name, or container start arguments should be replaced, if used by env var or path that would cause some problem, I'll restrict the replace fields.

| sed "s#restic-timeout#fs-backup-timeout#g" \
| kubectl apply -f -

# optional, if using the velero daemon set
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it intentional that restic was replaced by velero?

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'm still keeping restic now, older version indeed is restic daemon set

```

## Notes
### Default backup storage location
We have deprecated the way to indicate the default backup storage location. Previously, that was indicated according to the backup storage location name set on the velero server-side via the flag `velero server --default-backup-storage-location`. Now we configure the default backup storage location on the velero client-side. Please refer to the [About locations][9] on how to indicate which backup storage location is the default one.
We have deprecated the way to indicate the default backup storage location. Previously, that was indicated according to the backup storage location name set on the velero server-side via the flag `velero server --default-backup-storage-location`. Now we configure the default backup storage location on the velero client-side. Please refer to the [About locations][10] on how to indicate which backup storage location is the default one.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this still needed? I don't think we need it in all future versions.

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've removed it

@qiuming-best qiuming-best force-pushed the upgrade-doc branch 3 times, most recently from bd04630 to 6de4c46 Compare October 21, 2022 02:02
@codecov-commenter
Copy link

codecov-commenter commented Oct 21, 2022

Codecov Report

Merging #5468 (f848f50) into main (11a7c79) will increase coverage by 0.17%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #5468      +/-   ##
==========================================
+ Coverage   40.56%   40.74%   +0.17%     
==========================================
  Files         236      238       +2     
  Lines       20484    20561      +77     
==========================================
+ Hits         8310     8377      +67     
- Misses      11565    11568       +3     
- Partials      609      616       +7     
Impacted Files Coverage Δ
pkg/uploader/provider/restic.go 26.04% <0.00%> (-0.56%) ⬇️
pkg/controller/backup_deletion_controller.go 57.26% <0.00%> (-0.26%) ⬇️
pkg/cmd/server/server.go 6.57% <0.00%> (-0.02%) ⬇️
pkg/builder/object_meta.go 0.00% <0.00%> (ø)
pkg/builder/volume_snapshot_content_builder.go 0.00% <0.00%> (ø)
pkg/builder/volume_snapshot_builder.go 0.00% <0.00%> (ø)
pkg/controller/backup_controller.go 55.84% <0.00%> (+7.68%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Signed-off-by: Ming <mqiu@vmware.com>
@qiuming-best qiuming-best added the Website non-docs changes for the website label Oct 25, 2022
@OrlinVasilev
Copy link
Contributor

@qiuming-best why are you removing 1.9 upgrade guide?

@qiuming-best
Copy link
Contributor Author

@qiuming-best why are you removing 1.9 upgrade guide?

@OrlinVasilev 1.9 upgrade guide is not been removed, In site/content/docs/main/ we should put the newest version of document, so I replace v1.9 to v1.10 doc

@OrlinVasilev
Copy link
Contributor

@qiuming-best
image
And
image

@OrlinVasilev
Copy link
Contributor

OrlinVasilev commented Oct 26, 2022

@qiuming-best fixed TOC for you! pleas check!

Signed-off-by: OrlinVasilev <ovasilev@vmware.com>

# optional, if using the restic daemon set
dsjson=$(kubectl get ds -n velero -ojson)
kubectl delete ds -n velero --all --force --grace-period 0
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we not using force deletion on DaemonSet update?
And I'm not sure Deployment can be updated without deletion, but DaemonSet needs deletion first.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@blackpiglet

I'm not sure Deployment can be updated without deletion, but DaemonSet needs deletion first.
all scripts are been tested.

for velero deployment, it's not been deleted, here just update images and fields.

but for restic is no longer exists in 1.10, so it must be deleted, So I create a new daemonset with the new name 'node-agent', using force deletion just want less time-consuming.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was about to ask why the process is so hackie :)

@qiuming-best
Copy link
Contributor Author

fix main TOC for 1.10

@OrlinVasilev thanks a lot !!!

Server:
Version: v1.10.0
```

Copy link
Contributor

Choose a reason for hiding this comment

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

We also need to tell users that some left over resources need to be deleted manually:
resticrepository CRs
velero-restic-credentials secret

There is no problem to leave these unused resources there, but if users want to delete them, they need to do it manually through kubectl

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

Signed-off-by: Ming <mqiu@vmware.com>
@Lyndon-Li Lyndon-Li added this to the 1.10 milestone Nov 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation has-changelog Website non-docs changes for the website
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants