Skip to content
This repository was archived by the owner on Nov 11, 2024. It is now read-only.

Conversation

@zikalino
Copy link

Purpose

  • ...

Does this introduce a breaking change?

  • Yes
  • No

Pull Request Type

What kind of change does this Pull Request introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Documentation content changes
  • Other... Please describe:

How to Test

  • Install ansible
$ pip install ansible[azure]
  • [Optional] Install ansible role
$ ansible-galaxy install Azure.azure_preview_modules
$ pip install -r ~/.ansible/roles/Azure.azure_preview_modules/files/requirements-azure.txt
  • Get the sample playbook
git clone https://github.com/Azure-Samples/ansible-playbooks.git
cd ansible-playbooks
git checkout [branch-name]
  • Test the code
ansible-playbook <filename>.yml

What to Check

Verify that the playbook is successfully run.

Other Information

@zikalino zikalino changed the title first updating application gateway Sep 20, 2018
.travis.yml Outdated
diffstr=$(git diff $branch remotes/origin/master --name-only -- '*.yml' --no-pager);
changedfiles=($diffstr);
excludedList="vm_create_existingvnet_deployjavaapp.yml .travis.yml aks_create_scale.yml webapp.yml rest/sql-managed-instance.yml vm_create_image.yml";
excludedList="vm_create_existingvnet_deployjavaapp.yml .travis.yml aks_create_scale.yml webapp.yml rest/sql-managed-instance.yml vm_create_image.yml appgateway_create.yml";
Copy link
Contributor

Choose a reason for hiding this comment

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

why appgate is excluded?

Copy link
Author

Choose a reason for hiding this comment

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

because preview moduled don't have right version yet....

Copy link
Contributor

Choose a reason for hiding this comment

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

preview_modules should keep sync with upstream, pls fix preview_module firstly

@@ -0,0 +1 @@
MIIMAjCCCeqgAwIBAgITLQAAMpnXBx230XCKQgAAAAAymTANBgkqhkiG9w0BAQsFADCBizELMAkGA1UEBhMCVVMxEzARBgNVBAgTCldhc2hpbmd0b24xEDAOBgNVBAcTB1JlZG1vbmQxHjAcBgNVBAoTFU1pY3Jvc29mdCBDb3Jwb3JhdGlvbjEVMBMGA1UECxMMTWljcm9zb2Z0IElUMR4wHAYDVQQDExVNaWNyb3NvZnQgSVQgVExTIENBIDUwHhcNMTcwNzIwMTc0NzA4WhcNMTkwNzEwMTc0NzA4WjAXMRUwEwYDVQQDEwx3d3cuYmluZy5jb20wggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQC6jsg+/7DlIrdgFOcaDlK3RQ9sIgkJsgpj+ZxAbIe3ziyimIxjVlHX87pqgXcNhaYNbCFD0iPm+aUfbv4GDTLR+AIr8eSegqxZ+CBToYM67NhpVYra1KAvY4XgqxorO4FB9IWYJRqhI3SZeZ3lLK5t9XuUMicG8l52nJfpPdXXvBca2wUCq8FHEObG81vJzESA0htLLPTjdUWBQnXPiW5bqzlGHzzv8ISV6jtDLNNa5JRlhSlXho+6pCedhNF7MP4yTaantPvAELLRWX13VhjgoCcRCCu0s8rxW5DuVWl2Pb2iw35MFnNWlcoVwq0AjAfGA+xEba/WLid6qfkQctYjAgMBAAGjggfQMIIHzDAdBgNVHQ4EFgQUCYflhSl4MCAls91+3GztpSmoA3AwCwYDVR0PBAQDAgSwMB8GA1UdIwQYMBaAFAj+JZ906ocEwry7jqg4XzPG0WxlMIGsBgNVHR8EgaQwgaEwgZ6ggZuggZiGS2h0dHA6Ly9tc2NybC5taWNyb3NvZnQuY29tL3BraS9tc2NvcnAvY3JsL01pY3Jvc29mdCUyMElUJTIwVExTJTIwQ0ElMjA1LmNybIZJaHR0cDovL2NybC5taWNyb3NvZnQuY29tL3BraS9tc2NvcnAvY3JsL01pY3Jvc29mdCUyMElUJTIwVExTJTIwQ0ElMjA1LmNybDCBhQYIKwYBBQUHAQEEeTB3MFEGCCsGAQUFBzAChkVodHRwOi8vd3d3Lm1pY3Jvc29mdC5jb20vcGtpL21zY29ycC9NaWNyb3NvZnQlMjBJVCUyMFRMUyUyMENBJTIwNS5jcnQwIgYIKwYBBQUHMAGGFmh0dHA6Ly9vY3NwLm1zb2NzcC5jb20wPgYJKwYBBAGCNxUHBDEwLwYnKwYBBAGCNxUIh9qGdYPu2QGCyYUbgbWeYYX062CBXYTS30KC55N6AgFkAgEQMB0GA1UdJQQWMBQGCCsGAQUFBwMCBggrBgEFBQcDATBNBgNVHSAERjBEMEIGCSsGAQQBgjcqATA1MDMGCCsGAQUFBwIBFidodHRwOi8vd3d3Lm1pY3Jvc29mdC5jb20vcGtpL21zY29ycC9jcHMwJwYJKwYBBAGCNxUKBBowGDAKBggrBgEFBQcDAjAKBggrBgEFBQcDATCCBW0GA1UdEQSCBWQwggVgggx3d3cuYmluZy5jb22CEGRpY3QuYmluZy5jb20uY26CEyoucGxhdGZvcm0uYmluZy5jb22CCiouYmluZy5jb22CCGJpbmcuY29tghZpZW9ubGluZS5taWNyb3NvZnQuY29tghMqLndpbmRvd3NzZWFyY2guY29tghljbi5pZW9ubGluZS5taWNyb3NvZnQuY29tghEqLm9yaWdpbi5iaW5nLmNvbYINKi5tbS5iaW5nLm5ldIIOKi5hcGkuYmluZy5jb22CGGVjbi5kZXYudmlydHVhbGVhcnRoLm5ldIINKi5jbi5iaW5nLm5ldIINKi5jbi5iaW5nLmNvbYIQc3NsLWFwaS5iaW5nLmNvbYIQc3NsLWFwaS5iaW5nLm5ldIIOKi5hcGkuYmluZy5uZXSCDiouYmluZ2FwaXMuY29tgg9iaW5nc2FuZGJveC5jb22CFmZlZWRiYWNrLm1pY3Jvc29mdC5jb22CG2luc2VydG1lZGlhLmJpbmcub2ZmaWNlLm5ldIIOci5iYXQuYmluZy5jb22CECouci5iYXQuYmluZy5jb22CEiouZGljdC5iaW5nLmNvbS5jboIPKi5kaWN0LmJpbmcuY29tgg4qLnNzbC5iaW5nLmNvbYIQKi5hcHBleC5iaW5nLmNvbYIWKi5wbGF0Zm9ybS5jbi5iaW5nLmNvbYINd3AubS5iaW5nLmNvbYIMKi5tLmJpbmcuY29tgg9nbG9iYWwuYmluZy5jb22CEXdpbmRvd3NzZWFyY2guY29tgg5zZWFyY2gubXNuLmNvbYIRKi5iaW5nc2FuZGJveC5jb22CGSouYXBpLnRpbGVzLmRpdHUubGl2ZS5jb22CDyouZGl0dS5saXZlLmNvbYIYKi50MC50aWxlcy5kaXR1LmxpdmUuY29tghgqLnQxLnRpbGVzLmRpdHUubGl2ZS5jb22CGCoudDIudGlsZXMuZGl0dS5saXZlLmNvbYIYKi50My50aWxlcy5kaXR1LmxpdmUuY29tghUqLnRpbGVzLmRpdHUubGl2ZS5jb22CCzNkLmxpdmUuY29tghNhcGkuc2VhcmNoLmxpdmUuY29tghRiZXRhLnNlYXJjaC5saXZlLmNvbYIVY253ZWIuc2VhcmNoLmxpdmUuY29tggxkZXYubGl2ZS5jb22CDWRpdHUubGl2ZS5jb22CEWZhcmVjYXN0LmxpdmUuY29tgg5pbWFnZS5saXZlLmNvbYIPaW1hZ2VzLmxpdmUuY29tghFsb2NhbC5saXZlLmNvbS5hdYIUbG9jYWxzZWFyY2gubGl2ZS5jb22CFGxzNGQuc2VhcmNoLmxpdmUuY29tgg1tYWlsLmxpdmUuY29tghFtYXBpbmRpYS5saXZlLmNvbYIObG9jYWwubGl2ZS5jb22CDW1hcHMubGl2ZS5jb22CEG1hcHMubGl2ZS5jb20uYXWCD21pbmRpYS5saXZlLmNvbYINbmV3cy5saXZlLmNvbYIcb3JpZ2luLmNud2ViLnNlYXJjaC5saXZlLmNvbYIWcHJldmlldy5sb2NhbC5saXZlLmNvbYIPc2VhcmNoLmxpdmUuY29tghJ0ZXN0Lm1hcHMubGl2ZS5jb22CDnZpZGVvLmxpdmUuY29tgg92aWRlb3MubGl2ZS5jb22CFXZpcnR1YWxlYXJ0aC5saXZlLmNvbYIMd2FwLmxpdmUuY29tghJ3ZWJtYXN0ZXIubGl2ZS5jb22CE3dlYm1hc3RlcnMubGl2ZS5jb22CFXd3dy5sb2NhbC5saXZlLmNvbS5hdYIUd3d3Lm1hcHMubGl2ZS5jb20uYXUwDQYJKoZIhvcNAQELBQADggIBADTpW/UWeupk40OP6k4yxihKStswxwqPAfMRmx4XyqmTAawAKRNM+6EZth1BQdPdOplwRTvs69kkmUHJH+ZjYXBezEACWkzEiNUQnzkRWajdSQIz08Ubj/mBD6U8xLYD+NXgiB0xNWabd8aiPsqPaj6I3qkNw4JvtgtHZQG1zlwC5/Lu6yV3DM3sKpQMyBmOnX6nVUiS0MTOzLgZOQzRk07nO7EXWGcKTmDBjE8cqv5IA/jQ6gtaxCI5pDxfXK4ct7oQyoChfxOXcEDKMmMndFmg9ch5c4an/FRM2cgzDfjR01A71LNUpLUdOjNV0T+ZEStqEpdyDFfjrHGDtzLyqEz3iyvvQFyjmlGh6OtZXwjCPpnVSrKCmfJKio0kUxyq+6t5tZAQbPVgFKiMrVnU+sgvmNVip1toijyz8vMVCkwJ2G++7xjJukoELMxZ50W4/SAMZLy1Asx02NBwYCu9+CTQPVnmPe7rmxhlQRBOfDNa1+5jwRHY64YudEzKhWR1uqS3ABd/fk+TL86yuNYGAgxnOm1FtOGieRgViV3+NzC+bDbuUOtmbD/GvDGmRwJRcCTHL7jBmkHePh2ABY93NE/IbkaDP6l1Kw98AfqkzSUxhqHXuThe7KIoX9/0zv4AA1WZFis1QvAG7dpl9eio6vCdC/73HvBAlqRL+7Mb1uu0
Copy link
Contributor

Choose a reason for hiding this comment

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

any other decent way for cert access..

Copy link
Author

Choose a reason for hiding this comment

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

what would you suggest? should we include keyvault in the sample or sth like that?
I think it's ok like this as this is just a sample.....

Copy link
Author

Choose a reason for hiding this comment

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

ok, I will update preview modules first :-)

Copy link
Contributor

Choose a reason for hiding this comment

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

ok..

Copy link
Author

Choose a reason for hiding this comment

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

maybe we could have better names for the certs, like sample-xx-cert?

subnet_name: ansiblesubnetname
appgw_name: appgw{{ rpfx }}
azure_subscription_id: "{{ lookup('env','AZURE_SUBSCRIPTION_ID') }}"
resource_group: zimsappgwnew
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't use personal name as for the name of resource_group.

Copy link
Author

Choose a reason for hiding this comment

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

yeah, and we have lots of that even in the oldest modules. we should have some naming conventions so documentation looks consinstent, and not "MyResourceGroup" / "MyRG" /"someones_rg"


- name: Create instance of Application Gateway
azure_rm_appgw:
azure_rm_appgateway:
Copy link
Contributor

Choose a reason for hiding this comment

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

In 2.7, the name is azure_rm_appgateway. But in roles, the nae is azure_rm_appgw. We should align them.

