fix: remove PowerShell from Windows registry interactions #2993
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rbtr
added
cns
rbtr
force-pushed
the
fix/depwsh
branch
5 times, most recently
from
6cf43ba
to
3496998
Compare
|
In #2315, it was changed to conditionally because there was a livesite that our customer got error during the cns start up to set SdnRemoteArpMacAddress. somehow hns was not enabled on their machines so we need to skip this for them. |
|
@ZetaoZhuang is it okay to try to set it? Or can we not even try to set it if HNS is not enabled? |
from that livesite, if we set it when hns is not enabled, we will get error complaining about something like "hns state path does not exist" or so. that is the reason why we made it conditional. I believe we could set it if it does not complain anymore. |
ZetaoZhuang
reviewed
Sep 11, 2024
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
rbtr
force-pushed
the
fix/depwsh
branch
3 times, most recently
from
9b9d321
to
f1f6c51
Compare
Signed-off-by: Evan Baker <[email protected]>
|
/azp run Azure Container Networking PR |
|
Azure Pipelines successfully started running 1 pipeline(s). |
rbtr
added a commit
that referenced
this pull request
Nov 22, 2024
remove powershell from windows registry txs Signed-off-by: Evan Baker <[email protected]>
rbtr
added a commit
that referenced
this pull request
Nov 22, 2024
remove powershell from windows registry txs Signed-off-by: Evan Baker <[email protected]>
rbtr
added a commit
that referenced
this pull request
Nov 22, 2024
remove powershell from windows registry txs Signed-off-by: Evan Baker <[email protected]>
This was referenced Nov 22, 2024
github-merge-queue bot
pushed a commit
that referenced
this pull request
Nov 27, 2024
) remove powershell from windows registry txs Signed-off-by: Evan Baker <[email protected]>
github-merge-queue bot
pushed a commit
that referenced
this pull request
Nov 27, 2024
) remove powershell from windows registry txs Signed-off-by: Evan Baker <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reason for Change:
PowerShell is heavy (+30mb of memory footprint per invocation). We don't need to use it to interact with the Windows Registry (or ServiceManager) - the Go stdlib has good native bindings that should use instead.
Notes:
This functionality was introduced in #1306. In #2315, it was changed to conditionally set the reg key value only if the HNS path exists. Why? It seems simpler and harmless to always try to set the reg key, so that's what I have done. Unclear if this is problematic - cc @ZetaoZhuang
Supersedes and closes #2974 and #2961 (and #2949)