Skip to content

Add elastic pool reference support to MSSQLDatabase#1198

Merged
jonasz-lasut merged 1 commit intocrossplane-contrib:mainfrom
nicolamacoir:feat/mssqldatabase-elastic-pool-ref
Apr 23, 2026
Merged

Add elastic pool reference support to MSSQLDatabase#1198
jonasz-lasut merged 1 commit intocrossplane-contrib:mainfrom
nicolamacoir:feat/mssqldatabase-elastic-pool-ref

Conversation

@nicolamacoir
Copy link
Copy Markdown
Contributor

@nicolamacoir nicolamacoir commented Apr 16, 2026

Summary

  • Add elasticPoolIdRef and elasticPoolIdSelector support to MSSQLDatabase, allowing declarative references to MSSQLElasticPool resources instead of requiring hard-coded Azure resource IDs
  • Follows the same pattern already used for serverIdRef/serverIdSelector

Fixes #1197

Test plan

  • Verify code generation produces the expected elasticPoolIdRef and elasticPoolIdSelector fields in the CRD types
  • Validate that referencing an MSSQLElasticPool by name resolves correctly

@jonasz-lasut
Copy link
Copy Markdown
Collaborator

jonasz-lasut commented Apr 16, 2026

Thank you @nicolamacoir for opening the PR!
Could you remove the Account resource in the examples for the MSSQLDatabase in cluster v1beta2 and namespaced v1beta1 as it's not used in the E2E tests for this resource and from the looks of it will lead to test failure as account has name example?

EDIT:
Please replace it with a password secret, similarly as in elasticpool:

apiVersion: v1
data:
example-key: dGVzdFBhc3N3b3JkITEyMw==
kind: Secret
metadata:
name: example-secret
namespace: upbound-system
type: Opaque

@jonasz-lasut
Copy link
Copy Markdown
Collaborator

jonasz-lasut commented Apr 16, 2026

I haven't noticed that there's no updates to CRDs.
Have you run make generate after updating the config.go file?

Please update config/namespaced/sql/config.go as well with the new reference

@nicolamacoir
Copy link
Copy Markdown
Contributor Author

I haven't noticed that there's no updates to CRDs. Have you run make generate after updating the config.go file?

Please update config/namespaced/sql/config.go as well with the new reference

Hi @jonasz-lasut
Thanks for your response. I thought this was done through CI; I'll generate the CRDs and commit them straight away.

@jonasz-lasut
Copy link
Copy Markdown
Collaborator

/test-examples="examples/sql/namespaced/v1beta1/mssqldatabase.yaml"

@jonasz-lasut
Copy link
Copy Markdown
Collaborator

jonasz-lasut commented Apr 16, 2026

The E2E test has failed because the server name was not globally unique

        create failed: async create failed: failed to create the resource: [{0 creating Server (Subscription: "2895a7df-ae9f-41b8-9e78-3ce4926df838"
        Resource Group Name: "example"
        Server Name: "example"): polling after CreateOrUpdate: polling failed: the Azure API returned the following error:

        Status: "NameAlreadyExists"
        Code: ""
        Message: "The name 'example.database.windows.net' already exists. Choose a different name."
        Activity Id: ""

        ---

        API Response:

        ----[start]----
        {"name":"7ec5defe-8d91-4bb1-8146-9526b7172fe3","status":"Failed","startTime":"2026-04-16T12:57:28.287Z","error":{"code":"NameAlreadyExists","message":"The name 'example.database.windows.net' already exists. Choose a different name."}}
        -----[end]-----
          []}]

Could you change the server's (MSSQLServer) name in the examples to mssqldatabasev1beta2 and mssqldatabasev1beta1ns respectively for cluster example and namespaced example?

Comment thread examples/sql/cluster/v1beta2/mssqldatabase.yaml Outdated
Comment thread examples/sql/namespaced/v1beta1/mssqldatabase.yaml Outdated
@jonasz-lasut
Copy link
Copy Markdown
Collaborator

/test-examples="examples/sql/namespaced/v1beta1/mssqldatabase.yaml"

@nicolamacoir
Copy link
Copy Markdown
Contributor Author

Hi @jonasz-lasut
Thanks for your guidance!

I think the latest error is related to timeouts. The test ran for 1211 seconds and was killed (signal: killed). The resource status was empty. This really does look like it just timed out waiting for the Azure resources to provision. The signal: killed confirms the process was terminated externally.

The elastic pool example uses uptest.upbound.io/timeout: "3600" its MSSQLServer, I guess for exactly this reason? I've added it to the mssqldatabase resource as well.

@jonasz-lasut
Copy link
Copy Markdown
Collaborator

jonasz-lasut commented Apr 16, 2026

This time it was a misconfiguration on a Database resource, MSSQL was not heavily tested and we did not have any new features for a long time so the examples may be a little stałe

Here's the screenshot as I can't easily copy-paste YAML on my phone 😓

Screenshot_20260416-214507.png

You can always find the dump of a full controlplane in the E2E job artifacts. There's a file called all-crossplane.yaml which contains the managed resources, CRDs and everything else related to the test

@jonasz-lasut
Copy link
Copy Markdown
Collaborator

/test-examples="examples/sql/namespaced/v1beta1/mssqldatabase.yaml

@jonasz-lasut
Copy link
Copy Markdown
Collaborator

jonasz-lasut commented Apr 17, 2026

ResourceGroup is failing because it's calling example and we already have a resource group with this name in another region.

I'd propose to change the RG name similarly as with MSSQLServer - mssqldatabasev1beta2 and mssqldatabasev1beta1ns respectively for cluster example and namespaced example

@jonasz-lasut
Copy link
Copy Markdown
Collaborator

/test-examples="examples/sql/namespaced/v1beta1/mssqldatabase.yaml"

@nicolamacoir
Copy link
Copy Markdown
Contributor Author

Hi @jonasz-lasut,

I found another bad reference (the ResourceGroup name collision). Hopefully all is good now.

I've also took the opportunity to add an elastic pool to the example so that the new elasticPoolIdSelector reference is actually tested end-to-end.

@nicolamacoir
Copy link
Copy Markdown
Contributor Author

Hi @jonasz-lasut. Could you give the test another hit pls? Would be great to have this feature merged and released. Thanks!

@jonasz-lasut
Copy link
Copy Markdown
Collaborator

Hi @nicolamacoir
Have you checked a root cause why the previous E2E run has failed? I did not have time to check it and if it's not resource spec related then we should solve it first

@nicolamacoir
Copy link
Copy Markdown
Contributor Author

Hi @nicolamacoir Have you checked a root cause why the previous E2E run has failed? I did not have time to check it and if it's not resource spec related then we should solve it first

I’ve identified the root cause. (I just discovered how to check the artifacts as you suggested before; apologies for my ignorance.)

        create failed: async create failed: failed to create the resource: [{0 creating Server (Subscription: "2895a7df-ae9f-41b8-9e78-3ce4926df838"
        Resource Group Name: "mssqldatabasev1beta1ns"
        Server Name: "mssqldatabasev1beta1ns"): polling after CreateOrUpdate: polling failed: the Azure API returned the following error:

        Status: "NameAlreadyExists"
        Code: ""
        Message: "The name 'mssqldatabasev1beta1ns' already exists. Choose a different name."
        Activity Id: ""

I’m puzzled by the existence of that database. I couldn’t find any other matching resources in the repository. Perhaps I lack sufficient understanding or context about the setup of the E2E test suite. I would assume that all resources are deleted after each run.

@jonasz-lasut
Copy link
Copy Markdown
Collaborator

jonasz-lasut commented Apr 22, 2026

They should be, we may have caught a bug :) For now you can change the name in examples so we can run the e2e suite and I'll open a thread on maintainer slack that the resources are not cleaned up

mssqldatabase... --> sqldatabase... in examples should be fine

EDIT: found a bug in namespaced resources cleanup, I'll run cluster tests for now

@jonasz-lasut
Copy link
Copy Markdown
Collaborator

/test-examples="examples/sql/cluster/v1beta2/mssqldatabase.yaml"

@jonasz-lasut
Copy link
Copy Markdown
Collaborator

jonasz-lasut commented Apr 22, 2026

Good that we've tested with the cluster version.
We don't backport changes between versions automatically and as of now there's no automated storage version conversions between e.g. v1beta1 and v1beta2 (there'a an effort from Sergen but it's not merged here yet).

You'll need to manually copy the diff in apis/cluster/sql/v1beta2/zz_mssqldatabase_types.go to apis/cluster/sql/v1beta1/zz_mssqldatabase_types.go as well and run make generate to update the v1beta1 version which is also a storage version

I'd also appreciate if you could squash your commits before we can proceed with merging the code later

@nicolamacoir nicolamacoir force-pushed the feat/mssqldatabase-elastic-pool-ref branch from 4b73890 to 74e711a Compare April 22, 2026 14:56
Comment thread apis/cluster/sql/v1beta1/zz_mssqldatabase_types.go Outdated
Comment thread apis/cluster/sql/v1beta1/zz_mssqldatabase_types.go Outdated
@jonasz-lasut
Copy link
Copy Markdown
Collaborator

Appreciate the quick fix, I've added a small comment as you've backported the new fields into Observation in v1beta1 as well and they're not supposed to be there.

You'll have to remove them from the commented file and run make generate again

Add elasticPoolIdRef/elasticPoolIdSelector fields to MSSQLDatabase,
allowing users to declaratively reference an MSSQLElasticPool resource
instead of hard-coding the Azure resource ID.

Changes:
- Add elastic_pool_id reference config for both cluster and namespaced
  providers pointing to azurerm_mssql_elasticpool
- Backport reference fields to v1beta1 (InitParameters and Parameters
  only, not Observation)
- Regenerate CRDs, types, resolvers, and deepcopy methods
- Rewrite examples following the proven elastic pool pattern with
  unique resource names and labels to avoid E2E test conflicts
- Add MSSQLElasticPool to examples to test the new reference

Fixes crossplane-contrib#1197

Signed-off-by: Nicola <nicola.macoir@rightcrowd.com>
@nicolamacoir nicolamacoir force-pushed the feat/mssqldatabase-elastic-pool-ref branch from 74e711a to 606cd78 Compare April 23, 2026 07:48
@jonasz-lasut
Copy link
Copy Markdown
Collaborator

/test-examples="examples/sql/cluster/v1beta2/mssqldatabase.yaml"

@jonasz-lasut jonasz-lasut self-requested a review April 23, 2026 08:26
Copy link
Copy Markdown
Collaborator

@jonasz-lasut jonasz-lasut left a comment

Choose a reason for hiding this comment

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

LGTM @nicolamacoir
Really appreciate all the effort you've put into this PR!

@jonasz-lasut jonasz-lasut merged commit 9bd8f99 into crossplane-contrib:main Apr 23, 2026
9 checks passed
@nicolamacoir
Copy link
Copy Markdown
Contributor Author

Thanks for all your help and quick guidance!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add elastic pool reference support to MSSQLDatabase

2 participants