Skip to content

Refactor Initialize-NatNetwork missing CNI plugin installation (3/4) #54

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

Merged

Conversation

TinaMor
Copy link
Contributor

@TinaMor TinaMor commented Feb 6, 2025

PR description

Refactor initialize-NatNetwork command to:

  1. Check (and install) missing plugins first before checking if the network exists.
  2. Default the WinCNI Path to $ENV:Programfiles\containerd\cni

Testing information

> Initialize-NatNetwork -Force -Confirm:$false -Gateway 192.168.0.1

Creating NAT network
WARNING: Windows CNI plugins have not been installed. Installing Windows CNI plugins at 'C:\Program Files\containerd\cni'
Downloading CNI plugin version 0.3.1 at C:\Program Files\containerd\cni
Extracting WinCNIPlugin to C:\Program Files\containerd\cni\bin
Cleanup to remove downloaded files
CNI plugin version 0.3.1 (microsoft/windows-container-networking) successfully installed at C:\Program Files\containerd\cni
WARNING: nat already exists. To view existing networks, use "Get-HnsNetwork". To remove the existing network use the "Remove-HNSNetwork" command.

Checklist

As part of our commitment to engineering excellence, before submitting this PR, please make sure:

  • You've tested this code in both Desktop & Server environments and AMD & ARM64 enviroments (functional testing).
  • You've added unit tests for new code.
  • You've added/updated documentation in the cmdlet docs, command-reference.md and the modules help files.
  • You've reviewed the PR/code best practices defined in the CONTRIBUTING.md.

In addition, after this PR has been reviewed, please agree to:

  • If changes have been made to your PR in the process of addressing comments, please make sure to test again the final version in both AMD and ARM64 environments.
  • Validate your changes have not introduced any regressions.

@TinaMor TinaMor changed the title Refactor Initialize-NatNetwork mising CNI plugin installation Refactor Initialize-NatNetwork mising CNI plugin installation (3/4) Mar 11, 2025
@TinaMor TinaMor changed the title Refactor Initialize-NatNetwork mising CNI plugin installation (3/4) Refactor Initialize-NatNetwork missing CNI plugin installation (3/4) Mar 11, 2025
@TinaMor TinaMor force-pushed the tinamor/refactor-Initialize-NatNetwork branch 4 times, most recently from 57c9bff to 1e821cb Compare March 12, 2025 12:59
@TinaMor TinaMor force-pushed the tinamor/refactor-Initialize-NatNetwork branch from 1e821cb to 4e56577 Compare March 12, 2025 15:39
@TinaMor TinaMor marked this pull request as ready for review March 12, 2025 15:41
@TinaMor TinaMor requested a review from iankingori as a code owner March 12, 2025 15:41
if (!$Force) {
$title = "Windows CNI plugins have not been installed."
$question = "Do you want to install the Windows CNI plugins?"
$choices = '&Yes', '&No'

Choose a reason for hiding this comment

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

Why did we remove the consent prompt for additional installation when -force is used?

Copy link
Contributor Author

@TinaMor TinaMor Mar 12, 2025

Choose a reason for hiding this comment

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

We implemented the function using ShouldProcess() which handles -Confirm flag. This then became redundant.


$PSCmdlet.ShouldProcess

@TinaMor TinaMor merged commit 90003ba into microsoft:main Apr 8, 2025
6 checks passed
@TinaMor TinaMor deleted the tinamor/refactor-Initialize-NatNetwork branch April 15, 2025 11:06
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.

3 participants