-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
csi: bump csi snapshotter image to v5 #9652
Conversation
2e4dff6
to
fa02d9e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sathieu Thanks for the PR can you also update the RBAC changes required as per changelog https://github.com/kubernetes-csi/external-snapshotter/blob/master/CHANGELOG/CHANGELOG-5.0.md#other-cleanup-or-flake and also update the version at https://github.com/rook/rook/blob/master/tests/framework/utils/snapshot.go#L30?
Signed-off-by: Mathieu Parent <mathieu.parent@insee.fr>
fa02d9e
to
c57416b
Compare
@Madhu-1 Requested changes done. Back to you! |
@sathieu CI failure https://github.com/rook/rook/runs/4962590767?check_suite_focus=true can you please check |
@@ -27,7 +27,7 @@ import ( | |||
const ( | |||
// snapshotterVersion from which the snapshotcontroller and CRD will be | |||
// installed | |||
snapshotterVersion = "v4.0.0" | |||
snapshotterVersion = "v5.0.1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
earlier snapshotter was getting deployed in the default namespace, after 4.0.0 its getting deployed in kube-system namespace, I think this change is still not enough we might need to do a string replace to replace kube-system with default or change
rook/tests/framework/utils/snapshot.go
Line 86 in c57416b
namespace := "default" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, otherwise based on the rook/snapshot sidecar version we can make the change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This pull request has merge conflicts that must be resolved before it can be merged. @sathieu please rebase it. https://rook.io/docs/rook/latest/development-flow.html#updating-your-fork |
Thanks @Madhu-1 |
Description of your changes:
Bump csi snapshotter image to v5.0.1.
Breaking changes (from 5.0.1 release notes:
There is also:
Which issue is resolved by this Pull Request:
Resolves #
Checklist:
make codegen
) has been run to update object specifications, if necessary.