Merge pull request #1173 from dtantsur/vmedia-limit

Exclude hosts with virtual media from PROVISIONING_LIMIT

docs/configuration.md

@@ -39,7 +39,8 @@ client certificate SAN validation.
 Operator. Default is the number of CPUs, but no less than 2 and no more than 8.
 
 `PROVISIONING_LIMIT` -- The desired maximum number of hosts that could be (de)provisioned
-simultaneously by the Operator. The Operator will try to enforce this limit,
+simultaneously by the Operator. The limit does not apply to hosts that use
+virtual media for provisioning. The Operator will try to enforce this limit,
 but overflows could happen in case of slow provisioners and / or higher number of
 concurrent reconciles. For such reasons, it is highly recommended to keep
 BMO_CONCURRENCY value lower than the requested PROVISIONING_LIMIT. Default is 20.

pkg/provisioner/ironic/ironic.go

@@ -1919,6 +1919,16 @@ func (p *ironicProvisioner) IsReady() (result bool, err error) {
 }
 
 func (p *ironicProvisioner) HasCapacity() (result bool, err error) {
+	bmcAccess, err := p.bmcAccess()
+	if err != nil {
+		return false, err // shouldn't actually happen so late in the process
+	}
+
+	// If the current host uses virtual media, do not limit it. Virtual
+	// media deployments may work without DHCP and can share the same image.
+	if bmcAccess.SupportsISOPreprovisioningImage() {
+		return true, nil
+	}
 
 	hosts, err := p.loadBusyHosts()
 	if err != nil {
@@ -1938,7 +1948,7 @@ func (p *ironicProvisioner) loadBusyHosts() (hosts map[string]struct{}, err erro
 
 	hosts = make(map[string]struct{})
 	pager := nodes.List(p.client, nodes.ListOpts{
-		Fields: []string{"uuid,name,provision_state,driver_internal_info,target_provision_state"},
+		Fields: []string{"uuid,name,provision_state,boot_interface"},
 	})
 
 	page, err := pager.AllPages()
@@ -1958,7 +1968,11 @@ func (p *ironicProvisioner) loadBusyHosts() (hosts map[string]struct{}, err erro
 			nodes.Inspecting, nodes.InspectWait,
 			nodes.Deploying, nodes.DeployWait,
 			nodes.Deleting:
-			hosts[node.Name] = struct{}{}
+			// FIXME(dtantsur): this is a bit silly, but we don't have an easy way
+			// to reconstruct AccessDetails from a DriverInfo.
+			if !strings.Contains(node.BootInterface, "virtual-media") {
+				hosts[node.Name] = struct{}{}
+			}
 		}
 	}
 

pkg/provisioner/ironic/provisioncapacity_test.go

@@ -24,6 +24,8 @@ func TestHasCapacity(t *testing.T) {
 		provisioningLimit int
 		nodeStates        []nodes.ProvisionState
 		hostName          string
+		bootInterface     string
+		bmcAddress        string
 
 		expectedHasCapacity bool
 		expectedError       string
@@ -55,6 +57,22 @@ func TestHasCapacity(t *testing.T) {
 			provisioningLimit: 1,
 			nodeStates:        []nodes.ProvisionState{nodes.Active, nodes.AdoptFail, nodes.Adopting, nodes.Available, nodes.CleanFail},
 
+			expectedHasCapacity: true,
+		},
+		{
+			name:              "enough-capacity-due-virtual-media",
+			provisioningLimit: 1,
+			nodeStates:        states,
+			bmcAddress:        "redfish-virtualmedia://example.com/redfish/v1/Systems/1",
+
+			expectedHasCapacity: true,
+		},
+		{
+			name:              "enough-capacity-due-other-virtual-media",
+			provisioningLimit: 1,
+			nodeStates:        states,
+			bootInterface:     "redfish-virtual-media",
+
 			expectedHasCapacity: true,
 		},
 	}
@@ -67,6 +85,7 @@ func TestHasCapacity(t *testing.T) {
 				allNodes = append(allNodes, nodes.Node{
 					Name:           fmt.Sprintf("myns%snode-%d", nameSeparator, n),
 					ProvisionState: string(state),
+					BootInterface:  tc.bootInterface,
 				})
 			}
 
@@ -78,6 +97,9 @@ func TestHasCapacity(t *testing.T) {
 
 			host := makeHost()
 			host.Name = tc.hostName
+			if tc.bmcAddress != "" {
+				host.Spec.BMC.Address = tc.bmcAddress
+			}
 
 			auth := clients.AuthConfig{Type: clients.NoAuth}