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

allow to set boot diagnostics storage account to managed #1206

Merged

Conversation

Klaas-
Copy link
Contributor

@Klaas- Klaas- commented Jun 30, 2023

SUMMARY

Allow to use "managed" for boot diagnostic storage accounts, this means no storage account gets created.
https://learn.microsoft.com/en-us/azure/virtual-machines/boot-diagnostics#enable-managed-boot-diagnostics

Fixes #993

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

azure_rm_virtualmachine.py

Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com>
@Fred-sun
Copy link
Collaborator

Fred-sun commented Jul 6, 2023

@Klaas- Wouldn't it be nice to add a parameter to boot_diagnistics, such as' type',

type: managed --- Enabled with managed storag account
type: custome --- Enable with custome storage account

@Fred-sun Fred-sun added question Further information is requested medium_priority Medium priority work in In trying to solve, or in working with contributors labels Jul 6, 2023
@Klaas-
Copy link
Contributor Author

Klaas- commented Jul 6, 2023

I changed the behavior -- there is now an optional argument type: managed - if you set that the storage account will be managed. Anything else will result in the same behavior as before. I would suggest to change the default to managed on the next major release

@Fred-sun
Copy link
Collaborator

Fred-sun commented Jul 7, 2023

@Klaas- Please change the following lines accordingly.

if self.boot_diagnostics['enabled']:
Update: if self.boot_diagnostics['enabled'] and self.boot_diagnostics.get('type') != 'managed':

if self.boot_diagnostics_present and self.boot_diagnostics['enabled']:

Update: if self.boot_diagnostics_present and self.boot_diagnostics['enabled'] and self.boot_diagnostics .get('type') != 'managed':

@Fred-sun Fred-sun added ready_for_review The PR has been modified and can be reviewed and merged and removed work in In trying to solve, or in working with contributors labels Jul 7, 2023
@xuzhang3
Copy link
Collaborator

@Klaas- LGTM 🚢

@xuzhang3 xuzhang3 merged commit b1f8fd3 into ansible-collections:dev Jul 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
medium_priority Medium priority question Further information is requested ready_for_review The PR has been modified and can be reviewed and merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

azure_rm_virtualmachine boot diagnostics don't allow for managed storage accounts
3 participants