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.

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

Support annotation in VolumeSnapshot and VolumeSnapshotContent resources #2986

Merged
merged 28 commits into from
Aug 5, 2024
Merged

Support annotation in VolumeSnapshot and VolumeSnapshotContent resources #2986

merged 28 commits into from
Aug 5, 2024

Conversation

saima-s
Copy link
Contributor

@saima-s saima-s commented Jul 17, 2024 •
edited
Loading

Change Overview

This PR is about adding support for annotation in some methods of Snapshotter interface. Methods are listed below:

  1. Create
  2. Clone
  3. CreateFromSource
  4. CreateContentFromSource

Pull request type

Please check the type of change your PR introduces:

  • 🌻 Feature

Issues

  • fixes #issue-number

Test Plan

Test case-1

Created snapshot and restored those snapshots in another namespace.

Result

Result can be found here

  • 💪 Manual
  • ⚡ Unit test
  • 💚 E2E

@saima-s saima-s changed the title WIP Support annotation in VolumeSnapshot and VolumeSnapshotContent CRDs Jul 17, 2024
@saima-s saima-s marked this pull request as ready for review July 19, 2024 10:35
viveksinghggits
viveksinghggits reviewed Jul 26, 2024
View reviewed changes
pkg/kube/snapshot/snapshot.go Outdated Show resolved Hide resolved
pkg/kube/snapshot/snapshot.go Outdated Show resolved Hide resolved
pkg/kube/snapshot/snapshot.go Outdated Show resolved Hide resolved
pkg/kube/snapshot/snapshot.go Outdated Show resolved Hide resolved
pkg/kube/snapshot/snapshot_alpha.go Outdated Show resolved Hide resolved
pkg/kube/snapshot/snapshot_alpha.go Outdated Show resolved Hide resolved
viveksinghggits
viveksinghggits requested changes Jul 29, 2024
View reviewed changes
pkg/kube/snapshot/snapshot.go Outdated Show resolved Hide resolved
pkg/kube/snapshot/snapshot.go Outdated Show resolved Hide resolved
pkg/kube/snapshot/snapshot.go Outdated Show resolved Hide resolved
pkg/kube/snapshot/snapshot.go Outdated Show resolved Hide resolved
pkg/kube/snapshot/snapshot_alpha.go Outdated Show resolved Hide resolved
pkg/function/create_csi_snapshot.go Outdated Show resolved Hide resolved
pkg/function/create_csi_snapshot_static.go Outdated Show resolved Hide resolved
pkg/function/delete_csi_snapshot_content_test.go Outdated Show resolved Hide resolved
pkg/function/delete_csi_snapshot_test.go Outdated Show resolved Hide resolved
pkg/function/restore_csi_snapshot_test.go Outdated Show resolved Hide resolved
PrasadG193
PrasadG193 reviewed Jul 29, 2024
View reviewed changes
pkg/kube/snapshot/mocks/mock_snapshotter.go Outdated Show resolved Hide resolved
pkg/kube/snapshot/snapshot_alpha.go Outdated Show resolved Hide resolved
pkg/kube/snapshot/snapshot_alpha.go Outdated Show resolved Hide resolved
pkg/kube/snapshot/snapshot_alpha.go Outdated Show resolved Hide resolved
PrasadG193
PrasadG193 reviewed Jul 30, 2024
View reviewed changes
pkg/kube/snapshot/mocks/mock_snapshotter.go Outdated Show resolved Hide resolved
pkg/kube/snapshot/types.go Outdated Show resolved Hide resolved
pkg/kube/snapshot/types.go Outdated Show resolved Hide resolved
PrasadG193
PrasadG193 reviewed Jul 30, 2024
View reviewed changes
Copy link
Contributor

@PrasadG193 PrasadG193 left a comment

Choose a reason for hiding this comment

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

Need to correct few code comment, overall looks in good shape now

