shim layer for sending claim resources from existing storagerequest crs #3022
Conversation
Signed-off-by: Leela Venkaiah G <[email protected]>
Signed-off-by: Leela Venkaiah G <[email protected]>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: leelavg The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Part of RHSTOR-6918 skipped unit tests will be fixed in a new commit. |
Signed-off-by: Leela Venkaiah G <[email protected]>
| func getStorageRequestHash(consumerUUID, storageClaimName string) string { | ||
| s := struct { | ||
| StorageConsumerUUID string `json:"storageConsumerUUID"` | ||
| StorageClaimName string `json:"storageClaimName"` | ||
| }{ | ||
| consumerUUID, | ||
| storageClaimName, | ||
| } | ||
|
|
||
| requestName, err := json.Marshal(s) | ||
| if err != nil { | ||
| klog.Errorf("failed to marshal a name for a storage class request based on %v. %v", s, err) | ||
| panic("failed to marshal storage class request name") | ||
| } | ||
| md5Sum := md5.Sum(requestName) | ||
| return hex.EncodeToString(md5Sum[:16]) | ||
| } | ||
|
|
||
| // getStorageRequestName generates a name for a StorageRequest resource. | ||
| func getStorageRequestName(consumerUUID, storageClaimName string) string { | ||
| return fmt.Sprintf("storagerequest-%s", getStorageRequestHash(consumerUUID, storageClaimName)) | ||
| } |
There was a problem hiding this comment.
Do we really need these 2 functions as standalone functions? I see the logic belong for a single flow which is the GetRequest logic. Can we fold everything there?
It will make it easier to move around and potentially remove in the future.
| return fmt.Sprintf("storagerequest-%s", getStorageRequestHash(consumerUUID, storageClaimName)) | ||
| } | ||
|
|
||
| func (c *ocsConsumerManager) GetRequest(ctx context.Context, consumerUUID, storageClaimName string) (*ocsv1alpha1.StorageRequest, error) { |
There was a problem hiding this comment.
Please add a "TODO" comment explaining that this function is to be remove when we do the jump to 4.20
| err := c.client.Get(ctx, types.NamespacedName{Name: generatedRequestName, Namespace: c.namespace}, storageRequestObj) | ||
| if err != nil { | ||
| klog.Errorf("failed to get a StorageRequest named %q for consumer %q and request %q. %v", generatedRequestName, consumerUUID, storageClaimName, err) | ||
| return nil, err | ||
| } |
There was a problem hiding this comment.
Isn't the point to remove the storage request API with this shim. If so it does not make sense to load the resource
There was a problem hiding this comment.
remove the storage request API with this shim
- not the intention w/ this specific PR, I'm trying to remove build time dependency on StorageRequest and when all the required resources (via different PRs but in 4.19 itself) are updated on StorageConsumer API we'll directly send the necessary details to older clients loading StorageConsumer.
| // this is only here for backward compatibility as we expect no existing older client | ||
| // wants to fulfill claims as current provider deprecated storagerequests | ||
| _, err := s.consumerManager.GetRequest(ctx, req.StorageConsumerUUID, req.StorageClaimName) |
There was a problem hiding this comment.
I think the better approach is to always return internal error (or some other error) for this operation
| // client cluster want to stop using storageclasses and data on the backend can be removed | ||
| // TODO: this should be transferred into removal of storageclasses and corresponding data |
There was a problem hiding this comment.
I think the better approach is to always return internal error (or some other error) for this operation
|
/hold until storageconsumer API is fully developed. |
openshift-ci
bot
added
the
do-not-merge/hold
until client-op is on same version as provider running convergence code provider should continue to support older clients by sending already claimed resources.
new claim creation and deletion of existing claims are no-ops but fulfill storageclaim continues to send already created resources before upgrade until client-op also upgraded.