Add default storage class name #52
Conversation
| if err != nil { | ||
| if apierrors.IsNotFound(err) { | ||
| // No storageclass found | ||
| return nil, nil |
There was a problem hiding this comment.
I think based on this comment An error is returned if no default storage class is found we have to return an error.
There was a problem hiding this comment.
Here it looks like it's when no storageclass is found at all, not that we didn't find a default one. That happens below at the end of this function
There was a problem hiding this comment.
@Ricardo-Osorio I see, but the comment tells something different. /that's why i never trust in comments and usually try to avoid/. apierrors.IsNotFound(err) == no default storage class is found, so i expect An error is returned. We have to:
a) change the comment
b) change the behavior
There was a problem hiding this comment.
After looking at the ticket again: the etcdcluster CR requires an explicit storageClassName for the volume.
Therefore if it has not been set and we cannot find a storage class to use, then returning an error makes sense here imo.
| // storage class is found. | ||
| func (r *EtcdClusterReconciler) getDefaultStorageClass(ctx context.Context) (*kstoragev1.StorageClass, error) { | ||
| storageClasses := &kstoragev1.StorageClassList{} | ||
| err := r.List(ctx, storageClasses) |
There was a problem hiding this comment.
| err := r.List(ctx, storageClasses) | |
| if err := r.List(ctx, storageClasses); err != nil { |
| // getDefaultStorageClassName returns the name of the default storage class in the cluster, if more | ||
| // than one storage class is set to default, the first one discovered is returned. An error is returned | ||
| // if no default storage class is found. | ||
| func (r *EtcdClusterReconciler) getDefaultStorageClassName(ctx context.Context) (string, error) { |
There was a problem hiding this comment.
Do we really need a separated function for defaultSC.Name?
There was a problem hiding this comment.
I guess not right now, but I like having the option to get the SC in case we need it in future (seeing as we need to pull it from the API here anyway).
Edit: On second thought, if we are now returning an error (as above) a single function with the sole task of acquiring the name makes more sense.
Changes
Discover and set default storage class name if not set
Verification