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

PWX-35626: Add a last 4 digit UID string from volumesnapshotschedule to volumesnapshot name to avoid name collision. #1686

Merged
merged 1 commit into from
Mar 28, 2024

Conversation

diptiranjanpx
Copy link
Contributor

Signed-Off-By: Diptiranjan

What type of PR is this?
bug

What this PR does / why we need it:
Adding the last 4 digit of UID to volumesnapshot names to avoid name collision in case of long volumesnapshotschedule with similar names resulting in similar volumesnapshot names if time frequency matches.

Does this PR change a user-facing CRD or CLI?:
no

Is a release note needed?:

Issue: Similar volumesnapshot names were getting created if volumesnapshotschedule frequency matches and after trimming produces similar substrings.
User Impact: For one volume snapshot might not be taken and be already marked as successful.
Resolution: Adding a 4 digit randomness to the name to avoid name collision for volumesnapshots resulting from different volumesnapshotschedules.

Does this change need to be cherry-picked to a release branch?:

Test

Volumesnapshotschedule names with after trim should have given output

➜  elasticsearch git:(PWX-35626) ✗ storkctl get volumesnapshotschedule -n es
NAME                                                     PVC                                    POLICYNAME   PRE-EXEC-RULE   POST-EXEC-RULE   RECLAIM-POLICY   SUSPEND   LAST-SUCCESS-TIME
elasticdata-vol-elasticsearch-data-0-interval-1-minute   elasticdata-vol-elasticsearch-data-0   every-1m                                      Delete           false     26 Mar 24 10:42 UTC
elasticdata-vol-elasticsearch-data-1-interval-1-minute   elasticdata-vol-elasticsearch-data-1   every-1m                                      Delete           false     26 Mar 24 10:42 UTC
elasticdata-vol-elasticsearch-data-2-interval-1-minute   elasticdata-vol-elasticsearch-data-2   every-1m                                      Delete           false     26 Mar 24 10:42 UTC

But with the last 4 digits of UID as suffix to create random  string, volume snapshots are coming like
➜  elasticsearch git:(PWX-35626) ✗ storkctl get volumesnapshots -n es
NAME                                                              PVC                                    STATUS   CREATED               COMPLETED             TYPE
elasticdata-vol-elasticsearch-d-interval-22e7-2024-03-26-104052   elasticdata-vol-elasticsearch-data-0   Ready    26 Mar 24 10:40 UTC   26 Mar 24 10:40 UTC   local
elasticdata-vol-elasticsearch-d-interval-22e7-2024-03-26-104152   elasticdata-vol-elasticsearch-data-0   Ready    26 Mar 24 10:41 UTC   26 Mar 24 10:41 UTC   local
elasticdata-vol-elasticsearch-d-interval-22e7-2024-03-26-104252   elasticdata-vol-elasticsearch-data-0   Ready    26 Mar 24 10:42 UTC   26 Mar 24 10:42 UTC   local
elasticdata-vol-elasticsearch-d-interval-5313-2024-03-26-104058   elasticdata-vol-elasticsearch-data-2   Ready    26 Mar 24 10:40 UTC   26 Mar 24 10:40 UTC   local
elasticdata-vol-elasticsearch-d-interval-5313-2024-03-26-104158   elasticdata-vol-elasticsearch-data-2   Ready    26 Mar 24 10:41 UTC   26 Mar 24 10:41 UTC   local
elasticdata-vol-elasticsearch-d-interval-5313-2024-03-26-104258   elasticdata-vol-elasticsearch-data-2   Ready    26 Mar 24 10:42 UTC   26 Mar 24 10:42 UTC   local
elasticdata-vol-elasticsearch-d-interval-d305-2024-03-26-104055   elasticdata-vol-elasticsearch-data-1   Ready    26 Mar 24 10:40 UTC   26 Mar 24 10:40 UTC   local
elasticdata-vol-elasticsearch-d-interval-d305-2024-03-26-104155   elasticdata-vol-elasticsearch-data-1   Ready    26 Mar 24 10:41 UTC   26 Mar 24 10:41 UTC   local
elasticdata-vol-elasticsearch-d-interval-d305-2024-03-26-104255   elasticdata-vol-elasticsearch-data-1   Ready    26 Mar 24 10:42 UTC   26 Mar 24 10:42 UTC   local


Integration test: https://jenkins.pwx.dev.purestorage.com/job/Users/job/dipti/job/dipti-snapshot-k8s-1-25-0/4/

@diptiranjanpx diptiranjanpx added bug integration-test The change adds or updates an integration test labels Mar 27, 2024
Copy link
Contributor

@olavangad-px olavangad-px left a comment

Choose a reason for hiding this comment

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

LGTM.

Just curious is a partial uuid better than using rand to generate a salt of fixed length?

@diptiranjanpx
Copy link
Contributor Author

LGTM.

Just curious is a partial uuid better than using rand to generate a salt of fixed length?

I thought about it but did not went in that because of following.

  1. Did not want to loose more chars to the random string (max to be kept at 4 chars).
  2. Time can't be used as the seed as that was originally the problem creator.
    So I thought what can be better than taking volumesnapshotschedule's uuid itself as in that cluster is unique and to really have a collision, trimmed volumesnapshotschedule, time creation of volumesnapshots and last 4 digits of UID of volumesnapshotschedules have to match which in my opinion probablity of that should be very very less.

Copy link
Contributor

@pp511 pp511 left a comment

Choose a reason for hiding this comment

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

Looks good.

@diptiranjanpx diptiranjanpx merged commit e271503 into master Mar 28, 2024
3 checks passed
@diptiranjanpx diptiranjanpx deleted the PWX-35626 branch May 27, 2024 08:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug integration-test The change adds or updates an integration test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants