Make it easier to choose Azure SQL SKU

[yorek]

Description

Using the Azure SQL DB Free Offer as default deployment option. Added also the ability to specify the desired SKU.

Fixes # (issue)

#8876

Checklist

• Is this feature complete?
  • [x] Yes. Ready to ship.
  • [ ] No. Follow-up changes expected.
• Are you including unit tests for the changes and scenario tests if relevant?
  • [x] Yes
  • [ ] No
• Did you add public API?
  • [ ] Yes
    • If yes, did you have an API Review for it?
      • [ ] Yes
      • [ ] No
    • Did you add <remarks /> and <code /> elements on your triple slash comments?
      • [ ] Yes
      • [ ] No
  • [x] No
• Does the change make any security assumptions or guarantees?
  • [ ] Yes
    • If yes, have you done a threat model and had a security review?
      • [ ] Yes
      • [ ] No
  • [x] No
• Does the change require an update in our Aspire docs?
  • [x] Yes
    • Link to aspire-docs issue (consider using one of the following templates):
      • New (or update) doc-idea template
      • New breaking-change template
      • New diagnostic template
  • [ ] No

[yorek]

@dotnet-policy-service agree company="Microsoft"

[davidfowl]

You made a breaking API change

[yorek]

You made a breaking API change

I added a new optional parameter to AddDatabase, is that considered a breaking change?

I guess, otherwise, you are referring to the change to the AzureSqlDatabaseResource that now requires the SKU, but this is used only in the AddDatabase method, which still allows for the same syntax as before this change.

Let me know what you think should be changed

[davidfowl]

You can’t add a new required parameters to the constructor. That’s why I suggested a WithSku method. The sku is optional

[yorek]

You can’t add a new required parameters to the constructor. That’s why I suggested a WithSku method. The sku is optional

Ok. I checked where the constructor was used and it was used only in the AddDatabase so I thought it would have been fine, as it seems more natural to me to specify which SKU do you want when you call the AddDatabase method instead of having to discover and learn that there is another method to use to set the SKU.

I'll do the change in the next days it shouldn't take long

[davidfowl]

You can do it there but it’s still optional on the database itself

[yorek]

Done, it was easier that I thought :) Let me know if this is better.

[eerhardt]

This is looking pretty good.

Can you open a breaking change doc issue: New breaking-change template, since we are changing the default SKU here. We should have it in our docs that this change happened.

[yorek]

This is looking pretty good.

Can you open a breaking change doc issue: New breaking-change template, since we are changing the default SKU here. We should have it in our docs that this change happened.

Done: https://github.com/dotnet/docs-aspire/issues/3144

[davidfowl]

Shouldn’t we be exposing the cdk type?

[eerhardt]

Shouldn’t we be exposing the cdk type?

What does this mean, specifically?

We have a ConfigureInfrastructure on the Sql server resource, where you can get the child cdk objects for the SqlDatabase, and manipulate them.

[davidfowl]

I believe the underlying Azure.Provisioning is a SqlSku type

[yorek]

Anything left to do in order to have this PR merged?

[davidfowl]

@eerhardt make sure we expose Sqlsku and not a string (see primitive obsession)

[yorek]

@eerhardt make sure we expose Sqlsku and not a string (see primitive obsession)

If that is needed, then adding WithSku is not enough, as there is no "free" Sku, but there is a free offer that can be applied to the database, and that will use a specific Sku. I propose to introduce a new WithFreeOffer method that can be used as an alternative to WithSku. The new method will set the correct Sku to be used with the free offer and enable the free option.

For example:

var db = sql.AddDatabase("db", "dbName").WithFreeOffer();
Assert.Equal("GP_S_Gen5_2", db.Sku.Name);

using WithSku and WithFreeOffer will result in an exception if the WithSku try to set the sku to something different then GP_S_Gen5_2 (as this is the one used by the Free Offer)

Thoughts?

[sebastienros]

What about only WithSku() and use a constant for the free/basic/... offers that return the correct SqlSku?

[yorek]

What about only WithSku() and use a constant for the free/basic/... offers that return the correct SqlSku?

Not sure I follow. There is no "free" Sku...you can set the free offer option and then the Sku will be set automatically to "GP_S_Gen5_2"...but you can set that Sku even without benefitting from the free offer.

[davidfowl]

Not sure I follow. There is no "free" Sku...you can set the free offer option and then the Sku will be set automatically to "GP_S_Gen5_2"...but you can set that Sku even without benefitting from the free offer.

var db = sql.AddDatabase("db", "dbName").WithSku(SqlSku.FreeOffer);

[yorek]

Not sure I follow. There is no "free" Sku...you can set the free offer option and then the Sku will be set automatically to "GP_S_Gen5_2"...but you can set that Sku even without benefitting from the free offer.

var db = sql.AddDatabase("db", "dbName").WithSku(SkuName.FreeOffer);

But then when the SqlSku returned would be the "GP_S_Gen5_2", with no information if that is a free offer or not. So if we go that route then a IsFreeOfferEnabled property should be added IMHO.

[yorek]

For example:

var db = sql.AddDatabase("db", "dbName").WithSku(SkuName.FreeOffer);
Assert.Equal("GP_S_Gen5_2", db.Sku.Name);
Assert.Equal(true, db.IsFreeOfferEnabled);

[yorek]

@davidfowl @sebastienros Any feedback on this proposal?

[sebastienros]

@davidfowl @eerhardt @yorek

Here is a thought after looking at it again.

The free offer can't be defined by setting only a SqlSku property since it requires to also set these:

sqlDatabase.Sku = new SqlSku() { Name = "GP_S_Gen5_2" };
sqlDatabase.UseFreeLimit = true;
sqlDatabase.FreeLimitExhaustionBehavior = FreeLimitExhaustionBehavior.AutoPause;

This means we can't just expose a SqlSku property as @davidfowl is asking. We can however expose WithFreeOffer() but since we want this to be the default I don't see why we should have this method.

My suggestion is to just configure databases to be free by default by setting the correct bicep properties, and if someone wants a custom SqlSku they should use ConfigureInfrastructure to change the values, like it's done for any other Azure resource.

Optionally, if we decide to change the default to non-Bicep defaults, I would also suggest creating a WithDefaultSku() instead that would reset the "free" to whatever is the default provisioning is. This will provide an easy way for users to get the same behavior as 9.2 and earlier if they had to.

[eerhardt]

This means we can't just expose a SqlSku property as @davidfowl is asking. We can however expose WithFreeOffer() but since we want this to be the default I don't see why we should have this method. My suggestion is to just configure databases to be free by default by setting the correct bicep properties, and if someone wants a custom SqlSku they should use ConfigureInfrastructure to change the values, like it's done for any other Azure resource.

I agree with this. We don't expose a "WithSku" helper conveinence method on any other Azure resource.

Optionally, if we decide to change the default to non-Bicep defaults, I would also suggest creating a WithDefaultSku() instead that would reset the "free" to whatever is the default provisioning is. This will provide an easy way for users to get the same behavior as 9.2 and earlier if they had to.

I can see the value in having a method like this. My only comment is that I wouldn't want the method to be named WithDefaultSku() because it is confusing. We are defaulting the Sku to "free", so that is what people would consider the default.

Maybe if we called it WithDefaultAzureSku() and when that is called, we left the SKU unset in bicep - then you get the default from Azure.

[yorek]

So, to summarize:

• No WithSku() method
• When creating the Azure SQL resource, the Aspire Azure SQL resource will deploy the free Azure SQL DB offer by default
• A new method WithDefaultAzureSku() allows developer to resort to the default Azure SKU (basically, it omits completely the SKU specification in the generated bicep file)

Is that correct, and all agree that this is the way to go?

[eerhardt]

Is that correct, and all agree that this is the way to go?

That seems to be the best approach to me.

[yorek]

I made the changes, but while doing it I realized that having a WithSku is really just better. With that you can define the sku for each database, so you can have a server with 3 databases, each with a different sku. When using ConfigureInfrastructure instead, you do it at the server level (AFAIK), which means all databases in that server will have the defined SKU.

[sebastienros]

When using ConfigureInfrastructure instead, you do it at the server level (AFAIK), which means all databases in that server will have the defined SKU.

You can pick which database you want to alter

var sql2 = builder.AddAzureSqlServer("sql2")
        .ConfigureInfrastructure(c =>
        {
            const string FREE_DB_SKU = "GP_S_Gen5_2";

            foreach (var database in c.GetProvisionableResources().OfType<SqlDatabase>())
            {
                database.Sku = new SqlSku() { Name = FREE_DB_SKU };
            }
        });

[davidfowl]

Thank you @yorek !