external: add Openshift Virt Storage Class when crd is present by ShravaniVangur · Pull Request #3703 · red-hat-storage/ocs-operator

ShravaniVangur · 2026-02-18T09:45:07Z

Creates the virtualization storage class when the kubevirt crd is present.Adds the default label based on instance.Spec.ManagedResources.CephBlockPools.DefaultVirtualizationStorageClass.
Resolves: RHSTOR-8594

ShravaniVangur · 2026-02-19T07:47:44Z

/cc @parth-gr

controllers/storagecluster/external_resources.go

parth-gr · 2026-02-19T07:54:57Z

controllers/storagecluster/external_resources.go

+		return err
+	}
+	if crd.UID != "" {
+		scc = StorageClassConfiguration{


This should be done inside the else if d.Name == cephRbdStorageClassName {

either create virt sc or create the rbd normal sc

This would be an additional RBD storage class and should not replace the existing one right?

I think it should be the only storageclass not a additionla one, check with @eran

parth-gr · 2026-02-19T07:55:19Z

/assign @Madhu-1 @travisn

rewantsoni · 2026-02-24T09:06:41Z

@ShravaniVangur @parth-gr I had one question here, the virt storageclass that we create should be created on top of the default radosnamespace or the custom radosnamespace?

parth-gr · 2026-02-24T13:05:42Z

@ShravaniVangur @parth-gr I had one question here, the virt storageclass that we create should be created on top of the default radosnamespace or the custom radosnamespace?

Should be created directly on top of the pool,
Should we also consider the replica-1 and rados namespace case for this?

controllers/storagecluster/external_resources.go

Madhu-1 · 2026-02-26T08:18:06Z

controllers/storagecluster/external_resources.go

+				instance.Namespace,
+				"",
+				"",
+				instance.Spec.ManagedResources.CephBlockPools.DefaultVirtualizationStorageClass,


What happens if we already have a default virtualization SC for internal mode? dont we need to error out?

Users are responsible for ensuring that the default field is marked on only one StorageCluster CR. The system does not perform validation to detect conflicts across multiple StorageCluster CRs.

openshift-ci · 2026-03-02T08:07:16Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Madhu-1, ShravaniVangur
Once this PR has been reviewed and has the lgtm label, please ask for approval from travisn. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Madhu-1 · 2026-03-02T08:08:50Z

@ShravaniVangur @parth-gr I had one question here, the virt storageclass that we create should be created on top of the default radosnamespace or the custom radosnamespace?

@parth-gr are we supporting two rados namespace in external mode? it is currently one right. we dont support multiple Radosnamespace in the same ODF cluster.

parth-gr · 2026-03-02T08:13:46Z

@ShravaniVangur @parth-gr I had one question here, the virt storageclass that we create should be created on top of the default radosnamespace or the custom radosnamespace?

@parth-gr are we supporting two rados namespace in external mode? it is currently one right. we dont support multiple Radosnamespace in the same ODF cluster.

Yes currently we support one per ocp cluster

controllers/storagecluster/external_resources.go

Creates the virtualization storage class when the kubevirt crd is present. Signed-off-by: ShravaniVangur <shravanivangur@gmail.com>

openshift-ci · 2026-03-02T08:42:31Z

New changes are detected. LGTM label has been removed.

parth-gr · 2026-03-02T09:25:37Z

controllers/storagecluster/external_resources.go

+	// create virtualization storageclass if VirtualMachineCRD is present
+	var scc StorageClassConfiguration
+	crd := &metav1.PartialObjectMetadata{}
+	crd.SetGroupVersionKind(extv1.SchemeGroupVersion.WithKind("CustomResourceDefinition"))


is getting a crd is sufficient? or we should loook for a cr?

The StorageClasses are created based on the availability of a CRD. So the check should be sufficient.

parth-gr

Overall looks good to me,
Can you do a one round of testing,

Maybe you can just simulate the virt env by creating the crd

rewantsoni · 2026-03-03T08:39:37Z

@ShravaniVangur @parth-gr I had one question here, the virt storageclass that we create should be created on top of the default radosnamespace or the custom radosnamespace?

Should be created directly on top of the pool, Should we also consider the replica-1 and rados namespace case for this?

directly on top of the pool -> That means the non-default rns right? This means that a single ocp cluster can have one storage class on a Rados namespace and another one on a non-default RNS (virt storageclass)

parth-gr · 2026-03-03T15:08:29Z

@ShravaniVangur @parth-gr I had one question here, the virt storageclass that we create should be created on top of the default radosnamespace or the custom radosnamespace?

Should be created directly on top of the pool, Should we also consider the replica-1 and rados namespace case for this?

directly on top of the pool -> That means the non-default rns right? This means that a single ocp cluster can have one storage class on a Rados namespace and another one on a non-default RNS (virt storageclass)

No only for the non-virt class, currently
In case we need it for the virt class, we need to change the implementation.

rewantsoni · 2026-03-04T06:41:02Z

controllers/storagecluster/external_resources.go

+	if crd.UID != "" {
+		scc = StorageClassConfiguration{
+			storageClass: util.NewDefaultVirtRbdStorageClass(
+				instance.Namespace,


@ShravaniVangur @parth-gr I think this is not right, this means that there can be a storageclass with clientProfile as raodsnamespace
and another storageclass virt one with clientProfile as openshift-storage-external.

What we should do is

If there is a radosnamespace in externalResources, then we create all rbd classes on top of that radosnamespace client profile.

If there is no radosnamespace, then we should create the classes on top of the default client profile.

This should also apply to the topology based storageclass for external mode.
cc: @Madhu-1 @travisn

Yes i discussed with @rewantsoni on this one, looks like we do have bug in the extrernal mode where when topology based provisioning is used its not using the rados namespace , its using detault one.
we have a hacky code that is modifying the clusterID to match the radosnamespace. we should apply for this one as well and we need to fix the topology based SC

openshift-ci bot requested a review from parth-gr February 19, 2026 07:47

parth-gr suggested changes Feb 19, 2026

View reviewed changes

openshift-ci bot assigned parth-gr Feb 19, 2026

openshift-ci bot assigned Madhu-1 and travisn Feb 19, 2026

ShravaniVangur force-pushed the RHSTOR-8247 branch from 3c636ae to 481fb61 Compare February 19, 2026 14:49

ShravaniVangur force-pushed the RHSTOR-8247 branch from 481fb61 to daf463b Compare February 26, 2026 07:22

Madhu-1 reviewed Feb 26, 2026

View reviewed changes

ShravaniVangur force-pushed the RHSTOR-8247 branch from daf463b to dafa795 Compare March 2, 2026 07:32

ShravaniVangur requested review from Madhu-1 and parth-gr March 2, 2026 07:44

Madhu-1 approved these changes Mar 2, 2026

View reviewed changes

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 2, 2026

parth-gr reviewed Mar 2, 2026

View reviewed changes

controllers/storagecluster/external_resources.go Outdated Show resolved Hide resolved

external: add Openshift Virt Storage Class when crd is present

9db195c

Creates the virtualization storage class when the kubevirt crd is present. Signed-off-by: ShravaniVangur <shravanivangur@gmail.com>

ShravaniVangur force-pushed the RHSTOR-8247 branch from dafa795 to 9db195c Compare March 2, 2026 08:42

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Mar 2, 2026

parth-gr reviewed Mar 2, 2026

View reviewed changes

rewantsoni suggested changes Mar 4, 2026

View reviewed changes

openshift-ci bot assigned rewantsoni Mar 4, 2026

Conversation

ShravaniVangur commented Feb 18, 2026

Uh oh!

ShravaniVangur commented Feb 19, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

parth-gr commented Feb 19, 2026

Uh oh!

rewantsoni commented Feb 24, 2026

Uh oh!

parth-gr commented Feb 24, 2026

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Mar 2, 2026

Uh oh!

Madhu-1 commented Mar 2, 2026

Uh oh!

parth-gr commented Mar 2, 2026

Uh oh!

Uh oh!

openshift-ci bot commented Mar 2, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

parth-gr left a comment

Choose a reason for hiding this comment

Uh oh!

rewantsoni commented Mar 3, 2026

Uh oh!

parth-gr commented Mar 3, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants