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 RestoreSize to VolumeSnapshot #347

Merged
merged 1 commit into from
Oct 18, 2024
Merged

Add RestoreSize to VolumeSnapshot #347

merged 1 commit into from
Oct 18, 2024

Conversation

jooseppi-luna
Copy link
Contributor

Description

-- Note: this is identical to an earlier PR with unsigned commits --
Currently, when the csi-powermax takes a snapshot of a volume, it does not set a RestoreSize for the snapshot. This PR adds the sizeInBytes field to the snapshot to set the RestoreSize. The following additional work is also done:

  1. Add clarification to README about building the PMax image.
  2. Fix several places in the helm tests where a namespace was hardcoded.
  3. Update some deprecated kubectl exec commands in helm tests.
  4. Update the size of the pvc being restored to in the 2vols+restore helm test to 8194Mi -- even though the requested size of the original volume was 8Gi (8192Mi), PMax rounds to the next cylinder boundary, which gives the pvc an actual size of 8194Mi. A separate doc PR will go up in relation to this.

GitHub Issues

List the GitHub issues impacted by this PR:

GitHub Issue #
dell/csm#1529

Checklist:

  • Have you run format,vet & lint checks against your submission?
  • Have you made sure that the code compiles?
  • Did you run the unit & integration tests successfully?
  • Have you maintained at least 90% code coverage?
  • Have you commented your code, particularly in hard-to-understand areas
  • Have you done corresponding changes to the documentation
  • Did you run tests in a real Kubernetes cluster?
  • Backward compatibility is not broken

How Has This Been Tested?

Ran unit tests and snaprestore helm test. Cannot currently run cert-csi, because it fails due to the cylinder-boundary rounding. Working on a separate cert-csi PR to fix this, but was instructed by @AronAtDell and @mjsdell to already merge because we have verified the fix with helm tests. Integration tests are currently broken, but I will run them once they are fixed.

* get snapshot size from vol size

* Update controller.go

* add log message

* fix quote mark

* %d instead of %s

* debug log

* dumb

* use utkarsh math

* helm fixes

* get rid of hardcoded ns

* Update createFromSnap.yaml

* no more hardcoded ns

* hardcoded ns bad

* hard coded n s

* kubectl exec syntax update

* kubectl exec syntax update

* add note about dependent repos

* get rid of debug printout

* fix unit test target

* fmt fixes

* controller fmt

* gofumpt

* remove debug logs

* undo makefile changes

* Update pvc0.yaml

---------

Co-authored-by: root <root@master-1-jkPwn32okjktZ.domain>
@jooseppi-luna jooseppi-luna changed the title Add RestoreSize to VolumeSnapshot (#346) Add RestoreSize to VolumeSnapshot Oct 18, 2024
Copy link
Contributor

@abhi16394 abhi16394 left a comment

Choose a reason for hiding this comment

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

LGTM

@jooseppi-luna jooseppi-luna merged commit 837ae8e into main Oct 18, 2024
5 checks passed
@jooseppi-luna jooseppi-luna deleted the add-restoresize branch October 18, 2024 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants