Skip to content

Commit 67080e1

Browse files
committed
Add topology field for NodeDeletionTimeout
Revert "Add topology field for NodeDeletionTimeout" This reverts commit 419cdc6. Add topology field for NodeDeletionTimeout #7031 Add topology field for NodeDeletionTimeout #7031
1 parent 0017077 commit 67080e1

File tree

12 files changed

+113
-13
lines changed

12 files changed

+113
-13
lines changed

api/v1alpha4/conversion.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,10 @@ func (src *Cluster) ConvertTo(dstRaw conversion.Hub) error {
4747
dst.Spec.Topology.ControlPlane.NodeDrainTimeout = restored.Spec.Topology.ControlPlane.NodeDrainTimeout
4848
}
4949

50+
if restored.Spec.Topology.ControlPlane.NodeDeletionTimeout != nil {
51+
dst.Spec.Topology.ControlPlane.NodeDeletionTimeout = restored.Spec.Topology.ControlPlane.NodeDeletionTimeout
52+
}
53+
5054
if restored.Spec.Topology.Workers != nil {
5155
if dst.Spec.Topology.Workers == nil {
5256
dst.Spec.Topology.Workers = &clusterv1.WorkersTopology{}
@@ -55,6 +59,7 @@ func (src *Cluster) ConvertTo(dstRaw conversion.Hub) error {
5559
dst.Spec.Topology.Workers.MachineDeployments[i].FailureDomain = restored.Spec.Topology.Workers.MachineDeployments[i].FailureDomain
5660
dst.Spec.Topology.Workers.MachineDeployments[i].Variables = restored.Spec.Topology.Workers.MachineDeployments[i].Variables
5761
dst.Spec.Topology.Workers.MachineDeployments[i].NodeDrainTimeout = restored.Spec.Topology.Workers.MachineDeployments[i].NodeDrainTimeout
62+
dst.Spec.Topology.Workers.MachineDeployments[i].NodeDeletionTimeout = restored.Spec.Topology.Workers.MachineDeployments[i].NodeDeletionTimeout
5863
}
5964
}
6065
}

api/v1alpha4/zz_generated.conversion.go

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

api/v1beta1/cluster_types.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,12 @@ type ControlPlaneTopology struct {
121121
// NOTE: NodeDrainTimeout is different from `kubectl drain --timeout`
122122
// +optional
123123
NodeDrainTimeout *metav1.Duration `json:"nodeDrainTimeout,omitempty"`
124+
125+
// NodeDeletionTimeout defines how long the controller will attempt to delete the Node that the Machine
126+
// hosts after the Machine is marked for deletion. A duration of 0 will retry deletion indefinitely.
127+
// Defaults to 10 seconds.
128+
// +optional
129+
NodeDeletionTimeout *metav1.Duration `json:"nodeDeletionTimeout,omitempty"`
124130
}
125131

126132
// WorkersTopology represents the different sets of worker nodes in the cluster.
@@ -167,6 +173,12 @@ type MachineDeploymentTopology struct {
167173
// +optional
168174
NodeDrainTimeout *metav1.Duration `json:"nodeDrainTimeout,omitempty"`
169175

176+
// NodeDeletionTimeout defines how long the controller will attempt to delete the Node that the Machine
177+
// hosts after the Machine is marked for deletion. A duration of 0 will retry deletion indefinitely.
178+
// Defaults to 10 seconds.
179+
// +optional
180+
NodeDeletionTimeout *metav1.Duration `json:"nodeDeletionTimeout,omitempty"`
181+
170182
// Variables can be used to customize the MachineDeployment through patches.
171183
// +optional
172184
Variables *MachineDeploymentVariables `json:"variables,omitempty"`

api/v1beta1/zz_generated.deepcopy.go

Lines changed: 10 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

api/v1beta1/zz_generated.openapi.go

Lines changed: 12 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

config/crd/bases/cluster.x-k8s.io_clusters.yaml

Lines changed: 13 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

internal/contract/controlplane.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,3 +226,10 @@ func (c *ControlPlaneMachineTemplate) NodeDrainTimeout() *Duration {
226226
path: Path{"spec", "machineTemplate", "nodeDrainTimeout"},
227227
}
228228
}
229+
230+
// NodeDeletionTimeout provides access to the nodeDeletionTimeout of a MachineTemplate.
231+
func (c *ControlPlaneMachineTemplate) NodeDeletionTimeout() *Duration {
232+
return &Duration{
233+
path: Path{"spec", "machineTemplate", "nodeDeletionTimeout"},
234+
}
235+
}

internal/contract/controlplane_test.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,28 @@ func TestControlPlane(t *testing.T) {
170170
g.Expect(found).To(BeTrue())
171171
g.Expect(durationString).To(Equal(expectedDurationString))
172172
})
173+
174+
t.Run("Manages spec.machineTemplate.nodeDeletionTimeout", func(t *testing.T) {
175+
g := NewWithT(t)
176+
177+
duration := metav1.Duration{Duration: 2*time.Minute + 5*time.Second}
178+
expectedDurationString := "2m5s"
179+
g.Expect(ControlPlane().MachineTemplate().NodeDeletionTimeout().Path()).To(Equal(Path{"spec", "machineTemplate", "nodeDeletionTimeout"}))
180+
181+
err := ControlPlane().MachineTemplate().NodeDeletionTimeout().Set(obj, duration)
182+
g.Expect(err).ToNot(HaveOccurred())
183+
184+
got, err := ControlPlane().MachineTemplate().NodeDeletionTimeout().Get(obj)
185+
g.Expect(err).ToNot(HaveOccurred())
186+
g.Expect(got).ToNot(BeNil())
187+
g.Expect(*got).To(Equal(duration))
188+
189+
// Check that the literal string value of the duration is correctly formatted.
190+
durationString, found, err := unstructured.NestedString(obj.UnstructuredContent(), "spec", "machineTemplate", "nodeDeletionTimeout")
191+
g.Expect(err).ToNot(HaveOccurred())
192+
g.Expect(found).To(BeTrue())
193+
g.Expect(durationString).To(Equal(expectedDurationString))
194+
})
173195
}
174196

175197
func TestControlPlaneIsUpgrading(t *testing.T) {

internal/controllers/topology/cluster/desired_state.go

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,13 @@ func (r *Reconciler) computeControlPlane(ctx context.Context, s *scope.Scope, in
242242
}
243243
}
244244

245+
// If it is required to manage the NodeDeletionTimeout for the control plane, set the corresponding field.
246+
if s.Blueprint.Topology.ControlPlane.NodeDeletionTimeout != nil {
247+
if err := contract.ControlPlane().MachineTemplate().NodeDeletionTimeout().Set(controlPlane, *s.Blueprint.Topology.ControlPlane.NodeDeletionTimeout); err != nil {
248+
return nil, errors.Wrap(err, "failed to set spec.machineTemplate.nodeDeletionTimeout in the ControlPlane object")
249+
}
250+
}
251+
245252
// Sets the desired Kubernetes version for the control plane.
246253
version, err := r.computeControlPlaneVersion(ctx, s)
247254
if err != nil {
@@ -531,12 +538,13 @@ func computeMachineDeployment(_ context.Context, s *scope.Scope, desiredControlP
531538
Annotations: mergeMap(machineDeploymentTopology.Metadata.Annotations, machineDeploymentBlueprint.Metadata.Annotations),
532539
},
533540
Spec: clusterv1.MachineSpec{
534-
ClusterName: s.Current.Cluster.Name,
535-
Version: pointer.String(version),
536-
Bootstrap: clusterv1.Bootstrap{ConfigRef: contract.ObjToRef(desiredMachineDeployment.BootstrapTemplate)},
537-
InfrastructureRef: *contract.ObjToRef(desiredMachineDeployment.InfrastructureMachineTemplate),
538-
FailureDomain: machineDeploymentTopology.FailureDomain,
539-
NodeDrainTimeout: machineDeploymentTopology.NodeDrainTimeout,
541+
ClusterName: s.Current.Cluster.Name,
542+
Version: pointer.String(version),
543+
Bootstrap: clusterv1.Bootstrap{ConfigRef: contract.ObjToRef(desiredMachineDeployment.BootstrapTemplate)},
544+
InfrastructureRef: *contract.ObjToRef(desiredMachineDeployment.InfrastructureMachineTemplate),
545+
FailureDomain: machineDeploymentTopology.FailureDomain,
546+
NodeDrainTimeout: machineDeploymentTopology.NodeDrainTimeout,
547+
NodeDeletionTimeout: machineDeploymentTopology.NodeDeletionTimeout,
540548
},
541549
},
542550
},

internal/controllers/topology/cluster/desired_state_test.go

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,7 @@ func TestComputeControlPlane(t *testing.T) {
263263
replicas := int32(3)
264264
duration := 10 * time.Second
265265
nodeDrainTimeout := metav1.Duration{Duration: duration}
266+
nodeDeletionTimeout := metav1.Duration{Duration: duration}
266267
cluster := &clusterv1.Cluster{
267268
ObjectMeta: metav1.ObjectMeta{
268269
Name: "cluster1",
@@ -276,8 +277,9 @@ func TestComputeControlPlane(t *testing.T) {
276277
Labels: map[string]string{"l2": ""},
277278
Annotations: map[string]string{"a2": ""},
278279
},
279-
Replicas: &replicas,
280-
NodeDrainTimeout: &nodeDrainTimeout,
280+
Replicas: &replicas,
281+
NodeDrainTimeout: &nodeDrainTimeout,
282+
NodeDeletionTimeout: &nodeDeletionTimeout,
281283
},
282284
},
283285
},
@@ -315,6 +317,7 @@ func TestComputeControlPlane(t *testing.T) {
315317
assertNestedField(g, obj, version, contract.ControlPlane().Version().Path()...)
316318
assertNestedField(g, obj, int64(replicas), contract.ControlPlane().Replicas().Path()...)
317319
assertNestedField(g, obj, duration.String(), contract.ControlPlane().MachineTemplate().NodeDrainTimeout().Path()...)
320+
assertNestedField(g, obj, duration.String(), contract.ControlPlane().MachineTemplate().NodeDeletionTimeout().Path()...)
318321
assertNestedFieldUnset(g, obj, contract.ControlPlane().MachineTemplate().InfrastructureRef().Path()...)
319322

320323
// Ensure no ownership is added to generated ControlPlane.
@@ -1318,15 +1321,17 @@ func TestComputeMachineDeployment(t *testing.T) {
13181321
replicas := int32(5)
13191322
failureDomain := "always-up-region"
13201323
nodeDrainTimeout := metav1.Duration{Duration: 10 * time.Second}
1324+
nodeDeletionTimeout := metav1.Duration{Duration: 10 * time.Second}
13211325
mdTopology := clusterv1.MachineDeploymentTopology{
13221326
Metadata: clusterv1.ObjectMeta{
13231327
Labels: map[string]string{"foo": "baz"},
13241328
},
1325-
Class: "linux-worker",
1326-
Name: "big-pool-of-machines",
1327-
Replicas: &replicas,
1328-
FailureDomain: &failureDomain,
1329-
NodeDrainTimeout: &nodeDrainTimeout,
1329+
Class: "linux-worker",
1330+
Name: "big-pool-of-machines",
1331+
Replicas: &replicas,
1332+
FailureDomain: &failureDomain,
1333+
NodeDrainTimeout: &nodeDrainTimeout,
1334+
NodeDeletionTimeout: &nodeDeletionTimeout,
13301335
}
13311336

13321337
t.Run("Generates the machine deployment and the referenced templates", func(t *testing.T) {
@@ -1355,6 +1360,7 @@ func TestComputeMachineDeployment(t *testing.T) {
13551360
g.Expect(*actualMd.Spec.Replicas).To(Equal(replicas))
13561361
g.Expect(*actualMd.Spec.Template.Spec.FailureDomain).To(Equal(failureDomain))
13571362
g.Expect(*actualMd.Spec.Template.Spec.NodeDrainTimeout).To(Equal(nodeDrainTimeout))
1363+
g.Expect(*actualMd.Spec.Template.Spec.NodeDeletionTimeout).To(Equal(nodeDeletionTimeout))
13581364
g.Expect(actualMd.Spec.ClusterName).To(Equal("cluster1"))
13591365
g.Expect(actualMd.Name).To(ContainSubstring("cluster1"))
13601366
g.Expect(actualMd.Name).To(ContainSubstring("big-pool-of-machines"))

0 commit comments

Comments
 (0)