disable cleaning up hns network and state files from windows cni #879
Conversation
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
cnm/ipam/ipam.go
Outdated
| requiresRequestReplay = false | ||
| returnCode = 0 | ||
| returnStr = "Success" | ||
| rehydrateIpamInfoOnReboot = true |
There was a problem hiding this comment.
I thought this should be "false"?
There was a problem hiding this comment.
this is not cni..true for cnm
…working into remove_cleanup
tamilmani1989
force-pushed
the
remove_cleanup
branch
from
42d3a96 to
e3bf3e1
Compare
|
|
Nope this one: https://github.com/Azure/aks-engine/blob/master/staging/provisioning/windows/cleanupnetwork.ps1 |
cni/ipam/ipam.go
Outdated
|
|
||
| const ( | ||
| ipamV6 = "azure-vnet-ipamv6" | ||
| rehydrateIpamInfoOnReboot = false |
cni/network/network.go
Outdated
|
|
||
| const ( | ||
| rehydrateNetworkInfoOnReboot = false | ||
| ) |
There was a problem hiding this comment.
why do we need a constant false? this just hides whatever the real test is from me when I read the code where it is checked (like, is this actually a "is running on windows" bit?)
I think it would be much clearer if, where ever we use this conditional, we were checking the actual underlying condition that makes this switch
There was a problem hiding this comment.
nope this is not running on windows.. cnm and cni uses same code path. have to rehydrate for cnm but not for cni
cni/network/network_linux.go
Outdated
| ) | ||
|
|
||
| const ( | ||
| snatConfigFileName = "/tmp/snatConfig" |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
auto trigger pipeline getting failed due to some infra issue. Manually triggered pipeline for this PR: https://msazure.visualstudio.com/One/_build/results?buildId=44757594&view=results |
5 participants
Reason for Change:
Disable cleaning up hns network and state files from windows cni. Since state and hns network is cleaned up by CRI startup script, no need for cni to cleanup those.
Issue Fixed:
Requirements:
Notes:
This PR is workaround for the issue: #947