Fix issue with the hash generation for MirrorPeer #245
Conversation
controllers/utils/hash.go
Outdated
|
|
||
| return hex.EncodeToString(checksum[:])[0 : len(BucketGenerateName)+1+12] |
There was a problem hiding this comment.
Syntactic sugar.
name will be generated like odrbucket-123456789
+1 = for the '-'
+12 = just the first 12 characters
This will be converted to hexcode
There was a problem hiding this comment.
Restored the previously written comment
| for _, peer := range mirrorPeer.Spec.Items { | ||
| peerAccumulator = append(peerAccumulator, peer.ClusterName) | ||
| peerAccumulator += peer.ClusterName |
There was a problem hiding this comment.
no sorting required?
There was a problem hiding this comment.
This is just reverting to older code for generating the bucket class names to resolve upgrade issue. The newer code for provider RDR sorts it.
controllers/utils/hash.go
Outdated
|
|
||
| checksum := sha1.Sum([]byte(strings.Join(peerAccumulator, "-"))) | ||
|
|
||
| return hex.EncodeToString(checksum[:])[0 : len(BucketGenerateName)+1+12] |
There was a problem hiding this comment.
I think both the returns are the same, they can be moved outside of the if condition.
There are two separate methods to generate hashes based on whether RDR is done for provider mode or internal mode The one for internal mode is being reverted in this commit as the it was causing issues with upgrades Signed-off-by: vbadrina <[email protected]>
vbnrh
force-pushed
the
fix-obc-naming-previous-version
branch
from
2ec88fa to
78291db
Compare
| checksum := sha1.Sum([]byte(strings.Join(peerAccumulator, "-"))) | ||
| return hex.EncodeToString(checksum[:]) | ||
| // truncate to bucketGenerateName + "-" + first 12 (out of 20) byte representations of sha1 checksum | ||
| return hex.EncodeToString(checksum[:])[0 : len(BucketGenerateName)+1+12] |
There was a problem hiding this comment.
is this wont provide any length error for the older upgrade case, because I dont see you adding 1+12 in else case
There was a problem hiding this comment.
both if and else will calculate the checksum in different ways
This checksum will return the first 12 characters in hex string.
|
@GowthamShanmugam: changing LGTM is restricted to collaborators In response to this: Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: GowthamShanmugam, vbnrh The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/test integration-test |
|
/lgtm |
|
@GowthamShanmugam: changing LGTM is restricted to collaborators In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
openshift-merge-bot
bot
merged commit 27674c2
into
red-hat-storage:main
|
/cherry-pick release-4.18 |
|
@vbnrh: new pull request created: #247 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
There are two separate methods to generate hashes based on whether RDR is done for provider mode or internal mode
The one for internal mode is being reverted in this commit as the it was causing issues with upgrades
https://issues.redhat.com/browse/DFBUGS-961