Skip to content

Making change in SQL MI tests to delegate subnet to SQL MI service#11874

Merged
VeryEarly merged 2 commits intoAzure:masterfrom
stefbomdcs:subnetdelegationtosqlmi
May 25, 2020
Merged

Making change in SQL MI tests to delegate subnet to SQL MI service#11874
VeryEarly merged 2 commits intoAzure:masterfrom
stefbomdcs:subnetdelegationtosqlmi

Conversation

@stefbomdcs
Copy link
Copy Markdown
Contributor

@stefbomdcs stefbomdcs commented May 11, 2020 •
edited
Loading

Description

Making change in SQL MI create related tests to have subnet delegated to SQL MI service as this will be forced by SQL MI and all create requests for non-delegated subnets will fail.

Checklist

  • I have read the Submitting Changes section of CONTRIBUTING.md
  • The title of the PR is clear and informative
  • The appropriate ChangeLog.md file(s) has been updated:
    • For any service, the ChangeLog.md file can be found at src/{{SERVICE}}/{{SERVICE}}/ChangeLog.md
    • A snippet outlining the change(s) made in the PR should be written under the ## Upcoming Release header -- no new version header should be added
  • The PR does not introduce breaking changes
  • If applicable, the changes made in the PR have proper test coverage
  • For public API changes to cmdlets:
    • a cmdlet design review was approved for the changes in this repository (Microsoft internal only)
    • the markdown help files have been regenerated using the commands listed here

@adxsdkps
Copy link
Copy Markdown
Collaborator

adxsdkps commented May 11, 2020

Can one of the admins verify this patch?

@msftclas
Copy link
Copy Markdown

msftclas commented May 11, 2020 •
edited
Loading

CLA assistant check
All CLA requirements met.

@stefbomdcs stefbomdcs changed the title Making change in tests to delegate subnet to SQL MI service Making change in SQL MI tests to delegate subnet to SQL MI service May 11, 2020
@VeryEarly
Copy link
Copy Markdown
Collaborator

VeryEarly commented May 12, 2020

/azp run

@azure-pipelines
Copy link
Copy Markdown
Contributor

azure-pipelines bot commented May 12, 2020

Azure Pipelines successfully started running 3 pipeline(s).

@VeryEarly VeryEarly requested a review from petrajkogit May 12, 2020 01:43
@VeryEarly
Copy link
Copy Markdown
Collaborator

VeryEarly commented May 12, 2020

Hi @petrajkogit ,

since the change is on Common.ps1, can you please verify whether it has impact on other test cases

@VeryEarly VeryEarly self-assigned this May 12, 2020
@stefbomdcs
Copy link
Copy Markdown
Contributor Author

stefbomdcs commented May 12, 2020 •
edited
Loading

Hi @petrajkogit ,

since the change is on Common.ps1, can you please verify whether it has impact on other test cases

I've run one of the tests which is using the changed method. However, it looks like I'll need to run all tests which are using this method in recording mode, as there is one more ARM call added with this change.

@stefbomdcs
Change to add NSG to the subnet if there is no NSG assigned
09325c7 
Adding recording from one of the tests
@stefbomdcs
Copy link
Copy Markdown
Contributor Author

stefbomdcs commented May 14, 2020

@VeryEarly
Do you maybe know why it takes this long for these checks to complete?

@VeryEarly
Copy link
Copy Markdown
Collaborator

VeryEarly commented May 18, 2020

/azp run

@azure-pipelines
Copy link
Copy Markdown
Contributor

azure-pipelines bot commented May 18, 2020

Azure Pipelines successfully started running 3 pipeline(s).

@VeryEarly
Copy link
Copy Markdown
Collaborator

VeryEarly commented May 18, 2020

@VeryEarly
Do you maybe know why it takes this long for these checks to complete?

It looks like the CI was not triggered

@stefbomdcs
Copy link
Copy Markdown
Contributor Author

stefbomdcs commented May 18, 2020

@VeryEarly
Thanks for starting this. Am I missing some permission as it looks like that Merging is blocked?

@VeryEarly
Copy link
Copy Markdown
Collaborator

VeryEarly commented May 19, 2020

@stefbomdcs @petrajkogit ,

It looks to me the function CreateAndGetVirtualNetworkForManagedInstance
was been used by not just one test case.

Please also re-record test cases that referenced this function.

@stefbomdcs
Copy link
Copy Markdown
Contributor Author

stefbomdcs commented May 21, 2020

@VeryEarly
Yes, this function is referenced in all MI related test cases.
The tests which are creating MI are very expensive and creation of MI lasts about 4h.
I've tested the change thoroughly for all possible scenarios as it is scoped to adding delegation and NSG object to subnet.
Running all the tests will be very painful and time consuming.

@VeryEarly VeryEarly merged commit a313c79 into Azure:master May 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@petrajkogit petrajkogit petrajkogit approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@VeryEarly VeryEarly

Labels

needs-revision

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@stefbomdcs @adxsdkps @msftclas @VeryEarly @petrajkogit