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

Swap individual policy assignments to alzDefaultPolicyAssignments module in E2E tests #183

Merged
merged 22 commits into from
Mar 24, 2022

Conversation

jfaurskov
Copy link
Contributor

@jfaurskov jfaurskov commented Mar 11, 2022

Overview/Summary

Update unit test pipeline to include alzDefaultPolicyAssignments

This PR fixes/adds/changes/removes

  1. Change check for policy assignments file to alzDefaultPolicyAssignments.bicep
  2. Move policy assignment step to after subscription placement to allow more time for policy replication.
  3. Add script to do retry logic if policy assignment fails initially. Script will retry deployment 5 times with 30 second intervals and then error out if not succeeded.

Breaking Changes

N/A

Testing Evidence

Ran script manually, with custom policies not deployed to root of ALZ management group structure.
defaultpolassign1
After failing script waits 30 seconds and then retries.
defaultpolassign2
Separately entered the policies while script was running and it succeded.
defaultpolassign3

As part of this Pull Request I have

  • Checked for duplicate Pull Requests
  • Associated it with relevant ADO items
  • Ensured my code/branch is up-to-date with the latest changes in the main branch
  • Performed testing and provided evidence.
  • Updated relevant and associated documentation.

@ghost ghost added the Needs: Triage 🔍 Needs triaging by the team label Mar 11, 2022
Copy link
Collaborator

@jtracey93 jtracey93 left a comment

Choose a reason for hiding this comment

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

A few bits, also really want to ensure we have enough delay or retry logic if the policy assignments fail due to Azure/Enterprise-Scale#902.

Can we add some error handling if the deployment fails due to this, to wait for say 5 mins and try again, as this should give replication a chance to catch up in ARM

tests/pipelines/bicep-build-to-validate.yml Outdated Show resolved Hide resolved
tests/pipelines/bicep-build-to-validate.yml Outdated Show resolved Hide resolved
tests/pipelines/bicep-build-to-validate.yml Outdated Show resolved Hide resolved
@jtracey93 jtracey93 changed the title Update file to new path Swap individual policy assignments to alzDefaultPolicyAssignments module in E2E tests Mar 11, 2022
@jtracey93 jtracey93 added enhancement and removed Needs: Triage 🔍 Needs triaging by the team labels Mar 11, 2022
Co-authored-by: Jack Tracey <41163455+jtracey93@users.noreply.github.com>
@ghost ghost removed the Needs: Author Feedback label Mar 11, 2022
Co-authored-by: Jack Tracey <41163455+jtracey93@users.noreply.github.com>
@jfaurskov
Copy link
Contributor Author

Looking into the error handling bits

@jfaurskov jfaurskov requested a review from jtracey93 March 22, 2022 10:02
Copy link
Collaborator

@jtracey93 jtracey93 left a comment

Choose a reason for hiding this comment

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

Just a few bits 👍

Great work though 👍

.github/scripts/Set-AlzDefaultPolicyAssignment.ps1 Outdated Show resolved Hide resolved
.github/scripts/Set-AlzDefaultPolicyAssignment.ps1 Outdated Show resolved Hide resolved
tests/pipelines/bicep-build-to-validate.yml Outdated Show resolved Hide resolved
tests/pipelines/bicep-build-to-validate.yml Outdated Show resolved Hide resolved
Co-authored-by: Jack Tracey <41163455+jtracey93@users.noreply.github.com>
@ghost ghost removed the Needs: Author Feedback label Mar 23, 2022
jfaurskov and others added 5 commits March 23, 2022 08:13
@jfaurskov
Copy link
Contributor Author

/azp run E2E

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jfaurskov
Copy link
Contributor Author

/azp run E2E

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jfaurskov
Copy link
Contributor Author

/azp run E2E

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jfaurskov
Copy link
Contributor Author

/azp run E2E

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jfaurskov jfaurskov requested a review from jtracey93 March 23, 2022 14:03
@jfaurskov
Copy link
Contributor Author

/azp run E2E

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jfaurskov jfaurskov merged commit afad38e into Azure:main Mar 24, 2022
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.

2 participants