annotate storageconsumer with deployed mode and local to cluster #3068
Conversation
leelavg
changed the title
leelavg
force-pushed
the
old-cons
branch
3 times, most recently
from
40ccc42 to
dad66a1
Compare
Signed-off-by: Leela Venkaiah G <[email protected]>
|
|
||
| func (s *storageConsumer) ensureCreated(r *StorageClusterReconciler, storageCluster *ocsv1.StorageCluster) (ctrl.Result, error) { | ||
| storageConsumer := &ocsv1a1.StorageConsumer{} | ||
| storageConsumer.Name = localStorageConsumerName |
There was a problem hiding this comment.
We need to make sure that we are not creating this in 4.18 provider mode clusters
|
|
||
| var _ resourceManager = &storageConsumer{} | ||
|
|
||
| const localStorageConsumerName = "storageconsumer-local" |
There was a problem hiding this comment.
CR name is already storageConsumer, cant we use local?
| @@ -345,6 +353,25 @@ func (r *StorageRequestReconciler) reconcilePhases() (reconcile.Result, error) { | |||
| r.StorageRequest.Status.Phase = v1alpha1.StorageRequestReady | |||
| } | |||
|
|
|||
| clusterID := util.GetClusterID(r.ctx, r.Client, &r.log) | |||
There was a problem hiding this comment.
Can you please add a comment of this change
| } else { | ||
| return reconcile.Result{}, fmt.Errorf("failed to get cluster id") | ||
| } |
There was a problem hiding this comment.
instead of if else, fail early if the clusterID is empty?
|
/hold |
openshift-ci
bot
added
the
do-not-merge/hold
| storageConsumer.Namespace = storageCluster.Namespace | ||
| if _, err := controllerutil.CreateOrUpdate(r.ctx, r.Client, storageConsumer, func() error { | ||
| if err := controllerutil.SetControllerReference(storageCluster, storageConsumer, r.Scheme); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| // TODO: OldModeAnnotation should be removed after convergence | ||
| util.AddAnnotation(storageConsumer, defaults.StorageConsumerOldModeAnnotation, defaults.StorageConsumerOldModeInternal) | ||
| util.AddAnnotation(storageConsumer, defaults.StorageConsumerTypeAnnotation, defaults.StorageConsumerTypeLocal) | ||
| return nil | ||
| }); err != nil { |
There was a problem hiding this comment.
Don't use a create or update in this case. You intent is to update only and ignore not found errors
There was a problem hiding this comment.
need more discussion on this based on who owns the creation of storageconsumer.
|
|
[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 |
Signed-off-by: Leela Venkaiah G <[email protected]>
|
/hold will squash after final review |
this is intended for a backport to older release for configuring storageconsumer with all the details helping in convergence.
annotated details are
running mode: provider/internal (ceph resources among others can be calculated based on this)
consumer type: local (helps webhook to take the storageconsumer UID for defaulting multiple fields in storageclass)
with these details already mentioned in storageconsumer, the corresponding controller would branch out and owns existing resources.