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 virtual network auto-creation if no virtual network found for azure_virtual_machine_with_public_ip role #77

Conversation

anna-savina
Copy link
Collaborator

For azure_virtual_machine_with_public_ip role:

  • Added virtual network auto-creation if no virtual network found
  • Added simple test with default create/delete options to verify changes

@anna-savina anna-savina force-pushed the fix_azure_virtual_machine_with_public_ip_role branch from 88d4cbd to da0539a Compare June 3, 2024 06:07
Copy link
Contributor

@nirarg nirarg left a comment

Choose a reason for hiding this comment

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

Please separate to 2 commits: 1 for the integration test and one for the role changes
If there are jira tickets for these commits, please mention the ids in the commit message

I suggest to have the commit messages with the following structure:

ticket_id: What changed

Why changed

Notice there is one empty line between the what and why
(The why changed part is not mandatory)

Is this breaking change? If users already use this role, would they need to make changes after update?

@anna-savina anna-savina force-pushed the fix_azure_virtual_machine_with_public_ip_role branch 3 times, most recently from 4eaa1d7 to 9ef316d Compare June 3, 2024 14:07
@anna-savina
Copy link
Collaborator Author

Please separate to 2 commits: 1 for the integration test and one for the role changes If there are jira tickets for these commits, please mention the ids in the commit message

I suggest to have the commit messages with the following structure:

ticket_id: What changed

Why changed

done. Now PR contains two commits with Jira tickets ID

Is this breaking change? If users already use this role, would they need to make changes after update?

No, If a virtual network exists, as it was required before, all changes will be skipped (I tested this scenario as well).

@anna-savina anna-savina force-pushed the fix_azure_virtual_machine_with_public_ip_role branch from 9ef316d to 9c50418 Compare June 4, 2024 14:31
Copy link
Contributor

@nirarg nirarg left a comment

Choose a reason for hiding this comment

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

Looks good, thanks

Copy link
Contributor

@p3ck p3ck left a comment

Choose a reason for hiding this comment

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

Looks good. Just some minor comments.

@anna-savina anna-savina force-pushed the fix_azure_virtual_machine_with_public_ip_role branch from 9c50418 to 7a28b38 Compare June 5, 2024 14:29
@p3ck p3ck merged commit 6b0a56f into redhat-cop:main Jun 5, 2024
18 checks passed
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