feat: Add the creation of 'Microsoft.CognitiveServices/accounts/deployments' in Cognitive Service Module - avm/res/cognitive-services/account

[zedy-wj]

Description

Fixes #1131

Pipeline Reference

Pipeline

Type of Change

• Update to CI Environment or utlities (Non-module effecting changes)
• [x ] Azure Verified Module updates:
  • Bugfix containing backwards compatible bug fixes, and I have NOT bumped the MAJOR or MINOR version in version.json:
    • Someone has opened a bug report issue, and I have included "Closes #{bug_report_issue_number}" in the PR description.
    • The bug was found by the module author, and no one has opened an issue to report it yet.
  • Feature update backwards compatible feature updates, and I have bumped the MINOR version in version.json.
  • Breaking changes and I have bumped the MAJOR version in version.json.
  • Update to documentation

Checklist

• [x ] I'm sure there are no other open Pull Requests for the same update/change
• I have run Set-AVMModule locally to generate the supporting module files.
• My corresponding pipelines / checks run clean and green without any errors or warnings

@jongio - for notification.

[AlexanderSehr]

Hey @zedy-wj,

thank you for your PR. 2 asks if I may:

• Could you add the edployment of the new deployments parameter to e.g. the max test?
• Could you add a pipeline badge showing that the deployment works and also all static tests pass?

cc: @ilhaan

[zedy-wj]

@AlexanderSehr - I have updated max test. But I encountered an error during Static validation:

at $filesAreTheSame | Should -Be $true -Because ('The file hashes before and after applying the `Set-ModuleReadMe` function should be identical and should not have diff </br><pre>{0}</pre>. Please re-run the script for this module''s template.' -f $mdFormattedDiff), /home/runner/work/bicep-registry-modules/bicep-registry-modules/avm/utilities/pipelines/staticValidation/compliance/module.tests.ps1:327

I tried running the Set-ModuleReadMe function from local but that didn't seem to work.

Did I do something wrong? Could you take a look at it, thanks!

[AlexanderSehr]

@AlexanderSehr - I have updated max test. But I encountered an error during Static validation:

at $filesAreTheSame | Should -Be $true -Because ('The file hashes before and after applying the `Set-ModuleReadMe` function should be identical and should not have diff </br><pre>{0}</pre>. Please re-run the script for this module''s template.' -f $mdFormattedDiff), /home/runner/work/bicep-registry-modules/bicep-registry-modules/avm/utilities/pipelines/staticValidation/compliance/module.tests.ps1:327

I tried running the Set-ModuleReadMe function from local but that didn't seem to work.

Did I do something wrong? Could you take a look at it, thanks!

Well would you look at that - I should update the test's description (ref #1489).

You did one thing wrong in that dot-sourcing a function requires an invocation with leading separate dot: . <path>

That being said, I'd recommend to invoke the Set-AVMModule function you can find in the avm/utilities/tools folder. You can invoke it for the module's folder path and should be good to go

[AlexanderSehr]

Thanks for the notification @zedy-wj , just updated folloing @ilhaan's approval

[AlexanderSehr]

Hey @zedy-wj, we may have a bit of a problem / something I'd need your advice about

The PR added a new test for deployments, which is great. The caveat - it doesn't work in our subscription due to the error "The current subscription does not have feature required by this model 'Format: OpenAI, Name: gpt-35-turbo, Version: 0301'.".
Now, I tried to look up if it's just a feature that needs registering, but it seems we need to fill out a forms to actually sign the subscription up? Can you confirm? If so, this would impact not only us, but also the subscriptions of the owners and we should try to find an alternative test.

For the moment this means the deployment did fail and the new module version is not published.

PS: I also checked region availability (which could be a concern), but the deployment did try 'WestEurope' and the availability looks good for gpt-35-turbo - 0301.

cc: @ilhaan

[zedy-wj]

@AlexanderSehr - Okay, I suspect that the subscription used in the test does not support this feature. Could you help me verify the above guess in the test subscription?

Steps:

1. Open azure portal.
2. Create an Azure OpenAI ( choose your resource group and region, fill in name and all other by default).
   
3. Navigate to the OpenAI you just created and click Model Deployment, then continue to click Manage Deployments
   
4. Click New deployments, please confirm what deployment options are available to you.

When choosing the OpenAI model, I specifically chose gpt-35-turbo - 0301 due to the limitation of the West Europe region.

[AlexanderSehr]

Hey @zedy-wj,
thanks for elaborating :)

@ilhaan, is that ok with you? Personally, I don't know the CongitiveService resource well enough to judge if there is an easier alternative. If there is no easier solution, I'd strongly suggest to add the above instructions to the deployments test as a comment to help others that face the same issue.

Just putting this out there to ensure the service only so hard to maintain as absolutely necessary 😉

cc: @eriqua, @ChrisSidebotham, @jtracey93 for awareness. Exciting times 😄

[ilhaan]

@AlexanderSehr Can we just pick a model we know is available in all regions without extra registration steps?

[AlexanderSehr]

@AlexanderSehr Can we just pick a model we know is available in all regions without extra registration steps?

Right, that's the ask. Does anybody know of one? 😄

[ilhaan]

@AlexanderSehr Sorry misunderstood your question earlier.

@jceval any ideas?

[jongio]

It's likely due to the sub not being on the Azure OpenAI allow list. You can find out more here: https://learn.microsoft.com/en-us/legal/cognitive-services/openai/limited-access

You can request here: https://aka.ms/oai/access

[AlexanderSehr]

It's likely due to the sub not being on the Azure OpenAI allow list. You can find out more here: https://learn.microsoft.com/en-us/legal/cognitive-services/openai/limited-access

You can request here: https://aka.ms/oai/access

Hey @jongio,
absolutely. The ask comes down to finding a test scenario that does not need a special approval as this will make contributions from anybody who does not have this set up (e.g. external contribtors) very hard. Right, imagine somthing else is updated, somebody runs the CI in their fork and it keeps on failing on them.
So if we were to find an anternative deployment for the test that would be great. Just to keep the barrier low.

[jongio]

Unfortunately there's no way around it. How about we disable the test by default?

[AlexanderSehr]

Ok, so the resource itself is just for OpenAI. In that case your suggestion may be the way to go. @ilhaan how do you feel about that? Truth be told, I don't exactly like the idea of having code in main that is not tested.
But if the only other alternative is to add the AVM subscription alongside every contributor's / owner's subscription via a process to some list I can't say it's much more appealing either.

cc: @eriqua, @ChrisSidebotham, @jtracey93 (please share if you have some thoughts about this too)

Maybe we're in luck and sometime soon this onboarding isn't necessary anymore? 😄

[ilhaan]

@AlexanderSehr I am not a fan of either option, but if I had to pick one, I would go with @jongio's suggestion of disabling the test by default.

[AlexanderSehr]

Hey @ilhaan,
alright let's do that then. Mind you, there is no 'native' way to do this, so we have get creative and decide for the best option. The following things could do what we want to achieve

• We could remove the test folder (the test is gone and removed from the readme too)
• We could rename the deployment test's main.test.bicep to something like disabled_main.test.bicep or similar as the CI logic searches for main.test.bicep files specifically (would mean the test is not found and not added to the readme)
• We could change the logic inside the test file to only run the deployment test on an impossible condition (would run the test file but not the test and still render it in the readme. The downside: it's hard to see it's disabled / is not validated, even if we add a dislaimer comment.
• ...

cc: @eriqua, @ChrisSidebotham

[eriqua]

Hey @ilhaan, alright let's do that then. Mind you, there is no 'native' way to do this, so we have get creative and decide for the best option. The following things could do what we want to achieve

• We could remove the test folder (the test is gone and removed from the readme too)
• We could rename the deployment test's main.test.bicep to something like disabled_main.test.bicep or similar as the CI logic searches for main.test.bicep files specifically (would mean the test is not found and not added to the readme)
• We could change the logic inside the test file to only run the deployment test on an impossible condition (would run the test file but not the test and still render it in the readme. The downside: it's hard to see it's disabled / is not validated, even if we add a dislaimer comment.
• ...

cc: @eriqua, @ChrisSidebotham

Sorry, for me having a test file which is not continuously tested is really the last option. It's too likely to get stale and providing incorrect examples is way worse than not providing them at all.

That said, what about the following:

• Provide instructions on how to set up the environment in the documentation, as done for AADDS
• Short term: consider the test as an accepted known failure when coming from contributors. They can either set up the environment correctly following provided instructions or not
• Long term (effort to be estimated) update the CI to only run the test in upstream, or, for example, depending on a github variable switch. So that we provide contributors with an option to enable it on their side.

[jongio]

Can we have a flag like "Microsoft Test" or something like that. So it is run by our CI, but not by every contributor?

[zedy-wj]

@AlexanderSehr - Is there anything I can do currently to fix CI so that new version can be published?

[Agazoth]

Hi team. We already use these models in our deployments and need to rely on our own modules until this pr gets merged.

We always engage customers with a pre-req for a subscription with OpenAI enabled (https://aka.ms/oai/access) - in my opinion that is similar to having to register a resource provider.

This module is the last one we need to be able to provision our entire solution on AVM. I really wish you decide to enable OpenAI on the subscription you use for CI, so we can get the final bit of our template done. After all, it can't be too long before OpenAI becomes just another resource that can be activated on a subscription.

Under all circumstances, the documentation should align with what is actually available form the current version of the module. The official documentation currently states, that we can do this:

But when we try to do that in our deployment, we get the old module from the registry, that does not contain the deployments part of the template module:

Our IaC superman had to track the error all the way to this repo before he could figure out, why the module didn't work.

In the readme help for our private registry, we add extra information like ekstra requirements in the readme. Something similar could be done on this component. A simple Info box would do.

[AlexanderSehr]

r deployments and need to rely on our own modules until this pr gets merged.

We always engage customers with a pre-req for a subscription with O

Hey @zedy-wj,
sorry that I did not come back on this yet. I was out for a week and recoverying too.

@jongio I'll bring this up with the team. But I think that would be too far in the other direction as folks should / would need to be able to test it outside AVM. If we get a change in (unrelated to this feature) that would somehow break the feature and set PR would be merged and only validated in AVM, then the pipeline would fail, the code would be in main, and the module not published.

@jtracey93 did you have similar cases for any of the features you used to worked on. I'm currently leaning most towards disabling the test (e.g. my manipulating the test file) and rely on manual tests. But this can (and probably) will eventually likely bring us a world of pain - unless - by then deployments can be generally tested.

[jtracey93]

I think we just need to get this module published as AI is a hot topic right now. So disabling the test to get the new version out would be my suggestion for short term.

Longer term what @eriqua suggested seems pragmatic and a already used approach.

I think right now this option suggest by you @AlexanderSehr:

We could rename the deployment test's main.test.bicep to something like disabled_main.test.bicep or similar as the CI logic searches for main.test.bicep files specifically (would mean the test is not found and not added to the readme)

Whilst changing the readme generation logic to also include disabled_main.test.bicep files is the best compromise.

**I'm also agreeing with @jongio on changing the prefix from disabled to something else like msft_ci_only_...... **

[AlexanderSehr]

Hey @jtracey93,
thanks for replying. Then let's disable the test case. I wouldn't tweak the readme for this script though as we may easly introduce issues for all modules as opposed to having it to solve for one. If we want to have the best of both worlds, we'd need to not disable the test, but update it to something like:

// Before
metadata name = 'WAF-aligned'
metadata description = '''This instance deploys the module in alignment with the best-practices of the Azure Well-Architected Framework.'''

module testDeployment '../../../main.bicep' = [
  for iteration in ['init', 'idem']: {
    (...)
  }
]

//After
metadata name = 'WAF-aligned'
metadata description = '''
This instance deploys the module in alignment with the best-practices of the Azure Well-Architected Framework.
Note: This test case is currently only validated manually.
'''

module testDeployment '../../../main.bicep' = [
  for iteration in ['init', 'idem']: if (true == false) {
    (...)
  }
]

This would only update the readme because of the disclaimer and the (true == false) prevents the deployment from running.

However, like @eriqua pointed out this could mean we're providing a potentially stale example over time. So if we don't want to provide it at all, we'd need to disable the test via a rename of the test file and accept that the file is only tested manually.
Sure, we can provide instructions on how to setup the test environment, and that would make sense to help the manual testing, but is really only helping that. The CI won't be able to test it for the time being (just something to accept).

[AlexanderSehr]

Hi team. We already use these models in our deployments and need to rely on our own modules until this pr gets merged.

We always engage customers with a pre-req for a subscription with OpenAI enabled (https://aka.ms/oai/access) - in my opinion that is similar to having to register a resource provider.

This module is the last one we need to be able to provision our entire solution on AVM. I really wish you decide to enable OpenAI on the subscription you use for CI, so we can get the final bit of our template done. After all, it can't be too long before OpenAI becomes just another resource that can be activated on a subscription.

Under all circumstances, the documentation should align with what is actually available form the current version of the module. The official documentation currently states, that we can do this: (..)

But when we try to do that in our deployment, we get the old module from the registry, that does not contain the deployments part of the template module: (..)

Our IaC superman had to track the error all the way to this repo before he could figure out, why the module didn't work.

In the readme help for our private registry, we add extra information like ekstra requirements in the readme. Something similar could be done on this component. A simple Info box would do.

100% agree. It's the very unfortunate sideeffect of the PR being merged without the pipeline going through. Something we'll only be able to prevent in the future by enabling contributors to test in our environment (which is on the roadmap, but is no low-hangning fruit)

[AlexanderSehr]

@zedy-wj will be working on a PR now

Edit, ref

I had to resort to the option that essentially disables the test yet keeps the readme afterall. Reason being that we have a static tests that enforces a main.test.bicep in every test folder - which makes sense - yet means I cannot simply rename the file and call it a day. And honestly it seemed the better solution to then temporarily 'solve' in for this module, as opposed to changing a test that affects all modules.

What I did not instead was to

• disable the deployed as described above with an impossible condition
• add a big disclaimer to the test's description
• add a post-deployment tests which's only purpose it is to also add a warning to the workflow output
  

It's not perfect, but I think it's better than pretending nothing is going on.

[AlexanderSehr]

Hey @zedy-wj, @jongio & @Agazoth,
please note that the publishing went through and you should be able to use the feature now as intended: