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

Added guide to only deploy service health #55

Merged
merged 6 commits into from
Oct 27, 2023
Merged

Added guide to only deploy service health #55

merged 6 commits into from
Oct 27, 2023

Conversation

arjenhuitema
Copy link
Contributor

@arjenhuitema arjenhuitema commented Oct 25, 2023

Overview/Summary

Added guidance for deploying only the service health alerts.

This PR fixes/adds/changes/removes

  1. Adds documentation: Deploy-only-Service-Health-Alerts.md. For using ALZ Pattern to deploy only the service health alerts

Breaking Changes

N/A

As part of this Pull Request I have

  • Read the Contribution Guide and ensured this PR is compliant with the guide
  • Checked for duplicate Pull Requests
  • Associated it with relevant GitHub Issues or ADO Work Items (Internal Only)
  • Ensured my code/branch is up-to-date with the latest changes in the main branch
  • Ensured PR tests are passing
  • Updated relevant and associated documentation (e.g. Contribution Guide, Docs etc.)

@arjenhuitema arjenhuitema added the documentation Improvements or additions to documentation label Oct 25, 2023
Copy link
Contributor

@paulgrimley paulgrimley left a comment

Choose a reason for hiding this comment

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

Some changes - we need to

  1. Determine upper or lowercase for initiatives
  2. Use of Policy Set Definition or Initiative

@Brunoga-MS
Copy link
Collaborator

@arjenhuitema: documentation looks great 👍 Just few comments:

  1. I would be consistent on the language type we use. For instance, since Azure CLI is used and the deployment command is PowerShell, I would set the variable using PowerShell and not Bash. Showing them 2 could create confusion because of the $ sign (despite the note you correctly put) How common is using Bash? If so, we need to include both language samples for both variable definition and commands.
  2. in regards to editing the copy of policies.json file, could we clearly recommend to comment or delete unnecessary lines? "Edit the variables" could lead to change the value of them and in that case which value should I put?
  3. ine 217 typo: Assign a**n** Policy Set Definition
  4. 6. Assign the Service Health Policy Policy Set Definition: can we just use the template file? the parameters we need to pass are the same as the parameter file, and since it is going to be more consistent, why not using it directly? Moreover changing the parameter is also documented, so customers will also have reference

@arjenhuitema
Copy link
Contributor Author

Thanks @Brunoga-MS !

Made changes to address points 2 and 3.

I will keep point 1 as is. It´s consistent with the other deployment guides and we don´t want to assume what terminal is being used to run Azure CLI. I run Azure CLI from a PowerShell terminal, however I know others that run Azure CLI from Bash (Cloud Shell)

On point 4. We can´t, the existing parameter file contains all of the parameters used for a full deployment. Using the parameter file for deploying the Policy Set Assignment wil result in a template violation, as there will be parameters that are being supplied but don´t exist in the template that is deployed. Of course a customer could create their own parameter file from the JSON string in the command provided. For now, I will leave this as is, if in the future there are requests to provide an Arm template, or parameter file for this specific scenario we can evaluate.

Copy link
Contributor

@paulgrimley paulgrimley left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@Springstone Springstone left a comment

Choose a reason for hiding this comment

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

LGTM

@JoeyBarnes JoeyBarnes merged commit 0a34d7a into Azure:main Oct 27, 2023
@arjenhuitema arjenhuitema deleted the docs-individual-sh branch November 2, 2023 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants