Skip to content

Commit 3fd0121

Browse files
⚠️ Stop using deprecated replica counters in controllers (#12149)
* Stop using deprecated counters in controllers * Address comments * Address more comments
1 parent 1fc976f commit 3fd0121

34 files changed

+322
-533
lines changed

docs/book/src/developer/providers/migrations/v1.10-to-v1.11.md

+22-5
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,10 @@ proposal because most of the changes described below are a consequence of the wo
1616
- v1beta2 API version has been introduced; notable changes are:
1717
- The transition to the new K8s aligned conditions using `metav1.Conditions` types and the new condition semantic
1818
has been completed for all Kinds:
19-
- `status.conditions` has been replaced with `status.v1beta2.conditions`
20-
- the old `status.conditions` will continue to exist temporarily under `status.deprecated.v1beta1` for the sake of
21-
down conversions and to provide a temporary option for users willing to continue using old conditions.
19+
- `status.conditions` has been replaced with `status.v1beta2.conditions` based on metav1 condition types
20+
- the old `status.conditions` based on custom cluster API condition types will continue to exist temporarily
21+
under `status.deprecated.v1beta1.conditions` for the sake of down conversions and to provide a temporary option
22+
for users willing to continue using old conditions.
2223
- Support for terminal errors has been dropped from all Kinds
2324
- `status.failureReason` and `status.failureMessage` will continue to exist temporarily under `status.deprecated.v1beta1`
2425
for the sake of down conversions and to provide a temporary option for users willing to continue using old conditions.
@@ -27,7 +28,16 @@ proposal because most of the changes described below are a consequence of the wo
2728
- All the resources handling machine replicas have now a consistent set of replica counters based on corresponding
2829
conditions defined at machine level.
2930
- `status.readyReplicas`, `status.availableReplicas`, `status.upToDateReplicas` on `MachineDeployments`, `MachineSet`
30-
and `KubeadmControlPlane`
31+
and `KubeadmControlPlane`; please note that
32+
- `status.readyReplicas` has now a new semantic based on machine's `Ready` condition
33+
- `status.availableReplicas` has now a new semantic based on machine's `Available` condition
34+
- `status.upToDateReplicas` has now a new semantic (and name) based on machine's `UpToDate` condition
35+
- Temporarily, old replica counters will still be available under the `status.deprecated.v1beta1` struct; more specifically
36+
- `status.deprecated.v1beta1.readyReplicas` with old semantic for `MachineDeployments`, `MachineSet` and `KubeadmControlPlane`
37+
- `status.deprecated.v1beta1.availableReplicas` with old semantic for `MachineDeployments`, `MachineSet`
38+
- `status.deprecated.v1beta1.unavailableReplicas` with old semantic for `MachineDeployments`, `KubeadmControlPlane`
39+
- `status.deprecated.v1beta1.updatedReplicas` with old semantic (and name) for `MachineDeployments`, `KubeadmControlPlane`
40+
- `status.deprecated.v1beta1.fullyLabeledReplicas` for `MachineSet`
3141
- The `Cluster` resource reports replica counters for both control plane and worker machines.
3242

3343
### Cluster API Contract changes
@@ -51,7 +61,14 @@ See [provider contracts](../contracts/overview.md) for more details.
5161
### Deprecation
5262

5363
- v1beta1 API version is deprecated and it will be removed tentatively in August 2026
54-
- All the fields under `status.deprecated.v1beta1` in the new v1beta2 API are deprecated and whey will be removed
64+
- All the fields under `status.deprecated.v1beta1` in the new v1beta2 API are deprecated and whey will be removed. This includes:
65+
- `status.deprecated.v1beta1.conditions` based on custom cluster API condition types
66+
- `status.deprecated.v1beta1.failureReason` and `status.failureMessage`
67+
- `status.deprecated.v1beta1.readyReplicas` with old semantic for `MachineDeployments`, `MachineSet` and `KubeadmControlPlane`
68+
- `status.deprecated.v1beta1.availableReplicas` with old semantic for `MachineDeployments`, `MachineSet`
69+
- `status.deprecated.v1beta1.unavailableReplicas` with old semantic for `MachineDeployments`, `KubeadmControlPlane`
70+
- `status.deprecated.v1beta1.updatedReplicas` with old semantic (and name) for `MachineDeployments`, `KubeadmControlPlane`
71+
- `status.deprecated.v1beta1.fullyLabeledReplicas` for `MachineSet`
5572
- v1beta1 conditions utils are now deprecated, and will removed as soon as v1beta1 API will be removed
5673
- v1beta1 support in the patch helper is now deprecated, and will removed as soon as v1beta1 API will be removed
5774

exp/runtime/hooks/api/v1alpha1/topologymutation_variable_types.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ type ControlPlaneBuiltins struct {
125125

126126
// replicas is the value of the replicas field of the ControlPlane object.
127127
// +optional
128-
Replicas *int64 `json:"replicas,omitempty"`
128+
Replicas *int32 `json:"replicas,omitempty"`
129129

130130
// machineTemplate is the value of the .spec.machineTemplate field of the ControlPlane object.
131131
// +optional

exp/runtime/hooks/api/v1alpha1/zz_generated.deepcopy.go

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

exp/runtime/hooks/api/v1alpha1/zz_generated.openapi.go

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

exp/topology/desiredstate/desired_state.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,7 @@ func (g *generator) computeControlPlane(ctx context.Context, s *scope.Scope, inf
354354
// NOTE: If the Topology.ControlPlane.replicas value is nil, it is assumed that the control plane controller
355355
// does not implement support for this field and the ControlPlane object is generated without the number of Replicas.
356356
if s.Blueprint.Topology.ControlPlane.Replicas != nil {
357-
if err := contract.ControlPlane().Replicas().Set(controlPlane, int64(*s.Blueprint.Topology.ControlPlane.Replicas)); err != nil {
357+
if err := contract.ControlPlane().Replicas().Set(controlPlane, *s.Blueprint.Topology.ControlPlane.Replicas); err != nil {
358358
return nil, errors.Wrapf(err, "failed to set %s in the ControlPlane object", contract.ControlPlane().Replicas().Path())
359359
}
360360
}

exp/topology/desiredstate/desired_state_test.go

+3-7
Original file line numberDiff line numberDiff line change
@@ -1859,13 +1859,9 @@ func TestComputeMachineDeployment(t *testing.T) {
18591859
WithStatus(clusterv1.MachineDeploymentStatus{
18601860
ObservedGeneration: 2,
18611861
Replicas: 2,
1862-
Deprecated: &clusterv1.MachineDeploymentDeprecatedStatus{
1863-
V1Beta1: &clusterv1.MachineDeploymentV1Beta1DeprecatedStatus{
1864-
ReadyReplicas: 2,
1865-
UpdatedReplicas: 2,
1866-
AvailableReplicas: 2,
1867-
},
1868-
},
1862+
ReadyReplicas: ptr.To[int32](2),
1863+
UpToDateReplicas: ptr.To[int32](2),
1864+
AvailableReplicas: ptr.To[int32](2),
18691865
}).
18701866
Build()
18711867
mdsState = duplicateMachineDeploymentsState(mdsState)

internal/contract/controlplane.go

+61-96
Original file line numberDiff line numberDiff line change
@@ -93,95 +93,49 @@ func (c *ControlPlaneContract) ControlPlaneEndpoint() *ControlPlaneEndpoint {
9393
// NOTE: When working with unstructured there is no way to understand if the ControlPlane provider
9494
// do support a field in the type definition from the fact that a field is not set in a given instance.
9595
// This is why in we are deriving if replicas is required from the ClusterClass in the topology reconciler code.
96-
func (c *ControlPlaneContract) Replicas() *Int64 {
97-
return &Int64{
96+
func (c *ControlPlaneContract) Replicas() *Int32 {
97+
return &Int32{
9898
path: []string{"spec", "replicas"},
9999
}
100100
}
101101

102102
// StatusReplicas provide access to the status.replicas field in a ControlPlane object, if any. Applies to implementations using replicas.
103-
func (c *ControlPlaneContract) StatusReplicas() *Int64 {
104-
return &Int64{
103+
func (c *ControlPlaneContract) StatusReplicas() *Int32 {
104+
return &Int32{
105105
path: []string{"status", "replicas"},
106106
}
107107
}
108108

109-
// UpdatedReplicas provide access to the status.updatedReplicas field in a ControlPlane object, if any. Applies to implementations using replicas.
110-
// TODO (v1beta2): Rename to V1Beta1DeprecatedUpdatedReplicas and make sure we are only using this method for compatibility with old contracts.
111-
func (c *ControlPlaneContract) UpdatedReplicas(contractVersion string) *Int64 {
112-
if contractVersion == "v1beta1" {
113-
return &Int64{
114-
path: []string{"status", "updatedReplicas"},
115-
}
116-
}
117-
118-
return &Int64{
119-
path: []string{"status", "deprecated", "v1beta1", "updatedReplicas"},
120-
}
121-
}
122-
123109
// ReadyReplicas provide access to the status.readyReplicas field in a ControlPlane object, if any. Applies to implementations using replicas.
124-
// TODO (v1beta2): Rename to V1Beta1DeprecatedReadyReplicas and make sure we are only using this method for compatibility with old contracts.
125-
func (c *ControlPlaneContract) ReadyReplicas(contractVersion string) *Int64 {
126-
if contractVersion == "v1beta1" {
127-
return &Int64{
128-
path: []string{"status", "readyReplicas"},
129-
}
130-
}
131-
132-
return &Int64{
133-
path: []string{"status", "deprecated", "v1beta1", "readyReplicas"},
134-
}
135-
}
136-
137-
// UnavailableReplicas provide access to the status.unavailableReplicas field in a ControlPlane object, if any. Applies to implementations using replicas.
138-
// TODO (v1beta2): Rename to V1Beta1DeprecatedUnavailableReplicas and make sure we are only using this method for compatibility with old contracts.
139-
func (c *ControlPlaneContract) UnavailableReplicas(contractVersion string) *Int64 {
140-
if contractVersion == "v1beta1" {
141-
return &Int64{
142-
path: []string{"status", "unavailableReplicas"},
143-
}
144-
}
145-
146-
return &Int64{
147-
path: []string{"status", "deprecated", "v1beta1", "unavailableReplicas"},
148-
}
149-
}
150-
151-
// V1Beta2ReadyReplicas provide access to the status.readyReplicas field in a ControlPlane object, if any. Applies to implementations using replicas.
152-
// TODO (v1beta2): Drop V1Beta2 prefix..
153-
func (c *ControlPlaneContract) V1Beta2ReadyReplicas(contractVersion string) *Int32 {
154-
if contractVersion == "v1beta1" {
155-
return &Int32{
156-
path: []string{"status", "v1beta2", "readyReplicas"},
157-
}
158-
}
159-
110+
// NOTE: readyReplicas changed semantic in v1beta2 contract.
111+
func (c *ControlPlaneContract) ReadyReplicas() *Int32 {
160112
return &Int32{
161113
path: []string{"status", "readyReplicas"},
162114
}
163115
}
164116

165-
// V1Beta2AvailableReplicas provide access to the status.availableReplicas field in a ControlPlane object, if any. Applies to implementations using replicas.
166-
// TODO (v1beta2): Drop V1Beta2 prefix.x.
167-
func (c *ControlPlaneContract) V1Beta2AvailableReplicas(contractVersion string) *Int32 {
168-
if contractVersion == "v1beta1" {
169-
return &Int32{
170-
path: []string{"status", "v1beta2", "availableReplicas"},
171-
}
172-
}
173-
117+
// AvailableReplicas provide access to the status.availableReplicas field in a ControlPlane object, if any. Applies to implementations using replicas.
118+
// NOTE: availableReplicas was introduced by the v1beta2 contract; use unavailableReplicas for the v1beta1 contract.
119+
func (c *ControlPlaneContract) AvailableReplicas() *Int32 {
174120
return &Int32{
175121
path: []string{"status", "availableReplicas"},
176122
}
177123
}
178124

179-
// V1Beta2UpToDateReplicas provide access to the status.upToDateReplicas field in a ControlPlane object, if any. Applies to implementations using replicas.
180-
// TODO (v1beta2): Drop V1Beta2 prefix.ix.
181-
func (c *ControlPlaneContract) V1Beta2UpToDateReplicas(contractVersion string) *Int32 {
125+
// V1Beta1UnavailableReplicas provide access to the status.unavailableReplicas field in a ControlPlane object, if any. Applies to implementations using replicas.
126+
// NOTE: use availableReplicas when working with the v1beta2 contract.
127+
func (c *ControlPlaneContract) V1Beta1UnavailableReplicas() *Int64 {
128+
return &Int64{
129+
path: []string{"status", "unavailableReplicas"},
130+
}
131+
}
132+
133+
// UpToDateReplicas provide access to the status.upToDateReplicas field in a ControlPlane object, if any. Applies to implementations using replicas.
134+
// NOTE: upToDateReplicas was introduced by the v1beta2 contract; code will fall back to updatedReplicas for the v1beta1 contract.
135+
func (c *ControlPlaneContract) UpToDateReplicas(contractVersion string) *Int32 {
182136
if contractVersion == "v1beta1" {
183137
return &Int32{
184-
path: []string{"status", "v1beta2", "upToDateReplicas"},
138+
path: []string{"status", "updatedReplicas"},
185139
}
186140
}
187141

@@ -285,19 +239,16 @@ func (c *ControlPlaneContract) IsUpgrading(obj *unstructured.Unstructured) (bool
285239
// A control plane is considered scaling if:
286240
// - status.replicas is not yet set.
287241
// - spec.replicas != status.replicas.
288-
// - spec.replicas != status.updatedReplicas.
242+
// - spec.replicas != status.upToDateReplicas.
289243
// - spec.replicas != status.readyReplicas.
290-
// - status.unavailableReplicas > 0.
244+
// - spec.replicas != status.availableReplicas.
245+
// NOTE: this function is used only in E2E tests.
291246
func (c *ControlPlaneContract) IsScaling(obj *unstructured.Unstructured, contractVersion string) (bool, error) {
292247
desiredReplicas, err := c.Replicas().Get(obj)
293248
if err != nil {
294-
return false, errors.Wrap(err, "failed to get control plane spec replicas")
249+
return false, errors.Wrapf(err, "failed to get control plane %s", c.Replicas().Path().String())
295250
}
296251

297-
// TODO (v1beta2): Add a new code path using v1beta2 replica counters
298-
// note: currently we are still always using v1beta1 counters no matter if they are moved under deprecated
299-
// but we should stop doing this ASAP
300-
301252
statusReplicas, err := c.StatusReplicas().Get(obj)
302253
if err != nil {
303254
if errors.Is(err, ErrFieldNotFound) {
@@ -306,54 +257,68 @@ func (c *ControlPlaneContract) IsScaling(obj *unstructured.Unstructured, contrac
306257
// so that we can block any operations that expect control plane to be stable.
307258
return true, nil
308259
}
309-
return false, errors.Wrap(err, "failed to get control plane status replicas")
260+
return false, errors.Wrapf(err, "failed to get control plane %s", c.StatusReplicas().Path().String())
310261
}
311262

312-
updatedReplicas, err := c.UpdatedReplicas(contractVersion).Get(obj)
263+
upToDateReplicas, err := c.UpToDateReplicas(contractVersion).Get(obj)
313264
if err != nil {
314265
if errors.Is(err, ErrFieldNotFound) {
315266
// If updatedReplicas is not set on the control plane
316267
// we should consider the control plane to be scaling so that
317268
// we block any operation that expect the control plane to be stable.
318269
return true, nil
319270
}
320-
return false, errors.Wrap(err, "failed to get control plane status updatedReplicas")
271+
return false, errors.Wrapf(err, "failed to get control plane %s", c.UpToDateReplicas(contractVersion).Path().String())
321272
}
322273

323-
readyReplicas, err := c.ReadyReplicas(contractVersion).Get(obj)
274+
readyReplicas, err := c.ReadyReplicas().Get(obj)
324275
if err != nil {
325276
if errors.Is(err, ErrFieldNotFound) {
326277
// If readyReplicas is not set on the control plane
327278
// we should consider the control plane to be scaling so that
328279
// we block any operation that expect the control plane to be stable.
329280
return true, nil
330281
}
331-
return false, errors.Wrap(err, "failed to get control plane status readyReplicas")
282+
return false, errors.Wrapf(err, "failed to get control plane %s", c.ReadyReplicas().Path().String())
332283
}
333284

334-
unavailableReplicas, err := c.UnavailableReplicas(contractVersion).Get(obj)
335-
if err != nil {
336-
if !errors.Is(err, ErrFieldNotFound) {
337-
return false, errors.Wrap(err, "failed to get control plane status unavailableReplicas")
285+
var availableReplicas *int32
286+
if contractVersion == "v1beta1" {
287+
unavailableReplicas, err := c.V1Beta1UnavailableReplicas().Get(obj)
288+
if err != nil {
289+
if !errors.Is(err, ErrFieldNotFound) {
290+
return false, errors.Wrapf(err, "failed to get control plane %s", c.V1Beta1UnavailableReplicas().Path().String())
291+
}
292+
// If unavailableReplicas is not set on the control plane we assume it is 0.
293+
// We have to do this as the following happens after clusterctl move with KCP:
294+
// * clusterctl move creates the KCP object without status
295+
// * the KCP controller won't patch the field to 0 if it doesn't exist
296+
// * This is because the patchHelper marshals before/after object to JSON to calculate a diff
297+
// and as the unavailableReplicas field is not a pointer, not set and 0 are both rendered as 0.
298+
// If before/after of the field is the same (i.e. 0), there is no diff and thus also no patch to set it to 0.
299+
unavailableReplicas = ptr.To[int64](0)
300+
}
301+
availableReplicas = ptr.To(*desiredReplicas - int32(*unavailableReplicas))
302+
} else {
303+
availableReplicas, err = c.AvailableReplicas().Get(obj)
304+
if err != nil {
305+
if errors.Is(err, ErrFieldNotFound) {
306+
// If availableReplicas is not set on the control plane
307+
// we should consider the control plane to be scaling so that
308+
// we block any operation that expect the control plane to be stable.
309+
return true, nil
310+
}
311+
return false, errors.Wrapf(err, "failed to get control plane %s", c.AvailableReplicas().Path().String())
338312
}
339-
// If unavailableReplicas is not set on the control plane we assume it is 0.
340-
// We have to do this as the following happens after clusterctl move with KCP:
341-
// * clusterctl move creates the KCP object without status
342-
// * the KCP controller won't patch the field to 0 if it doesn't exist
343-
// * This is because the patchHelper marshals before/after object to JSON to calculate a diff
344-
// and as the unavailableReplicas field is not a pointer, not set and 0 are both rendered as 0.
345-
// If before/after of the field is the same (i.e. 0), there is no diff and thus also no patch to set it to 0.
346-
unavailableReplicas = ptr.To[int64](0)
347313
}
348314

349315
// Control plane is still scaling if:
350-
// * .spec.replicas, .status.replicas, .status.updatedReplicas,
351-
// .status.readyReplicas are not equal and
352-
// * unavailableReplicas > 0
316+
// * .spec.replicas, .status.replicas, .status.upToDateReplicas,
317+
// .status.readyReplicas, .status.availableReplicas are not equal.
353318
if *statusReplicas != *desiredReplicas ||
354-
*updatedReplicas != *desiredReplicas ||
319+
*upToDateReplicas != *desiredReplicas ||
355320
*readyReplicas != *desiredReplicas ||
356-
*unavailableReplicas > 0 {
321+
*availableReplicas != *desiredReplicas {
357322
return true, nil
358323
}
359324
return false, nil

0 commit comments

Comments
 (0)