Azure / mlops-v2

Azure MLOps (v2) solution accelerators. Enterprise ready templates to deploy your machine learning models on the Azure Platform.
https://learn.microsoft.com/en-us/azure/machine-learning/concept-model-management-and-deployment
MIT License
502 stars 248 forks source link

Quickstart.md - Job: Create Bicep Deployment issue with the storage account name #8

Closed carla-fiadeiro closed 1 year ago

carla-fiadeiro commented 2 years ago

Why?

While running pipeline with the Create Bicep Deployment Job we will get the error: {'code': 'StorageAccountAlreadyTaken', 'target': 'stmlopsv2819prod', 'message': 'The storage account named stmlopsv2819prod is already taken.'}

I believe storage account names are unique and since this was already executed I can't create a storage account with the same name.

How?

I believe in the both files config-infra-dev.yml and config-infra-prod.yml this line needs to change: st$(namespace)$(postfix)$(environment)

and we might have to add some initials of our company, subscription or even from our name

Thank you, Carla

azeltov commented 2 years ago

I see the same issue @carla-fiadeiro and i had to change my postfix to a unique value to deploy the bicep. Documentation needs to change in readme file:

   Under global, change postfix: 818 to postfix: 819 (line 8) and save.
carla-fiadeiro commented 2 years ago

Hi @azeltov,

First of all thank you for the reply.

Yes and I think this issue will also occur with the Key Vault and Container Registry has they also are expecting an unique name. We can change the postfix value to be an unique one but it's always a guess which value to use. I followed a different approach but once again it was a lucky shot, I changed the following files:

And it worked. And yes, no matter the approach we follow the documentation needs to change.

Once again thank you for the reply.

stevedem commented 2 years ago

Also ran into this same issue. I'm no bicep/ARM expert, but we should be able to generate a unique string (based on a hash of the resource group ID or subscription ID) so that this issue can be resolved.

chrey-gh commented 2 years ago

As this doc states (https://docs.microsoft.com/en-us/azure/storage/common/storage-account-overview#storage-account-endpoints) storage accounts need to be unique Azure wide and can only contain lowercase letters and numbers (3-24). That said the namespace and postfix in config-infra-dev.yml and config-infra-prod.yml should take care of uniquefying the storage account name and all other resources. The storage account name is not generated by bicep, it is set ultimately by the storage_account setting in the yml files mentioned above. So if you wanted to create some hash, you can do so and provide this value for the namespace and/or postfix in these yaml files. And it wont' affect keyvault or container registry in the same way, because they have not as limiting as constraints for uniqueness.

The way to go is as azeltov said to change the namespace and/or postfix to your liking.

Yes and we're going to adjust the documentation (Readme) in that sense. You don't change it to 819 but to a number of your liking. Also as I said, to achieve more uniqueness you could also change the namespace. Be aware though, that you stay within the limit of 24 for names of storage accounts.

ShaneOsborne commented 2 years ago

For me I have changed line 7 in in stoacct.bicep to -

name: 'st${uniqueString(resourceGroup().id)}'

I am not using base name as this is in the resource group, dont need to add -env to it and works more in line what we do in other repositories

You could add the unique id to the previous basename string as well, as long as for storage accounts this is lower than previous basename is 11 chars or less then the total will be 24 (uniqeString returns 13) which is within the limits.

Another option would be to change postfix from being a param, change the var name to something like 'uniqueid' and set this to the 'uniqueString(resourceGroup().id)'. This would then get used across many resources, would need testing but this is normally what I do to ensure clean rollouts. You still add a prefix and env as before and solves most issues

Happy to do either change if allowed :)

chrey-gh commented 2 years ago

Shane, thanks for your input. That change was already done yesterday. But only the storage account name got changed, since there's also a Terraform bit involved. Also the name(s) of these get set in the configuration yaml file, so I wouldn't want to break that. connection.

xiaoharper commented 2 years ago

I hit the same issue first with blob name, then with kv name. below is the error message with kv name. I edit the namespace and postfix in config-infra-prod.yml to make it work. It would be good to add more explanation and example in quickstart.md

"The vault name 'kv-mlopsv2zhanxia-2022prod' is invalid. A vault's name must be between 3-24 alphanumeric characters. The name must begin with a letter, end with a letter or digit, and not contain consecutive hyphens

chrey-gh commented 2 years ago

Xiaoharper, thanks for your input. The problem with your kv name is, that it is to long. It can only be 3-24 alphanumeric characters long. Yours is 26.