Skip to content

Commit

Permalink
Add 'EvictionStrategy: External' for external controllers that need t…
Browse files Browse the repository at this point in the history
…o react to VMI eviction

The cluster-api-provider-kubevirt (capk) project needs a way for VMI's to be blocked from eviction, yet signal capk that eviction has been called on the VMI so the capk controller can handle tearing the VMI down.

Here's why.

When a VMI is being evicted, we need a way to have that eviction blocked so the cluster api related controllers can properly drain the k8s node running within the VMI before the VMI is torn down. We already have all the mechanisms in place in the cluster-api controllers to coordinate this, we just need a way to detect the VMI needs to go down (without actually taking the VMI down)

With `EvictionStrategy: External`, kubevirt will create a PDB for the VMI which blocks eviction, and it will also set the `vmi.Status.EvacuationNodeName` on the vmi's status. When the capk controllers see that `vmi.Status.EvacuationNodeName` we'll start the process of draining and tearing down the VMI gracefully from our side.

Q: why not use termination grace period and single drain internally when ACPI shutdown is detected?
A: We need to support the node drain process and timeouts that the cluster-api controllers execute today

Q: Should we be concerned that this feature could block node drain indefinitely?
A: Users can already create a PDB today for their VMIs to block node drain indefinitely so we're not doing anything here that a user couldn't achieve on their own. This feature primarily just adds a way to detect that eviction was called on the VMI (via vmi.Status.EvacuationNodeName)

```release-note
Adds new EvictionStrategy "External" for blocking eviction which is handled by an external controller
```

Signed-off-by: David Vossel <davidvossel@gmail.com>
  • Loading branch information
