Manila: Share network by dlaw4608 · Pull Request #691 · k-orc/openstack-resource-controller

dlaw4608 · 2026-02-24T18:05:44Z

Add support for Openstack Manila Service Share Network resource
Closes: #687

github-actions · 2026-02-24T18:06:03Z

Failed to assess the semver bump. See logs for details.

winiciusallan

Hey @dlaw4608, I left a few comments, but I need some more time to take a look at the other test scenarios. For now, let me know what you think about what it is commented.

winiciusallan · 2026-03-01T18:52:21Z

-const ShareNetworkStatusInUse     = "in-use"
-const ShareNetworkStatusDeleting  = "deleting"
+
+const (


I believe we're not using any of these statuses throughout the code, and it looks like we don't have a status field in the ShareNetwork struct on Gophercloud's side. Is this useful?

Maybe do we need to create an issue to address this?

Yes you are correct they are not necessary here I removed them, thanks 👍

winiciusallan · 2026-03-01T19:00:19Z

+    - celExpr: "sharenetwork.status.resource.name == 'sharenetwork-create-full-override'"
+    - celExpr: "sharenetwork.status.resource.description == 'ShareNetwork from \"create full\" test'"


We don't need this check since KUTTL already matches fields from declaration vs status.

Thanks for the Feedback @winiciusallan , i could of sworn i had already left these comments last week but clearly never actually clicked the big green button ack.

It happens 😄

eshulman2

Generally looks good, dded a few comments and question, please LMK what you think.

eshulman2 · 2026-03-09T09:44:19Z

        enable_workaround_docker_io: 'false'
        branch: ${{ matrix.openstack_version }}
-        enabled_services: "openstack-cli-server,neutron-trunk"
+        enabled_services: "openstack-cli-server,neutron-trunk,manila,m-api,m-sch,m-shr,m-dat"


This comment is not specific to this patch but I believe we should consider a coherent approach to the services we enable in the gates and possibly making specific gates with specific services enabled for certain controllers like being done in gophercloud for loadbalancers for example. @mandre WDYT?

Agreed. At some point we'll have to split our jobs as they grow bigger (gophercloud is a good example).

eshulman2 · 2026-03-09T09:48:53Z

+	// networkRef is a reference to the ORC Network which this resource is associated with.
+	// +optional
+	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="networkRef is immutable"
+	NetworkRef *KubernetesNameRef `json:"networkRef"`


Should there be a rule here that make sure that if a network is set the subnet is also set? from the api referance:

The UUID of a neutron network when setting up or updating a share network subnet with neutron. Specify both a neutron network and a neutron subnet that belongs to that neutron network.

I agree! , I created a new validation rule

// +kubebuilder:validation:XValidation:rule="has(self.networkRef) == has(self.subnetRef)",message="networkRef and subnetRef must be specified together"

also added a test file in the apivalidations directory here, I used securitygroup_test.go and network_test.go as my main sources of reference (both in the apivalidations dir),

The CEL rule is a boolean equality and tests 4 combinations, both networkRef and subnetRef present = valid, both networkRef and subnetRef absent = valid aswell, but if either network Ref or subnetRef is missing or vice versa it is flagged as invalid.
The idea is to follow the API Ref statement creating a both or neither configuration rule.

The UUID of a neutron network when setting up or updating a share network subnet with neutron. Specify both a neutron network and a neutron subnet that belongs to that neutron network.

please lmk what you think 👍

eshulman2

regarding the gate failures

eshulman2 · 2026-03-09T11:57:31Z

+      ref: sharenetwork
+assertAll:
+    - celExpr: "sharenetwork.status.id != ''"
+    - celExpr: "sharenetwork.status.resource.neutronNetID == '"


seems like there is something wrong with the test here:

harness.go:395: failed to prepare expression evaluation: failed to load expression(s): failed to build CEL program from expression "sharenetwork.status.resource.neutronNetID == '": type-check error: ERROR: <input>:1:46: Syntax error: token recognition error at: ''' | sharenetwork.status.resource.neutronNetID == ' | .............................................^ ERROR: <input>:1:47: Syntax error: mismatched input '<EOF>' expecting {'[', '{', '(', '.', '-', '!', 'true', 'false', 'null', NUM_FLOAT, NUM_INT, NUM_UINT, STRING, BYTES, IDENTIFIER} | sharenetwork.status.resource.neutronNetID == ' | ..............................................^

I suspect it is a small typo yo have only a single ' instead of what I assumed you mean which is ' ' but I might be wrong

Good Spot, thanks for the help @eshulman2 👍

github-actions · 2026-03-09T17:04:19Z

Failed to assess the semver bump. See logs for details.

mandre

Just an initial pass for now, I believe we should make the dependencies immutable unless we have more test coverage proving it works as expected, and that the resources that are no longer dependencies no longer have a finalizer.

mandre · 2026-03-10T15:15:46Z

        enable_workaround_docker_io: 'false'
        branch: ${{ matrix.openstack_version }}
-        enabled_services: "openstack-cli-server,neutron-trunk"
+        enabled_services: "openstack-cli-server,neutron-trunk,manila,m-api,m-sch,m-shr,m-dat"


Agreed. At some point we'll have to split our jobs as they grow bigger (gophercloud is a good example).

mandre · 2026-03-10T16:34:57Z

  resource:
    name: sharenetwork-create-full-override
    description: ShareNetwork from "create full" test
-    # TODO(scaffolding): Add all fields the resource supports


What about SegmentationID, CIRD, NetworkType, ProjectID, etc. ? We should test their value, or lack thereof.

You are correct, I have included comments noting the missing checks for these fields, I'm OK with this being a partial implementation/first iteration, so that I can use this resource as a dependency in the Share Controller and then return to fully implementing this controller in a separate PR?, WDYT @mandre

mandre · 2026-03-10T16:50:05Z

 status:
  resource:
    name: sharenetwork-create-minimal
-    # TODO(scaffolding): Add all fields the resource supports


There too, we should make sure the resulting resource is exactly as we expect it to be, including for absence of fields in its status.

Answered above ☝️

mandre · 2026-03-10T16:51:29Z

  managementPolicy: managed
-  # TODO(scaffolding): Only add the mandatory fields. It's possible the resource
-  # doesn't have mandatory fields, in that case, leave it empty.
  resource: {}


mandre · 2026-03-10T17:06:55Z

  resource:
    name: sharenetwork-update-updated
    description: sharenetwork-update-updated
-    # TODO(scaffolding): match all fields that were modified


We need to validate that updating networkRef and subnetRef had any effect.

Removed as they are both set as immutable.

mandre · 2026-03-10T17:10:20Z

 	handleNameUpdate(&updateOpts, obj, osResource)
 	handleDescriptionUpdate(&updateOpts, resource, osResource)

-	// TODO(scaffolding): add handler for all fields supporting mutability


We should add a dependence on network and subnet here, and ensure we set the updateOpts correctly.

We may also need to remove the finalizer on the old dependencies, that might be a tricky one to handle. I would suggest you make the dependencies immutable for now.

Agreed, the dependencies are set as immutable for the time being 👍

go run ./cmd/scaffold-controller -interactive=false \ -kind=ShareNetwork \ -gophercloud-client=NewSharedFilesystemV2 \ -gophercloud-module=github.com/gophercloud/gophercloud/v2/openstack/sharedfilesystems/v2/sharenetworks \ -gophercloud-type=ShareNetwork \ -openstack-json-object=share_network \ -optional-create-dependency=Network \ -optional-create-dependency=Subnet Signed-off-by: Daniel Lawton <dlawton@redhat.com>

dlaw4608 · 2026-06-04T08:47:20Z

I think this PR is good to go for another look @mandre

mandre

I haven't finished reviewing, but leaving some comments here already.

mandre · 2026-06-05T06:02:32Z

+      ref: sharenetwork
+assertAll:
+    - celExpr: "sharenetwork.status.id != ''"
+    - celExpr: "sharenetwork.status.resource.neutronNetID == ''"


We now need to ensure these fields are unset.

Implements ShareNetwork controller to manage Manila share networks. - E2E tests included - API configured - Manila enabled in CI Signed-off-by: Daniel Lawton <dlawton@redhat.com> Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

dlaw4608 force-pushed the share_network branch from 4f9c96e to 46a1765 Compare February 24, 2026 18:13

github-actions Bot added the semver:major Breaking change label Feb 24, 2026

dlaw4608 force-pushed the share_network branch 2 times, most recently from 79282c8 to 08867bf Compare February 24, 2026 19:10

dlaw4608 marked this pull request as ready for review February 25, 2026 10:21

winiciusallan reviewed Mar 1, 2026

View reviewed changes

dlaw4608 force-pushed the share_network branch 2 times, most recently from b5292e8 to dacb270 Compare March 4, 2026 18:29

eshulman2 reviewed Mar 9, 2026

View reviewed changes

dlaw4608 force-pushed the share_network branch from dacb270 to 07f8a13 Compare March 9, 2026 17:04

github-actions Bot removed the semver:major Breaking change label Mar 9, 2026

dlaw4608 force-pushed the share_network branch from 07f8a13 to c9974c3 Compare March 9, 2026 17:35

github-actions Bot added the semver:major Breaking change label Mar 9, 2026

dlaw4608 force-pushed the share_network branch from c9974c3 to bc22e2b Compare March 10, 2026 13:10

mandre reviewed Mar 10, 2026

View reviewed changes

eshulman2 mentioned this pull request Mar 12, 2026

Load balancer support #638

Open

dlaw4608 force-pushed the share_network branch 6 times, most recently from 995c9dd to 5a7e8ba Compare March 16, 2026 18:46

dlaw4608 force-pushed the share_network branch 3 times, most recently from af8bc6d to 4caee9b Compare June 2, 2026 15:33

mandre reviewed Jun 4, 2026

View reviewed changes

dlaw4608 force-pushed the share_network branch from 4caee9b to 4a092dc Compare June 4, 2026 14:32

mandre reviewed Jun 5, 2026

View reviewed changes

dlaw4608 force-pushed the share_network branch 2 times, most recently from 8aa3bed to 3efb578 Compare June 5, 2026 10:05

Added ShareNetwork Controller for Manila Service

fa2108c

Implements ShareNetwork controller to manage Manila share networks. - E2E tests included - API configured - Manila enabled in CI Signed-off-by: Daniel Lawton <dlawton@redhat.com> Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

dlaw4608 force-pushed the share_network branch from 3efb578 to fa2108c Compare June 5, 2026 11:14

mandre approved these changes Jun 5, 2026

View reviewed changes

mandre added this pull request to the merge queue Jun 5, 2026

Merged via the queue into k-orc:main with commit 46a7c89 Jun 5, 2026
12 checks passed

		- celExpr: "sharenetwork.status.resource.name == 'sharenetwork-create-full-override'"
		- celExpr: "sharenetwork.status.resource.description == 'ShareNetwork from \"create full\" test'"

Conversation

dlaw4608 commented Feb 24, 2026

Uh oh!

github-actions Bot commented Feb 24, 2026

Uh oh!

winiciusallan left a comment

Choose a reason for hiding this comment

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eshulman2 left a comment

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!

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!

Uh oh!

Uh oh!

eshulman2 left a comment

Choose a reason for hiding this comment

Uh oh!

eshulman2 Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Mar 9, 2026

Uh oh!

mandre left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

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!

Choose a reason for hiding this comment

Uh oh!

dlaw4608 commented Jun 4, 2026

Uh oh!

mandre left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

eshulman2 Mar 9, 2026 •

edited

Loading