Additional validations for DHCP enabled container host (#102)

* Additional validations for DHCP enabled container host

* Removing debugs/comments

* Update documentation

* Minor change

* Minor changes

* Containerize Infra Params

* Update SPEC.md

scripts/autogencniconf/SPEC.md

@@ -16,7 +16,8 @@
   - [Appendix: Examples](#appendix-examples)
     - [Basic Conf](#basic-conf)
     - [Conf with additional policies](#conf-with-additional-policies)
-	- [Sample auto-generated CNI configuration](#sample-auto-generated-cni-configuration)
+    - [Conf with DHCP parameters](#conf-with-dhcp-parameters)
+    - [Sample auto-generated CNI configuration](#sample-auto-generated-cni-configuration)
 
 ## Version
 
@@ -30,7 +31,7 @@ Released versions of the spec are available as Git tags.
 
 | tag                                                                                  | spec permalink                                                                        | major changes                     |
 | ------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------- | --------------------------------- |
-| [`spec-v1.0.0`](https://github.com/microsoft/windows-container-networking/cni/releases/tag/spec-v1.0.0) | [spec at v1.0.0](https://github.com/microsoft/windows-container-networking/cni/blob/spec-v1.0.0/SPEC.md) | Removed non-list configurations; removed `version` field of `interfaces` array |
+| [`spec-v1.0.0`](https://github.com/microsoft/windows-container-networking/cni/releases/tag/spec-v1.0.0) | [spec at v1.0.0](https://github.com/microsoft/windows-container-networking/cni/blob/spec-v1.0.0/SPEC.md) | Initial draft |
 
 *Do not rely on these tags being stable.  In the future, we may change our mind about which particular commit is the right marker for a given historical spec version.*
 
@@ -76,7 +77,10 @@ Below sections specify the JSON format that needs to be passed after [encoding](
 - `LocalEndpoint` (string): IP Address of the local endpoint. Used to configure default policies for the endpoint. This parameter is *MANDATORY*.
 - `InfraPrefix` (string): CIDR of the management network of the underlying node. Used to configure default policies for the network. This parameter is *MANDATORY*.
 - `AddditionalPolicies` (dictionary): Defined [here](#configure-additional-policies). This parameter is *NOT MANDATORY*.
-### Configure Additional Policies
+- `InfraParams` (dictionary): This parameter contains configurations that are not translated into any CNI conf field, it is only meaningful to the infrastructure layer. This parameter is *NOT MANDATORY*.
+	- `DhcpEnabled` (boolean): Set to true if the container host management interface is expected to have a DHCP leased IP. This parameter is *NOT MANDATORY*. 
+	- `DhcpCheckTimeout` (integer): Wait for this time interval in seconds for the DHCP IP to get assigned before creating the HNS Network and generating the CNI conf. This parameter is *NOT MANDATORY*. This parameter can only be set if 'DhcpEnabled' field is set to true.
+### Configure Additional Policies 
 #### ACL Policy
 There are few system-defined default ACL policies. Users can configure additional ACL polices with below parameters.
 - `RemoteAddresses` (string): This parameter is *NOT MANDATORY*.
@@ -91,8 +95,8 @@ There are few system-defined default ACL policies. Users can configure additiona
 #### OutBound NAT Policy
 - `Exceptions` (string[]): List of IP Addresses/CIDRs to allow NATed outbound traffic. This parameter is *MANDATORY*.
 #### SDNRoute Policy
-- `DestinationPrefix` (string): .This parameter is *MANDATORY*.
-- `NeedEncap` (bool): . This parameter is *MANDATORY*.
+- `DestinationPrefix` (string): This parameter is *MANDATORY*.
+- `NeedEncap` (bool): This parameter is *MANDATORY*.
 ## Appendix: Examples
 ### Basic Conf
 ```jsonc
@@ -144,6 +148,18 @@ There are few system-defined default ACL policies. Users can configure additiona
 	]
 }
 ```
+### Conf with DHCP parameters
+```jsonc
+{
+	"Name": "azure-cni",
+	"Type": "sdnbridge",
+	"Subnet": "192.168.0.0/24",
+	"InfraPrefix": "172.16.0.0/24",
+	"Gateway": "192.168.0.2",
+	"DnsServers": "8.8.8.8",
+	"InfraParams" : { "DhcpEnabled": true, "DhcpCheckTimeout": 90 }
+}
+```
 ### Sample auto-generated CNI configuration
 ```jsonc
 VERBOSE: Generated CNI conf: .\cni.conf

scripts/autogencniconf/generateCNIConfig.ps1

@@ -55,6 +55,8 @@ set-variable -name ACL_POLICY -value ([string]"ACL") -Scope Script
 set-variable -name DEFAULT_PRIORITY -value ([string]"-1") -Scope Script # Used to help in sorting the policies based on priority even if priority is not specified by user
 set-variable -name USER_POLICY_PRIO_START -value ([int]3000) -Scope Script
 set-variable -name USER_POLICY_PRIO_END -value ([int]8000) -Scope Script
+set-variable -name DHCP_CHECK_TIMEOUT_MIN -value ([int]60) -Scope Script
+set-variable -name DHCP_CHECK_TIMEOUT_MAX -value ([int]600) -Scope Script
 
 enum OptionalKeysFlag {
     NoOptKeys = 1
@@ -81,6 +83,11 @@ class Policy {
     }
 }
 
+class InfraParams {
+    [bool] $DhcpEnabled
+    [int] $DhcpCheckTimeout
+}
+
 class CniArgs {
     [string] $Name
     [string] $Type
@@ -92,6 +99,7 @@ class CniArgs {
     [string[]] $DnsServers
     [Policy[]] $AdditionalPolicies
     [bool] $SkipDefaultPolicies # Undocumented parameter to disable system policies
+    [InfraParams] $InfraParams
 
     CniArgs([System.Object] $cniArgs) {
         # Mandatory Parameters
@@ -106,6 +114,26 @@ class CniArgs {
         # Optional Parameters
         if ($cniArgs.psobject.Properties.name.Contains('Version')) {$this.Version = $cniArgs.Version} else {$this.Version = $script:DEFAULT_CNI_VERSION}
         if ($cniArgs.psobject.Properties.name.Contains('SkipDefaultPolicies')) {$this.SkipDefaultPolicies = $true} else {$this.SkipDefaultPolicies = $false}
+        if ($cniArgs.psobject.Properties.name.Contains('InfraParams')) {
+            $cniArgsInfraParams = $cniArgs.InfraParams
+            $this.InfraParams = [InfraParams]::new()
+            if ($cniArgsInfraParams.psobject.Properties.name.Contains('DhcpEnabled')) {$this.InfraParams.DhcpEnabled = $cniArgsInfraParams.DhcpEnabled}
+            if ($this.InfraParams.DhcpEnabled -eq $true) {
+                $this.InfraParams.DhcpCheckTimeout = $script:DHCP_CHECK_TIMEOUT_MIN # Default value of 60 secs,  based on RFC 1541
+                if ($cniArgsInfraParams.psobject.Properties.name.Contains('DhcpCheckTimeout')) {
+                    if((-not (($cniArgsInfraParams.DhcpCheckTimeout -ge $script:DHCP_CHECK_TIMEOUT_MIN) -and ($cniArgsInfraParams.DhcpCheckTimeout -le $script:DHCP_CHECK_TIMEOUT_MAX))) -or (($cniArgsInfraParams.DhcpCheckTimeout % 10) -ne 0)) {
+                        Write-Verbose -Message ("DHCP Check timeout should be a multiple of 10 and have a value between {0} - {1}. Invalid dhcp check timeout parameter: {2}" -f $script:DHCP_CHECK_TIMEOUT_MIN, $script:DHCP_CHECK_TIMEOUT_MAX, $cniArgsInfraParams.DhcpCheckTimeout)
+                        throw ("Invalid DHCP Check timeout parameter: {0}" -f $cniArgsInfraParams.DhcpCheckTimeout)
+                    }
+                    $this.InfraParams.DhcpCheckTimeout = $cniArgsInfraParams.DhcpCheckTimeout
+                }
+            } else {
+                if ($cniArgsInfraParams.psobject.Properties.name.Contains('DhcpCheckTimeout')) {
+                    Write-Verbose -Message ("DHCP Check timeout should be a configured only when DhcpEnabled is set.")
+                    throw "Invalid DHCP Check timeout parameter should be configured only when DhcpEnabled parameter is set. Missing parameter DhcpEnabled."
+                }
+            }
+        }
         if ($cniArgs.psobject.Properties.name.Contains('AdditionalPolicies')) {
             for($i=0; $i -lt $cniArgs.AdditionalPolicies.length; $i++) {
                 # Following constraints are ensured for policy priorities (HNS supports priorities between 100-65500 for ACLs)
@@ -323,3 +351,5 @@ Remove-Variable -name DEFAULT_PRIORITY -Scope Script
 Remove-Variable -name ACL_POLICY -Scope Script
 Remove-Variable -name USER_POLICY_PRIO_START -Scope Script
 Remove-Variable -name USER_POLICY_PRIO_END -Scope Script
+Remove-Variable -name DHCP_CHECK_TIMEOUT_MIN -Scope Script
+Remove-Variable -name DHCP_CHECK_TIMEOUT_MAX -Scope Script

scripts/autogencniconf/test/autogencniconf_test.go

@@ -49,8 +49,19 @@ var _ = BeforeSuite(func() {
 	Expect(scriptPath).To(BeAnExistingFile())
 })
 
+
+
 var _ = Describe("Autogen CNI conf Tests", func() {
 
+	AfterEach(func() {
+		// Remove the cni conf if generated after every test case
+		_, err := os.Stat(cniConfPath)
+		if err == nil {
+			err = os.Remove(cniConfPath)
+			Expect(err).NotTo(HaveOccurred(), "cni conf file [%v] deletion failed with err [%s]", cniConfPath, err)
+		}
+  	})
+
 	// Test Case 1
 	It("Verifies that error is thrown if input json is invalid", func() {
 		cniArgs := getEncodedCniArgs("tc1_input.json")
@@ -232,4 +243,58 @@ var _ = Describe("Autogen CNI conf Tests", func() {
 			Expect(cniConfPath).NotTo(BeAnExistingFile())
 		})
 	})
+
+	// Test Case 8
+	When("Setting up networking for a DHCP enabled host", func() {
+
+		It("Verifies lower limit of the DHCP check timeout is honored", func() {
+			cniArgs := getEncodedCniArgs("tc8a_input.json")
+			Expect(cniArgs).NotTo(BeEmpty())
+
+			cmd := exec.Command("powershell", scriptPath, "-CniArgs", cniArgs, "-CniConfPath", cniConfPath)
+			out, err := cmd.CombinedOutput()
+			Expect(err).To(HaveOccurred(), "cmd [%s] failed with error [%v]. output is [%s]", cmd, err, out)
+			Expect(cniConfPath).NotTo(BeAnExistingFile())
+		})
+
+		It("Verifies higher limit of the DHCP check timeout is honored", func() {
+			cniArgs := getEncodedCniArgs("tc8b_input.json")
+			Expect(cniArgs).NotTo(BeEmpty())
+
+			cmd := exec.Command("powershell", scriptPath, "-CniArgs", cniArgs, "-CniConfPath", cniConfPath)
+			out, err := cmd.CombinedOutput()
+			Expect(err).To(HaveOccurred(), "cmd [%s] failed with error [%v]. output is [%s]", cmd, err, out)
+			Expect(cniConfPath).NotTo(BeAnExistingFile())
+		})
+
+		It("Verifies the DHCP check timeout is a multiple of 10", func() {
+			cniArgs := getEncodedCniArgs("tc8c_input.json")
+			Expect(cniArgs).NotTo(BeEmpty())
+
+			cmd := exec.Command("powershell", scriptPath, "-CniArgs", cniArgs, "-CniConfPath", cniConfPath)
+			out, err := cmd.CombinedOutput()
+			Expect(err).To(HaveOccurred(), "cmd [%s] failed with error [%v]. output is [%s]", cmd, err, out)
+			Expect(cniConfPath).NotTo(BeAnExistingFile())
+		})
+
+		It("Verifies default DHCP check timeout is used if the parameter is missing", func() {
+			cniArgs := getEncodedCniArgs("tc8d_input.json")
+			Expect(cniArgs).NotTo(BeEmpty())
+
+			cmd := exec.Command("powershell", scriptPath, "-CniArgs", cniArgs, "-CniConfPath", cniConfPath)
+			out, err := cmd.CombinedOutput()
+			Expect(err).NotTo(HaveOccurred(), "cmd [%s] failed with error [%v]. output is [%s]", cmd, err, out)
+			Expect(cniConfPath).To(BeAnExistingFile())
+		})
+
+		It("Verifies DHCP check timeout is honored only if DhcpEnabled parameter is set", func() {
+			cniArgs := getEncodedCniArgs("tc8e_input.json")
+			Expect(cniArgs).NotTo(BeEmpty())
+
+			cmd := exec.Command("powershell", scriptPath, "-CniArgs", cniArgs, "-CniConfPath", cniConfPath)
+			out, err := cmd.CombinedOutput()
+			Expect(err).To(HaveOccurred(), "cmd [%s] failed with error [%v]. output is [%s]", cmd, err, out)
+			Expect(cniConfPath).NotTo(BeAnExistingFile())
+		})
+	})
 })

scripts/autogencniconf/test/configs/tc8a_input.json

@@ -0,0 +1,10 @@
+{
+    "Name": "azure-cni",
+    "Type": "sdnbridge",
+    "Subnet": "192.168.0.0/24",
+    "Gateway": "192.168.0.2",
+    "InfraPrefix": "10.0.0.0/24",
+    "DnsServers": ["168.63.129.16"],
+    "ManagementIp": "10.0.0.10",
+    "InfraParams": { "DhcpEnabled": true, "DhcpCheckTimeout": 5 }
+}

scripts/autogencniconf/test/configs/tc8b_input.json

@@ -0,0 +1,10 @@
+{
+    "Name": "azure-cni",
+    "Type": "sdnbridge",
+    "Subnet": "192.168.0.0/24",
+    "Gateway": "192.168.0.2",
+    "InfraPrefix": "10.0.0.0/24",
+    "DnsServers": ["168.63.129.16"],
+    "ManagementIp": "10.0.0.10",
+    "InfraParams": { "DhcpEnabled": true, "DhcpCheckTimeout": 800 }
+}

scripts/autogencniconf/test/configs/tc8c_input.json

@@ -0,0 +1,10 @@
+{
+    "Name": "azure-cni",
+    "Type": "sdnbridge",
+    "Subnet": "192.168.0.0/24",
+    "Gateway": "192.168.0.2",
+    "InfraPrefix": "10.0.0.0/24",
+    "DnsServers": ["168.63.129.16"],
+    "ManagementIp": "10.0.0.10",
+    "InfraParams": { "DhcpEnabled": true, "DhcpCheckTimeout": 65 }
+}

scripts/autogencniconf/test/configs/tc8d_input.json

@@ -0,0 +1,10 @@
+{
+    "Name": "azure-cni",
+    "Type": "sdnbridge",
+    "Subnet": "192.168.0.0/24",
+    "Gateway": "192.168.0.2",
+    "InfraPrefix": "10.0.0.0/24",
+    "DnsServers": ["168.63.129.16"],
+    "ManagementIp": "10.0.0.10",
+    "InfraParams": { "DhcpEnabled": true }
+}

scripts/autogencniconf/test/configs/tc8e_input.json

@@ -0,0 +1,10 @@
+{
+    "Name": "azure-cni",
+    "Type": "sdnbridge",
+    "Subnet": "192.168.0.0/24",
+    "Gateway": "192.168.0.2",
+    "InfraPrefix": "10.0.0.0/24",
+    "DnsServers": ["168.63.129.16"],
+    "ManagementIp": "10.0.0.10",
+    "InfraParams": { "DhcpCheckTimeout": 80 }
+}