Skip to content

✨Introduce NodeDeletionStrategy to allow drain node when deleting cluster #12142

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions api/v1beta1/conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -573,3 +573,11 @@ func Convert_v1beta1_Condition_To_v1_Condition(_ *Condition, _ *metav1.Condition
// NOTE: legacy (v1beta1) conditions should not be automatically converted into v1beta2 conditions.
return nil
}

func Convert_v1beta2_Topology_To_v1beta1_Topology(in *clusterv1.Topology, out *Topology, s apimachineryconversion.Scope) error {
if err := autoConvert_v1beta2_Topology_To_v1beta1_Topology(in, out, s); err != nil {
return err
}

return nil
}
26 changes: 19 additions & 7 deletions api/v1beta1/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 19 additions & 0 deletions api/v1beta2/cluster_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -585,6 +585,15 @@ type Topology struct {
// +listMapKey=name
// +kubebuilder:validation:MaxItems=1000
Variables []ClusterVariable `json:"variables,omitempty"`

// nodeDeletionStrategy specifies the strategy to delete nodes in the cluster.
// Valid values are Force, Graceful and omitted.
// When omitted, the default behaviour will be Force.
// Graceful means that nodes will be deleted with drain.
// Force means that nodes will be deleted immediately without drain.
// +optional
// +kubebuilder:validation:Enum=force;graceful
NodeDeletionStrategy NodeDeletionStrategyType `json:"nodeDeletionStrategy,omitempty"`
}

// ControlPlaneTopology specifies the parameters for the control plane nodes in the cluster.
Expand Down Expand Up @@ -901,6 +910,16 @@ type MachinePoolVariables struct {
Overrides []ClusterVariable `json:"overrides,omitempty"`
}

// NodeDeletionStrategyType defines type of NodeDeletionStrategy.
type NodeDeletionStrategyType string

const (
// NodeDeletionStrategyForce defines a force type strategy that node will be deleted immediately without drain.
NodeDeletionStrategyForce NodeDeletionStrategyType = "Force"
// NodeDeletionStrategyGraceful defines a graceful type strategy that node will be deleted with drain.
NodeDeletionStrategyGraceful NodeDeletionStrategyType = "Graceful"
)

// ANCHOR_END: ClusterSpec

// ANCHOR: ClusterNetwork
Expand Down
7 changes: 7 additions & 0 deletions api/v1beta2/zz_generated.openapi.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 11 additions & 0 deletions config/crd/bases/cluster.x-k8s.io_clusters.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions internal/apis/core/v1alpha4/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions internal/controllers/machine/machine_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -714,8 +714,8 @@ func (r *Reconciler) nodeVolumeDetachTimeoutExceeded(machine *clusterv1.Machine)
// and if the Machine is not the last control plane node in the cluster.
func (r *Reconciler) isDeleteNodeAllowed(ctx context.Context, cluster *clusterv1.Cluster, machine *clusterv1.Machine) error {
log := ctrl.LoggerFrom(ctx)
// Return early if the cluster is being deleted.
if !cluster.DeletionTimestamp.IsZero() {
// Return early if the cluster is being deleted and cluster's nodeDeletionStrategy is not set or set to `force`.
if !cluster.DeletionTimestamp.IsZero() && (cluster.Spec.Topology == nil || cluster.Spec.Topology.NodeDeletionStrategy != clusterv1.NodeDeletionStrategyGraceful) {
return errClusterIsBeingDeleted
}

Expand Down
56 changes: 56 additions & 0 deletions internal/controllers/machine/machine_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2664,10 +2664,66 @@ func TestIsDeleteNodeAllowed(t *testing.T) {
DeletionTimestamp: &deletionts,
Finalizers: []string{clusterv1.ClusterFinalizer},
},
Spec: clusterv1.ClusterSpec{
Topology: &clusterv1.Topology{},
},
},
machine: &clusterv1.Machine{},
expectedError: errClusterIsBeingDeleted,
},
{
name: "has nodeRef and cluster is being deleted, nodeDeletionStrategy set to force",
cluster: &clusterv1.Cluster{
ObjectMeta: metav1.ObjectMeta{
Name: "test-cluster",
Namespace: metav1.NamespaceDefault,
DeletionTimestamp: &deletionts,
Finalizers: []string{clusterv1.ClusterFinalizer},
},
Spec: clusterv1.ClusterSpec{
Topology: &clusterv1.Topology{
NodeDeletionStrategy: clusterv1.NodeDeletionStrategyForce,
},
},
},
machine: &clusterv1.Machine{},
expectedError: errClusterIsBeingDeleted,
},
{
name: "has nodeRef and control plane is healthy, with nodeDeletionStrategy set to graceful",
cluster: &clusterv1.Cluster{
ObjectMeta: metav1.ObjectMeta{
Name: "test-cluster",
Namespace: metav1.NamespaceDefault,
},
Spec: clusterv1.ClusterSpec{
Topology: &clusterv1.Topology{
NodeDeletionStrategy: clusterv1.NodeDeletionStrategyForce,
},
},
},
machine: &clusterv1.Machine{
ObjectMeta: metav1.ObjectMeta{
Name: "created",
Namespace: metav1.NamespaceDefault,
Labels: map[string]string{
clusterv1.ClusterNameLabel: "test-cluster",
},
Finalizers: []string{clusterv1.MachineFinalizer},
},
Spec: clusterv1.MachineSpec{
ClusterName: "test-cluster",
InfrastructureRef: corev1.ObjectReference{},
Bootstrap: clusterv1.Bootstrap{DataSecretName: ptr.To("data")},
},
Status: clusterv1.MachineStatus{
NodeRef: &corev1.ObjectReference{
Name: "test",
},
},
},
expectedError: nil,
},
{
name: "has nodeRef and control plane is healthy and externally managed",
cluster: &clusterv1.Cluster{
Expand Down