Copy link
Author

Choose a reason for hiding this comment

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

azure_rm_appgateway is correct, name was changed while reviewing / merging process to ansible/ansible. now propagating back to preview modules

location: eastus
vnet_name: ansiblevnetname
subnet_name: ansiblesubnetname
appgw_name: zimsappgw
Copy link
Contributor

Choose a reason for hiding this comment

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

pls use name like 'myAppGW'

Copy link
Contributor

@yungezz yungezz left a comment

Choose a reason for hiding this comment

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

per comments

@Fred-sun
Copy link
Contributor

@zikalino

@Fred-sun
Copy link
Contributor

Fred-sun commented Oct 9, 2018

@zikalino, reviewers request change, Thanks!

@Fred-sun
Copy link
Contributor

@zikalino Could you help changed this according by @yungezz's comments? Thanks!

@Fred-sun
Copy link
Contributor

@zikalino

@Fred-sun
Copy link
Contributor

@zikalino need your update

@Fred-sun
Copy link
Contributor

Fred-sun commented Dec 3, 2018

@zikalino Could you help recheck the PR and update according by yungezz's comment? Thanks!

@Fred-sun
Copy link
Contributor

kindly ping

@Fred-sun
Copy link
Contributor

kindly ping

@Fred-sun
Copy link
Contributor

Fred-sun commented Jan 3, 2019

@zikalino Could you help recheck this and push for merge ? Thanks!

@Fred-sun
Copy link
Contributor

@zikalino

1 similar comment
@Fred-sun
Copy link
Contributor

@zikalino

@Fred-sun
Copy link
Contributor

Fred-sun commented Mar 4, 2019

kindly ping

@Fred-sun
Copy link
Contributor

@zikalino Please change this PR according by above comments? Thanks!

@Fred-sun
Copy link
Contributor

Fred-sun commented Apr 8, 2019

@zikalino

@Fred-sun
Copy link
Contributor

@zikalino Please update the PR according by the above comment? Push this to review! Thank you very much!

@Fred-sun
Copy link
Contributor

@zikalino need you changed! Thanks a lot!

@Fred-sun
Copy link
Contributor

@zikalino When you are free, please help to solve the PR mistake. Thank you very much!

@Fred-sun
Copy link
Contributor

@zikalino Need you to change! Please update when you're free! Thank you very much!

@Fred-sun
Copy link
Contributor

@zikalino This PR has been open for a long time, please follow the comments to change and promote the review. thank you very much!

@Fred-sun
Copy link
Contributor

Fred-sun commented Sep 3, 2019

@zikalino How will this PR be handled? hasn't been updated in a long time. Please check, thank you very much!

@Fred-sun
Copy link
Contributor

@zikalino Please help finish PR changed and push for review when you're free! Thanks a lot!

@Fred-sun
Copy link
Contributor

@zikalino Please help update this PR when you're free! Thank you very much!

@Fred-sun
Copy link
Contributor

kindly ping

@Fred-sun
Copy link
Contributor

@zikalino Can you help complete the change of PR and promote the merger? It's been around for a long time--Not much to change!. Thank you very much!

@Fred-sun
Copy link
Contributor

Fred-sun commented Mar 4, 2020

@haiyuazhang @gavinfish Can you help maitain this PR? Thank you very much!

@Fred-sun
Copy link
Contributor

@zikalino Thank you for taking the time to contribute to this PR. We will transfer ansible 's azure module related Issue and PR to azure collection (https://github.com/ansible-collections/azure/pulls), can you transfer the Issue to azure collection repo?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants