-
Notifications
You must be signed in to change notification settings - Fork 260
Add cleanup network script #1371
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
Conversation
cni/scripts/cleanupnetwork.ps1
Outdated
| @@ -0,0 +1,76 @@ | |||
| $Global:ClusterConfiguration = ConvertFrom-Json ((Get-Content "c:\k\kubeclusterconfig.json" -ErrorAction Stop) | out-string) | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this works only for k8s cluster.. can we make this script work irrespective of orchestrator?
cni/scripts/cleanupnetwork.ps1
Outdated
| $global:NetworkMode = "L2Bridge" | ||
| $global:ContainerRuntime = $Global:ClusterConfiguration.Cri.Name | ||
| $global:NetworkPlugin = $Global:ClusterConfiguration.Cni.Name | ||
| $global:HNSModule = "c:\k\hns.psm1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can get this from their github repo directly instead of relying on local disk
tamilmani1989
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor suggestion
cni/scripts/cleanupnetwork.ps1
Outdated
| @@ -0,0 +1,73 @@ | |||
| # ./cleanupnetwork.ps1 -CniName azure | |||
| param ( | |||
| [Parameter(Mandatory=$true)][string]$CniName | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shud be networkname
cni/scripts/cleanupnetwork.ps1
Outdated
| if ($global:NetworkPlugin -eq "azure") { | ||
| $networkname = "azure" | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove this code?
cni/scripts/cleanupnetwork.ps1
Outdated
| } | ||
| } | ||
|
|
||
| if ($global:NetworkPlugin -eq "azure") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove this check
cni/scripts/cleanupnetwork.ps1
Outdated
|
|
||
| Invoke-WebRequest -Uri https://raw.githubusercontent.com/microsoft/SDN/master/Kubernetes/windows/hns.psm1 -OutFile "c:\hns.psm1" -UseBasicParsing | ||
|
|
||
| $global:NetworkMode = "L2Bridge" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is networkmode used anywhere. if not remove it
cni/scripts/cleanupnetwork.ps1
Outdated
| @@ -0,0 +1,43 @@ | |||
| # ./cleanupnetwork.ps1 -CniDirectory c:\k -NetworkName azure | |||
| param ( | |||
| [string]$CniDirectory = "c:\k", | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets have c:\windows\system32 as default
cni/scripts/cleanupnetwork.ps1
Outdated
| # ./cleanupnetwork.ps1 -CniDirectory c:\k -NetworkName azure | ||
| param ( | ||
| [string]$CniDirectory = "c:\k", | ||
| [Parameter(Mandatory=$true)][string]$NetworkName |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove networkname parameter if not used
tamilmani1989
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
* add cleanup network script * generalize * generic cwd * loop through with prefix * simplify checks * remove network option
Reason for Change:
In the event that HNS gets into a bad state, due to some other service configuring HNS outside of CNI, this script can be used to provide manual mitigation
Issue Fixed:
Requirements:
Notes: