Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Alec Baird bicep #690

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Alec Baird bicep #690

wants to merge 3 commits into from

Conversation

Baird-Alec
Copy link

Bicep implementation for all ARM templates hosted in the GitHub. One major change that is required is THE API VERSION MUST BE HARDCODED, so when we want to change apiVersions, we must manually update to a newer version.

@ghost
Copy link

ghost commented May 17, 2022

CLA assistant check
All CLA requirements met.

@shanberg1
Copy link
Contributor

Bicep implementation for all ARM templates hosted in the GitHub. One major change that is required is THE API VERSION MUST BE HARDCODED, so when we want to change apiVersions, we must manually update to a newer version.

why is this?

@@ -0,0 +1,9 @@
function GetAvdSessionHostName {
Copy link
Contributor

Choose a reason for hiding this comment

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

are there any changes in the dsc folder or is it just copy pasted?

Copy link
Author

Choose a reason for hiding this comment

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

I just copy pasted the DSC folder as I didn't see anything needing to be changed

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should refactor the dsc folder out and use the same one for both arm and bicep templates

@Baird-Alec
Copy link
Author

The reason they have to be hardcoded is because BICEP doesn't allow string interpolation in its resource variables, so it is expecting an specific data format like foo@bar, where bar is ApiVersion. Because it prevents interpolation, the quickest solution (and possibly only) solution is to hardcode the ApiVersions. See here: https://docs.microsoft.com/en-us/azure/azure-resource-manager/bicep/resource-declaration?tabs=azure-powershell

(Note: There may be a way around this, but I didn't know if it was worth pursuing or not.

@shanberg1
Copy link
Contributor

It is unnecessary for github i think

@dhrump
Copy link
Contributor

dhrump commented Jun 2, 2022

Wondering why there are separate files for Availability set, workspace, networksecurity group which is different from arm template

1 similar comment
@dhrump
Copy link
Contributor

dhrump commented Jun 2, 2022

Wondering why there are separate files for Availability set, workspace, networksecurity group which is different from arm template

param availabilitySetTags object
param availabilitySetUpdateDomainCount int
param availabilitySetFaultDomainCount int
param avSetSKU string
Copy link
Contributor

Choose a reason for hiding this comment

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

Why there are two AVSet linked template

Fixed all errors that arose during manual deployment testing.
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.

4 participants