Skip to content

Commit a68e18d

Browse files
committed
hypershift:status: check for duplicated configmap status
on hypershift platform, when PerformanceProfile gets replaced with a new one, there will be a short window when two different PerformanceProfile are exist for the same nodePool until Hypershift operator catches up and deletes the old one. in this case, we want to avoid creating duplicated ConfigMap for the status because it will break Hypershift reconciliation loop: https://github.com/openshift/hypershift/blob/53dc14cbdd1e46d8b648ed49c6419d34209ef406/hypershift-operator/controllers/nodepool/nto.go\#L330 if a different ConfigMap for the status already exists, requeue the object for later. Signed-off-by: Talor Itzhak <titzhak@redhat.com>
1 parent 026db93 commit a68e18d

File tree

2 files changed

+54
-6
lines changed
  • pkg/performanceprofile/controller/performanceprofile/hypershift

2 files changed

+54
-6
lines changed

pkg/performanceprofile/controller/performanceprofile/hypershift/components/handler.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ func ConfigMapMeta(name, profileName, namespace, npNamespacedName string) *corev
236236
Name: name,
237237
Labels: map[string]string{
238238
hypershiftconsts.PerformanceProfileNameLabel: profileName,
239-
hypershiftconsts.NodePoolNameLabel: parseNamespacedName(npNamespacedName),
239+
hypershiftconsts.NodePoolNameLabel: ParseNamespacedName(npNamespacedName),
240240
},
241241
Annotations: map[string]string{
242242
hypershiftconsts.NodePoolNameLabel: npNamespacedName,
@@ -245,10 +245,10 @@ func ConfigMapMeta(name, profileName, namespace, npNamespacedName string) *corev
245245
}
246246
}
247247

248-
// parseNamespacedName expects a string with the format "namespace/name"
248+
// ParseNamespacedName expects a string with the format "namespace/name"
249249
// and returns the name only.
250250
// If given a string in the format "name" returns "name".
251-
func parseNamespacedName(namespacedName string) string {
251+
func ParseNamespacedName(namespacedName string) string {
252252
parts := strings.SplitN(namespacedName, "/", 2)
253253
if len(parts) > 1 {
254254
return parts[1]

pkg/performanceprofile/controller/performanceprofile/hypershift/status/writer.go

Lines changed: 51 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,19 @@ package status
33
import (
44
"context"
55
"fmt"
6-
7-
"k8s.io/klog/v2"
8-
ctrl "sigs.k8s.io/controller-runtime"
6+
"time"
97

108
corev1 "k8s.io/api/core/v1"
9+
"k8s.io/apimachinery/pkg/labels"
1110
"k8s.io/apimachinery/pkg/runtime"
11+
"k8s.io/klog/v2"
12+
13+
ctrl "sigs.k8s.io/controller-runtime"
1214
"sigs.k8s.io/controller-runtime/pkg/client"
1315

1416
performancev2 "github.com/openshift/cluster-node-tuning-operator/pkg/apis/performanceprofile/v2"
1517
"github.com/openshift/cluster-node-tuning-operator/pkg/performanceprofile/controller/performanceprofile/hypershift"
18+
handler "github.com/openshift/cluster-node-tuning-operator/pkg/performanceprofile/controller/performanceprofile/hypershift/components"
1619
hypershiftconsts "github.com/openshift/cluster-node-tuning-operator/pkg/performanceprofile/controller/performanceprofile/hypershift/consts"
1720
"github.com/openshift/cluster-node-tuning-operator/pkg/performanceprofile/controller/performanceprofile/status"
1821
conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1"
@@ -36,6 +39,24 @@ func (w *writer) Update(ctx context.Context, object client.Object, conditions []
3639
return ctrl.Result{}, fmt.Errorf("wrong type conversion; want=*ConfigMap got=%T", object)
3740
}
3841

42+
// on hypershift platform,
43+
// when PerformanceProfile gets replaced with a new one,
44+
// there will be a short window when two different PerformanceProfile are exist for the same nodePool
45+
// until Hypershift operator catches up and deletes the old one.
46+
// in this case, we want to avoid creating duplicated ConfigMap for the status because it will
47+
// break Hypershift reconciliation loop:
48+
// https://github.com/openshift/hypershift/blob/53dc14cbdd1e46d8b648ed49c6419d34209ef406/hypershift-operator/controllers/nodepool/nto.go#L330
49+
// if a different ConfigMap for the status already exists, requeue the object for later.
50+
exists, err := isDifferentConfigMapStatusExists(ctx, w.dataPlaneClient, instance)
51+
if err != nil {
52+
return ctrl.Result{}, err
53+
}
54+
if exists {
55+
return ctrl.Result{
56+
RequeueAfter: time.Second * 10,
57+
}, nil
58+
}
59+
3960
s, ok := instance.Data[hypershiftconsts.TuningKey]
4061
if !ok {
4162
return ctrl.Result{}, fmt.Errorf("key named %q not found in ConfigMap %q", hypershiftconsts.TuningKey, client.ObjectKeyFromObject(instance).String())
@@ -75,3 +96,30 @@ func (w *writer) updateDegradedCondition(ctx context.Context, instance client.Ob
7596
}
7697
return result, conditionError
7798
}
99+
100+
func isDifferentConfigMapStatusExists(ctx context.Context, cli client.Client, instance *corev1.ConfigMap) (bool, error) {
101+
nodePoolNamespacedName, ok := instance.Annotations[hypershiftconsts.NodePoolNameLabel]
102+
if !ok {
103+
return false, fmt.Errorf("annotation %q not found in ConfigMap %q annotations", hypershiftconsts.NodePoolNameLabel, client.ObjectKeyFromObject(instance).String())
104+
}
105+
cmList := &corev1.ConfigMapList{}
106+
opts := &client.ListOptions{
107+
Namespace: instance.GetNamespace(),
108+
LabelSelector: labels.SelectorFromSet(map[string]string{
109+
hypershiftconsts.NTOGeneratedPerformanceProfileStatusConfigMapLabel: "true",
110+
hypershiftconsts.NodePoolNameLabel: handler.ParseNamespacedName(nodePoolNamespacedName),
111+
}),
112+
}
113+
err := cli.List(ctx, cmList, opts)
114+
if err != nil {
115+
return false, fmt.Errorf("failed to list ConfigMaps in namespace %s %w", instance.GetNamespace(), err)
116+
}
117+
ConfigMapStatusName := hypershift.GetStatusConfigMapName(instance.Name)
118+
for _, cm := range cmList.Items {
119+
if ConfigMapStatusName != cm.GetName() {
120+
klog.InfoS("Different PerformanceProfile-ConfigMap status found, wait for its deletion", "Existing", cm.GetName(), "NewToBeCreated", ConfigMapStatusName)
121+
return true, nil
122+
}
123+
}
124+
return false, nil
125+
}

0 commit comments

Comments
 (0)