pkg/kube/snapshot/snapshot.go Outdated Show resolved Hide resolved
pkg/kube/snapshot/snapshot.go Outdated Show resolved Hide resolved
pkg/kube/snapshot/snapshot.go Outdated Show resolved Hide resolved
pkg/kube/snapshot/snapshot.go Outdated Show resolved Hide resolved
pkg/kube/snapshot/snapshot_alpha.go Outdated Show resolved Hide resolved
pkg/kube/snapshot/snapshot_alpha.go Outdated Show resolved Hide resolved
pkg/kube/snapshot/snapshot_alpha.go Outdated Show resolved Hide resolved
pkg/kube/snapshot/snapshot_beta.go Outdated Show resolved Hide resolved
pkg/kube/snapshot/snapshot_beta.go Outdated Show resolved Hide resolved
viveksinghggits
viveksinghggits reviewed Jul 31, 2024
View reviewed changes
pkg/function/create_csi_snapshot_static.go Outdated Show resolved Hide resolved
pkg/kube/snapshot/mocks/mock_snapshotter.go Outdated Show resolved Hide resolved
pkg/kube/snapshot/snapshot.go Outdated Show resolved Hide resolved
pkg/kube/snapshot/snapshot.go Outdated Show resolved Hide resolved
pkg/kube/snapshot/snapshot_beta.go Outdated Show resolved Hide resolved
pkg/kube/snapshot/snapshot_beta.go Outdated Show resolved Hide resolved
pkg/kube/snapshot/snapshot_beta.go Show resolved Hide resolved
pkg/kube/snapshot/types.go Show resolved Hide resolved
@hairyhum
Copy link
Contributor

@saima-s can you please make a release note for this change: https://github.com/kanisterio/kanister/blob/master/CONTRIBUTING.md#release-notes

viveksinghggits
viveksinghggits reviewed Aug 1, 2024
View reviewed changes
Copy link
Contributor

@viveksinghggits viveksinghggits left a comment

Choose a reason for hiding this comment

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

Almost there.

pkg/kube/snapshot/snapshot.go Outdated Show resolved Hide resolved
pkg/kube/snapshot/snapshot_alpha.go Outdated Show resolved Hide resolved
releasenotes/notes/support-annotation-on-snapshotter-function-ff9b7ba2daf10427.yaml Outdated Show resolved Hide resolved
viveksinghggits
viveksinghggits reviewed Aug 2, 2024
View reviewed changes
Copy link
Contributor

@viveksinghggits viveksinghggits left a comment

Choose a reason for hiding this comment

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

LGTM

pkg/kube/snapshot/snapshot_test.go Show resolved Hide resolved
pkg/kube/snapshot/snapshot_test.go Outdated Show resolved Hide resolved
pkg/kube/snapshot/types.go Outdated Show resolved Hide resolved
pkg/kube/snapshot/types.go Outdated Show resolved Hide resolved
pkg/kube/snapshot/types.go Outdated Show resolved Hide resolved
@saima-s saima-s changed the title Support annotation in VolumeSnapshot and VolumeSnapshotContent CRDs Support annotation in VolumeSnapshot and VolumeSnapshotContent resources Aug 2, 2024
viveksinghggits
viveksinghggits approved these changes Aug 4, 2024
View reviewed changes
pkg/kube/snapshot/types.go Outdated Show resolved Hide resolved
PrasadG193
PrasadG193 approved these changes Aug 5, 2024
View reviewed changes
Copy link
Contributor

@PrasadG193 PrasadG193 left a comment

Choose a reason for hiding this comment

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

LGTM. Have minor suggests in unit tests

pkg/kube/snapshot/snapshot_test.go Outdated Show resolved Hide resolved
pkg/kube/snapshot/snapshot_test.go Outdated Show resolved Hide resolved
@saima-s saima-s added the kueue label Aug 5, 2024
@mergify mergify bot merged commit ea6cb88 into master Aug 5, 2024
18 checks passed
@mergify mergify bot deleted the orange/saima/support-snapshot-annotation branch August 5, 2024 08:05
@github-actions github-actions bot mentioned this pull request Sep 19, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@viveksinghggits viveksinghggits viveksinghggits approved these changes

@hairyhum hairyhum hairyhum left review comments

@PrasadG193 PrasadG193 PrasadG193 approved these changes

Assignees
No one assigned
Labels
kueue
Projects
Kanister
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@saima-s @hairyhum @PrasadG193 @viveksinghggits