davidvossel committed Apr 7, 2022
1 parent 0db065c commit c1d77fa
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,26 @@ func (admitter *PodEvictionAdmitter) Admit(ar *admissionv1.AdmissionReview) *adm
if err != nil {
return denied(fmt.Sprintf("kubevirt failed getting the vmi: %s", err.Error()))
}
if !migrations.VMIMigratableOnEviction(admitter.ClusterConfig, vmi) {

evictionStrategy := migrations.VMIEvictionStrategy(admitter.ClusterConfig, vmi)
if evictionStrategy == nil {
// we don't act on VMIs without an eviction strategy
return validating_webhooks.NewPassingAdmissionResponse()
} else if !vmi.IsMigratable() {
return denied(fmt.Sprintf(
"VMI %s is configured with an eviction strategy but is not live-migratable", vmi.Name))
}

if !vmi.IsMarkedForEviction() && vmi.Status.NodeName == launcher.Spec.NodeName {
markForEviction := false

switch *evictionStrategy {
case virtv1.EvictionStrategyLiveMigrate:
if !vmi.IsMigratable() {
return denied(fmt.Sprintf("VMI %s is configured with an eviction strategy but is not live-migratable", vmi.Name))
}
markForEviction = true
case virtv1.EvictionStrategyExternal:
markForEviction = true
}

if markForEviction && !vmi.IsMarkedForEviction() && vmi.Status.NodeName == launcher.Spec.NodeName {
dryRun := ar.Request.DryRun != nil && *ar.Request.DryRun == true
err := admitter.markVMI(ar, vmi.Name, vmi.Status.NodeName, dryRun)
if err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -712,29 +712,34 @@ func podNetworkRequiredStatusCause(field *k8sfield.Path) metav1.StatusCause {
}
}

func isValidEvictionStrategy(evictionStrategy *v1.EvictionStrategy) bool {
return evictionStrategy == nil ||
*evictionStrategy == v1.EvictionStrategyLiveMigrate ||
*evictionStrategy == v1.EvictionStrategyNone ||
*evictionStrategy == v1.EvictionStrategyExternal

}

func validateLiveMigration(field *k8sfield.Path, spec *v1.VirtualMachineInstanceSpec, config *virtconfig.ClusterConfig) (causes []metav1.StatusCause) {
evictionStrategy := config.GetConfig().EvictionStrategy

if spec.EvictionStrategy != nil {
evictionStrategy = spec.EvictionStrategy
}
if !config.LiveMigrationEnabled() && evictionStrategy != nil &&
*evictionStrategy != v1.EvictionStrategyNone {
if !config.LiveMigrationEnabled() &&
evictionStrategy != nil &&
*evictionStrategy == v1.EvictionStrategyLiveMigrate {
causes = append(causes, metav1.StatusCause{
Type: metav1.CauseTypeFieldValueInvalid,
Message: "LiveMigration feature gate is not enabled",
Field: field.Child("evictionStrategy").String(),
})
} else if evictionStrategy != nil {
if *evictionStrategy != v1.EvictionStrategyLiveMigrate &&
*evictionStrategy != v1.EvictionStrategyNone {
causes = append(causes, metav1.StatusCause{
Type: metav1.CauseTypeFieldValueInvalid,
Message: fmt.Sprintf("%s is set with an unrecognized option: %s", field.Child("evictionStrategy").String(), *spec.EvictionStrategy),
Field: field.Child("evictionStrategy").String(),
})
}

} else if !isValidEvictionStrategy(evictionStrategy) {
causes = append(causes, metav1.StatusCause{
Type: metav1.CauseTypeFieldValueInvalid,
Message: fmt.Sprintf("%s is set with an unrecognized option: %s", field.Child("evictionStrategy").String(), *spec.EvictionStrategy),
Field: field.Child("evictionStrategy").String(),
})
}
return causes
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -502,17 +502,17 @@ func isPDBFromOldVMI(vmi *virtv1.VirtualMachineInstance, pdb *policyv1.PodDisrup
}

func (c *DisruptionBudgetController) sync(key string, vmiExists bool, vmi *virtv1.VirtualMachineInstance, pdb *policyv1.PodDisruptionBudget) error {
migratableOnDrain := c.vmiMigratableOnDrain(vmiExists, vmi)
needsEvictionProtection := c.vmiNeedsEvictionPDB(vmiExists, vmi)

// check for deletions if pod exists
if pdb != nil {
if !vmiExists || vmi.DeletionTimestamp != nil {
// being deleted
log.Log.Infof("deleting pdb %s/%s due to VMI deletion", pdb.Namespace, pdb.Name)
return c.deletePDB(key, pdb, vmi)
} else if !migratableOnDrain {
// vmi isn't set to migrate on eviction, so delete.
log.Log.Object(vmi).Infof("deleting pdb %s/%s due to not using evictionStrategy: LiveMigration", pdb.Namespace, pdb.Name)
} else if !needsEvictionProtection {
// vmi isn't set to prevent eviction, so delete the pdb
log.Log.Object(vmi).Infof("deleting pdb %s/%s due to not using evictionStrategy: LiveMigration|External", pdb.Namespace, pdb.Name)
return c.deletePDB(key, pdb, vmi)
} else if isPDBFromOldVMI(vmi, pdb) {
// pdb for non existent vmi
Expand All @@ -533,18 +533,29 @@ func (c *DisruptionBudgetController) sync(key string, vmiExists bool, vmi *virtv
return c.shrinkPDB(vmi, pdb)
}
}
} else if migratableOnDrain {
// pdb doesn't exist, create if vmi is set to migrate during drain.
} else if needsEvictionProtection {
// pdb doesn't exist, create if vmi's eviction strategy means it is protected during drain.
log.Log.Object(vmi).Infof("creating pdb for VMI %s/%s", vmi.Namespace, vmi.Name)
return c.createPDB(key, vmi)
}

return nil
}

func (c *DisruptionBudgetController) vmiMigratableOnDrain(vmiExists bool, vmi *virtv1.VirtualMachineInstance) bool {
func (c *DisruptionBudgetController) vmiNeedsEvictionPDB(vmiExists bool, vmi *virtv1.VirtualMachineInstance) bool {
if !vmiExists || vmi.DeletionTimestamp != nil {
return false
}
return migrations.VMIMigratableOnEviction(c.clusterConfig, vmi)

evictionStrategy := migrations.VMIEvictionStrategy(c.clusterConfig, vmi)
if evictionStrategy == nil {
return false
}

switch *evictionStrategy {
case virtv1.EvictionStrategyLiveMigrate, virtv1.EvictionStrategyExternal:
return true
default:
return false
}
}
1 change: 1 addition & 0 deletions staging/src/kubevirt.io/api/core/v1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -1810,6 +1810,7 @@ const (
const (
EvictionStrategyNone EvictionStrategy = "None"
EvictionStrategyLiveMigrate EvictionStrategy = "LiveMigrate"
EvictionStrategyExternal EvictionStrategy = "External"
)

// RestartOptions may be provided when deleting an API object.
Expand Down

0 comments on commit c1d77fa

Please sign in to comment.