Skip to content

Keystone: Implement RegisteredLimit Controller#782

Open
gndrmnn wants to merge 10 commits into
k-orc:mainfrom
gndrmnn:implement_registeredlimit
Open

Keystone: Implement RegisteredLimit Controller#782
gndrmnn wants to merge 10 commits into
k-orc:mainfrom
gndrmnn:implement_registeredlimit

Conversation

@gndrmnn

@gndrmnn gndrmnn commented May 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

WIP

  • No support for RegionID as k-orc does not have that resource currently
  • RegisteredLimits are unnamed, but have a unique constraint over ResourceName, RegionID, and DefaultValue leading to some interesting challengens. E.g. Declaring two yaml files with two RegisteredLimits with the same RegionID and ResourceName will race to change the same entry.

@github-actions

github-actions Bot commented May 11, 2026

Copy link
Copy Markdown

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

@gndrmnn
gndrmnn force-pushed the implement_registeredlimit branch from 22a08f7 to e46ac56 Compare May 12, 2026 14:31
@github-actions github-actions Bot added the semver:major Breaking change label May 12, 2026
@gndrmnn
gndrmnn force-pushed the implement_registeredlimit branch from e46ac56 to 4964e1d Compare May 13, 2026 13:40
@gndrmnn
gndrmnn force-pushed the implement_registeredlimit branch 4 times, most recently from 8379469 to 027f4aa Compare June 1, 2026 14:23
@gndrmnn
gndrmnn force-pushed the implement_registeredlimit branch 4 times, most recently from b6de206 to e46d4a0 Compare June 29, 2026 14:10
@gndrmnn
gndrmnn force-pushed the implement_registeredlimit branch from e46d4a0 to d91dcf9 Compare July 13, 2026 09:25
gndrmnn added 6 commits July 13, 2026 11:43
@gndrmnn
Scaffolding for the RegisteredLimit controller
95335a9 
$ go run ./cmd/scaffold-controller -interactive=false \
    -kind=RegisteredLimit \
    -gophercloud-client=NewIdentityV3 \
    -gophercloud-module=github.com/gophercloud/gophercloud/v2/openstack/identity/v3/registeredlimits \
    -gophercloud-type=RegisteredLimit \
    -openstack-json-object=registered_limits \
    -required-create-dependency=Service

On-behalf-of: SAP nils.gondermann@sap.com
@gndrmnn
Scaffolding for the RegisteredLimit controller
0f876a3 
Register with the resource generator

On-behalf-of: SAP nils.gondermann@sap.com
@gndrmnn
Scaffolding for the RegisteredLimit controller
2b23ba7 
Add the OpenStack client to scope

On-behalf-of: SAP nils.gondermann@sap.com
@gndrmnn
Scaffolding for the RegisteredLimit controller
4d13d7b 
Register the controller

On-behalf-of: SAP nils.gondermann@sap.com
@gndrmnn
RegisteredLimit Adapt to BatchCreate API
b954d46 
On-behalf-of: SAP nils.gondermann@sap.com
@gndrmnn
RegisteredLimit Type Declaration
390c88d 
On-behalf-of: SAP nils.gondermann@sap.com
@gndrmnn
gndrmnn force-pushed the implement_registeredlimit branch 9 times, most recently from 767c683 to 4c2f1fe Compare July 14, 2026 09:55
gndrmnn added 2 commits July 14, 2026 14:01
@gndrmnn
RegisteredLimit Actuator and Status
85327f8 
On-behalf-of: SAP nils.gondermann@sap.com
@gndrmnn
RegisteredLimit Tests and Config Samples
eec981d 
On-behalf-of: SAP nils.gondermann@sap.com
gndrmnn added 2 commits July 14, 2026 14:01
@gndrmnn
RegisteredLimit Actuator mutability
6a01211 
On-behalf-of: SAP nils.gondermann@sap.com
@gndrmnn
Scaffolding for the RegisteredLimit controller
8c90198 
Generate the OLM bundle

On-behalf-of: SAP nils.gondermann@sap.com
@gndrmnn
gndrmnn force-pushed the implement_registeredlimit branch from 4c2f1fe to 8c90198 Compare July 14, 2026 12:02
@gndrmnn

gndrmnn commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

@mandre

Hi, this PR is more or less good to be reviewed. I am currently running into a strange problem though: In the registeredlimit-import test we are creating a mandatory Service to use in the import filter. In some cases, the CI pipeline will fail with Waiting for service/<service name> to be ready. Other times, it successfully proceeds to Waiting for OpenStack resource to be created externally. As you can see right now, two CI tests failed while the epoxy one passed.

I have already setup a local keystone instance to test this against, and I can not reproduce the Waiting for service/<service name> to be ready problem. So it looks like this is some kind of timing problem in the test maybe?

What makes this especially weird is, I have looked at the Endpoint actuator and used it as inspiration for this PR. The Endpoint also requires a mandatory ServiceRef and even declares it the same way as I do in the endpoint-import CI test. That test however does not flap at all. So I seem to be doing something wrong, I can not put my finger on right now.

@gndrmnn
gndrmnn marked this pull request as ready for review July 15, 2026 07:02
@mandre

mandre commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

@mandre

Hi, this PR is more or less good to be reviewed. I am currently running into a strange problem though: In the registeredlimit-import test we are creating a mandatory Service to use in the import filter. In some cases, the CI pipeline will fail with Waiting for service/<service name> to be ready. Other times, it successfully proceeds to Waiting for OpenStack resource to be created externally. As you can see right now, two CI tests failed while the epoxy one passed.

Hi @gndrmnn, I won't be able to look at this right away. Perhaps @eshulman2, @dlaw4608, or @winiciusallan can do a first round of reviews, and might have ideas on what could be causing the tests to fail. I'll look at this when I'm back.

winiciusallan
winiciusallan reviewed Jul 17, 2026
View reviewed changes

@winiciusallan winiciusallan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @gndrmnn, try the suggestion and let's see how the tests behave. In the meantime, I'll review the rest of your code. Thanks for working on this.

Comment thread internal/controllers/registeredlimit/controller.go
},
finalizer, externalObjectFieldOwner,
)

@winiciusallan winiciusallan Jul 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're missing a serviceImportDependency here. What it's maybe happening is that your service/registeredlimit-import become available at some point, but your controller are not reconciling it again due to the missing watcher here, and it waits for OpenStack forever.

You can use the endpoint controller as a reference

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@winiciusallan winiciusallan winiciusallan left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

semver:major Breaking change

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@gndrmnn @mandre @winiciusallan