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

Add routing capabilities to the nested workload #5

Merged
merged 2 commits into from
Mar 12, 2024

Conversation

lrivallain
Copy link
Contributor

  • Add routing capabilities to the nested workload by deploying a router VM
    • Enable to test HCX networking including MON
  • Add a way to build labs without using the System Assigned Managed Identity
    • Scripts will use a user-customized yaml configuration file to get IP, users and passwords
    • Readme was updated to match this deployement method
  • Cleanup of some unused code and comments

* Add routing capabilities to the nested workload by deploying a router VM
  * Enable to test HCX networking including MON
* Add a way to build labs without using the System Assigned Managed Identity
  * Scripts will use a user-customized yaml configuration file to get IP, users and passwords
  * Readme was updated to match this deployement method
* Cleanup of some unused code and comments
lrivallain referenced this pull request in lrivallain/avshub Jan 3, 2024
Updated module 2 based on feedback and changes made for the last LevelUp.

Added new scenarios to test:

* Replication Assisted vMotion
* Mobility Optimized Network
* Network extension cutover

The MON scenario will only work with the following changes in
[https://github.com/Azure/avslabs/pull/5][avslabs] content to
provide routing capabilities to the lab.

Also made some organisation change to simplify the reading of
instructions.
@lrivallain
Copy link
Contributor Author

Ping @husamhilal : if you have any chance of reviewing this change proposal.

@husamhilal
Copy link
Collaborator

Ping @husamhilal : if you have any chance of reviewing this change proposal.

Thanks a lot @lrivallain . Please allow me couple days to review the proposed changes. I really appreciate your work here, and I really like adding a router VM. Thanks a lot!

Copy link
Collaborator

@husamhilal husamhilal left a comment

Choose a reason for hiding this comment

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

@lrivallain Thank you so much for all the changes and updates. I really loved the idea of adding the router to fix the routing blocker from the workload segments. I'm hoping that with this we can RDP or SSH into the VMs deployed in the nested environment.

I made few comments below, some of them require some minor updates from your end. Can you please update? Once you update, I'll go ahead and approve the PR 👍

scripts/nestedlabs.yml Outdated Show resolved Hide resolved
scripts/router-userdata.yaml Outdated Show resolved Hide resolved
scripts/labdeploy.ps1 Show resolved Hide resolved
scripts/labdeploy.ps1 Show resolved Hide resolved
$sRouteURL = "https://$nsxtHost/policy/api/v1/infra/tier-1s/$t1Name/static-routes/$NewVcVAppName"
$sRoute = Invoke-RestMethod -Uri $sRouteURL -Headers $Header -Method PUT -Body $Body -ContentType "application/json" -SkipCertificateCheck
Start-Sleep 15
Write-Log "Static route $NewVcVAppName created....."
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please, add logic to dispose vCenter connection: Disconnect-VIServer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

line 1004 ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@lrivallain . I cannot find line 1004. But yes. toward the end. We can check if the connection is still open, then we should close it to avoid exhausting resources. Also same for NSX-T please.

scripts/bootstrap.ps1 Show resolved Hide resolved
}
}
}
} else {
Write-Log "Building labs without using the System Assigned Managed Identity"
if (Test-Path "$TempPath/nestedlabs.yml" -PathType Leaf) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you put the following 4/5 lines in a seperate function and call it from here. just for clarity and consistency with the rest of the code. For example the function name can be:
Get-NestedLabConfigurationsFromYaml

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed a bit the logic behind to simplify the "if" sequence and to create a Set-NestedLabConfigurationsFromYaml. The function will try to copy the YAML file to the extraction path, if no file exists, the script will continue to managed ID method.

scripts/bootstrap-nestedlabs.ps1 Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@lrivallain
Copy link
Contributor Author

Hi @husamhilal ! Thanks a lot for the review. Very constructive !
I left opened some points for you to review or for discussion if I misunderstood.

Routing capabilities enable to connect to the workload VMs and to test a set of new HCX scenarios that I submitted to the avshub repository: Azure/avshub#7 like :

  • Mobility Optimized Network
  • Network extension cutover

$sRouteURL = "https://$nsxtHost/policy/api/v1/infra/tier-1s/$t1Name/static-routes/$NewVcVAppName"
$sRoute = Invoke-RestMethod -Uri $sRouteURL -Headers $Header -Method PUT -Body $Body -ContentType "application/json" -SkipCertificateCheck
Start-Sleep 15
Write-Log "Static route $NewVcVAppName created....."
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

@lrivallain . I cannot find line 1004. But yes. toward the end. We can check if the connection is still open, then we should close it to avoid exhausting resources. Also same for NSX-T please.

@husamhilal husamhilal merged commit 2b92f39 into Azure:main Mar 12, 2024
1 check 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.

2 participants