Skip to content

Commit 8adaf28

Browse files
committed
fix(hcco): OCPBUGS-88738 clean up orphaned mirrored CMs on NodePool deletion
The guard added in PR #8672 unconditionally skips deletion of guest-side ConfigMaps with NTOMirroredConfigLabel, preventing spurious MCO rollouts when the source CM is transiently absent. However, this also preserves CMs whose owning NodePool has been permanently deleted. Derive NodePool existence from the wantCMList already fetched from the HCP namespace: when a NodePool is deleted, its finalizer removes all its CMs, so zero CMs for a given NodePool means it has been deleted. Build an activeNodePools set and only skip deletion when the owning NodePool is still active. Signed-off-by: Vimal Solanki <vsolanki@redhat.com>
1 parent ca3d347 commit 8adaf28

2 files changed

Lines changed: 80 additions & 11 deletions

File tree

control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources.go

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3016,6 +3016,20 @@ func (r *reconciler) reconcileKubeletConfig(ctx context.Context) error {
30163016
for _, cm := range wantCMList.Items {
30173017
want.Insert(cm.Name)
30183018
}
3019+
// Derive which NodePools are still active from CMs in the HCP namespace.
3020+
// When a NodePool is deleted, its finalizer removes all its CMs from the HCP namespace,
3021+
// so zero CMs for a given NodePool means it has been deleted.
3022+
// Note: a narrow race exists for a NodePool with exactly one immutable kubelet-config CM
3023+
// during the one-time immutable-to-mutable migration — the NodePool controller briefly
3024+
// deletes the CM before recreating it. If HCCO reconciles in that window the NodePool
3025+
// appears inactive. This is acceptable: the window is milliseconds, the migration is a
3026+
// one-time event, and the guest CM would be recreated on the next reconcile.
3027+
activeNodePools := set.Set[string]{}
3028+
for _, cm := range wantCMList.Items {
3029+
if npName := cm.Labels[hyperv1.NodePoolLabel]; npName != "" {
3030+
activeNodePools.Insert(npName)
3031+
}
3032+
}
30193033
for _, cm := range wantCMList.Items {
30203034
hostedClusterCM := &corev1.ConfigMap{
30213035
ObjectMeta: metav1.ObjectMeta{
@@ -3059,16 +3073,21 @@ func (r *reconciler) reconcileKubeletConfig(ctx context.Context) error {
30593073
}
30603074
// Mirrored CMs have a source in the HCP namespace managed by the NodePool controller.
30613075
// During delete+recreate migrations or transient API errors the source can be briefly
3062-
// absent. Deleting the guest copy here would cause NTO to regenerate MachineConfigs
3063-
// without it, triggering MCO node rollouts. If the source is permanently removed
3064-
// (e.g. NodePool deletion), the orphaned guest CM is harmless and will be cleaned up
3065-
// when the HostedCluster is deleted.
3066-
// TODO(OCPBUGS-88738): check whether the owning NodePool (via NodePoolLabel) still exists
3067-
// before unconditionally skipping, to allow cleanup of truly orphaned CMs.
3076+
// absent. Deleting the guest copy would cause NTO to regenerate MachineConfigs
3077+
// without it, triggering MCO node rollouts. However, if the owning NodePool has been
3078+
// deleted, its finalizer has already removed all its CMs from the HCP namespace, so
3079+
// the guest copy is orphaned and safe to delete.
30683080
if cm.Labels[nodepool.NTOMirroredConfigLabel] == "true" {
3069-
log.Info("skipping deletion of mirrored ConfigMap; source may be transiently absent or permanently removed after NodePool deletion",
3070-
"configMap", client.ObjectKeyFromObject(cm).String())
3071-
continue
3081+
npName := cm.Labels[hyperv1.NodePoolLabel]
3082+
// Defensive: if the CM has no NodePoolLabel, we cannot determine whether
3083+
// its owning NodePool still exists; preserve it to avoid spurious rollouts.
3084+
if npName == "" || activeNodePools.Has(npName) {
3085+
log.Info("skipping deletion of mirrored ConfigMap; source transiently absent but owning NodePool still active",
3086+
"configMap", client.ObjectKeyFromObject(cm).String())
3087+
continue
3088+
}
3089+
log.Info("deleting orphaned mirrored ConfigMap; owning NodePool no longer exists",
3090+
"configMap", client.ObjectKeyFromObject(cm).String(), "nodePool", npName)
30723091
}
30733092
log.Info("delete mirror config ConfigMap", "config", client.ObjectKeyFromObject(cm).String())
30743093
if _, err := k8sutil.DeleteIfNeeded(ctx, r.client, cm); err != nil {

control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources_test.go

Lines changed: 52 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1649,13 +1649,21 @@ func TestReconcileKubeletConfig(t *testing.T) {
16491649
},
16501650
},
16511651
{
1652-
name: "When source CM is transiently absent, it should not delete the mirrored guest-side CM",
1653-
hostedControlPlaneObjects: []client.Object{},
1652+
// NodePool still active (another CM exists in HCP namespace with the same NodePoolLabel),
1653+
// but this particular source CM is transiently absent — preserve the guest copy.
1654+
name: "When source CM is transiently absent, it should not delete the mirrored guest-side CM",
1655+
hostedControlPlaneObjects: []client.Object{
1656+
// Another CM for the same NodePool proves it is still active.
1657+
makeMirroredKubeletConfigConfigMap(netutil.ShortenName("baz", npName1, validation.LabelValueMaxLength), hcpNamespace, npName1, kubeletConfig1),
1658+
},
16541659
existHostedControlPlaneObjects: []client.Object{
16551660
makeMirroredKubeletConfigConfigMap(netutil.ShortenName("bar", npName1, validation.LabelValueMaxLength), hcNamespace, npName1, kubeletConfig1),
1661+
// The "baz" CM is expected on the guest side too (reconciled from the HCP-namespace source above).
1662+
makeMirroredKubeletConfigConfigMap(netutil.ShortenName("baz", npName1, validation.LabelValueMaxLength), hcNamespace, npName1, kubeletConfig1),
16561663
},
16571664
expectedHostedClusterObjects: []client.Object{
16581665
makeMirroredKubeletConfigConfigMap(netutil.ShortenName("bar", npName1, validation.LabelValueMaxLength), hcNamespace, npName1, kubeletConfig1),
1666+
makeMirroredKubeletConfigConfigMap(netutil.ShortenName("baz", npName1, validation.LabelValueMaxLength), hcNamespace, npName1, kubeletConfig1),
16591667
},
16601668
},
16611669
{
@@ -1667,6 +1675,48 @@ func TestReconcileKubeletConfig(t *testing.T) {
16671675
},
16681676
expectedHostedClusterObjects: []client.Object{},
16691677
},
1678+
{
1679+
// NodePool deleted: its finalizer has removed all CMs from the HCP namespace,
1680+
// so the orphaned guest-side mirrored CM should be cleaned up.
1681+
name: "When NodePool is deleted, it should delete orphaned mirrored guest-side CM",
1682+
hostedControlPlaneObjects: []client.Object{},
1683+
existHostedControlPlaneObjects: []client.Object{
1684+
makeMirroredKubeletConfigConfigMap(netutil.ShortenName("bar", npName1, validation.LabelValueMaxLength), hcNamespace, npName1, kubeletConfig1),
1685+
},
1686+
expectedHostedClusterObjects: []client.Object{},
1687+
},
1688+
{
1689+
// Defensive: mirrored CM without NodePoolLabel cannot be attributed to any NodePool,
1690+
// so preserve it to avoid spurious MCO rollouts.
1691+
name: "When mirrored CM has no NodePoolLabel, it should be preserved",
1692+
hostedControlPlaneObjects: []client.Object{},
1693+
existHostedControlPlaneObjects: []client.Object{
1694+
&corev1.ConfigMap{
1695+
ObjectMeta: metav1.ObjectMeta{
1696+
Name: "orphan-no-np-label",
1697+
Namespace: hcNamespace,
1698+
Labels: map[string]string{
1699+
nodepool.KubeletConfigConfigMapLabel: "true",
1700+
nodepool.NTOMirroredConfigLabel: "true",
1701+
},
1702+
},
1703+
Data: map[string]string{"config": kubeletConfig1},
1704+
},
1705+
},
1706+
expectedHostedClusterObjects: []client.Object{
1707+
&corev1.ConfigMap{
1708+
ObjectMeta: metav1.ObjectMeta{
1709+
Name: "orphan-no-np-label",
1710+
Namespace: hcNamespace,
1711+
Labels: map[string]string{
1712+
nodepool.KubeletConfigConfigMapLabel: "true",
1713+
nodepool.NTOMirroredConfigLabel: "true",
1714+
},
1715+
},
1716+
Data: map[string]string{"config": kubeletConfig1},
1717+
},
1718+
},
1719+
},
16701720
{
16711721
name: "When guest CM is immutable but not a KubeletConfig, it should not be deleted",
16721722
hostedControlPlaneObjects: []client.Object{

0 commit comments

Comments
 (0)