Skip to content

Commit 646f88e

Browse files
Merge pull request #1213 from openshift-kni/skip-nrt-crd-annot
api: allow to skip reconciliation of NRT CRD
2 parents 7d0135b + 550be81 commit 646f88e

File tree

5 files changed

+226
-2
lines changed

5 files changed

+226
-2
lines changed

internal/api/annotations/annotations.go

+10
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@ const (
2626

2727
PauseReconciliationAnnotation = "config.numa-operator.openshift.io/pause-reconciliation"
2828
PauseReconciliationAnnotationEnabled = "enabled"
29+
30+
NRTAPIDefinitionAnnotation = "config.numa-operator.openshift.io/nrt-api-definition"
31+
NRTAPIFromCluster = "cluster" // trust whatever it is already in the cluster, if at all
2932
)
3033

3134
func IsCustomPolicyEnabled(annot map[string]string) bool {
@@ -48,3 +51,10 @@ func IsPauseReconciliationEnabled(annot map[string]string) bool {
4851
}
4952
return false
5053
}
54+
55+
func IsNRTAPIDefinitionCluster(annot map[string]string) bool {
56+
if v, ok := annot[NRTAPIDefinitionAnnotation]; ok && v == NRTAPIFromCluster {
57+
return true
58+
}
59+
return false
60+
}

internal/api/annotations/annotations_test.go

+35
Original file line numberDiff line numberDiff line change
@@ -122,3 +122,38 @@ func TestIsPauseReconciliationEnabled(t *testing.T) {
122122
})
123123
}
124124
}
125+
126+
func TestIsNRTAPIDefinitionCluster(t *testing.T) {
127+
testcases := []struct {
128+
description string
129+
annotations map[string]string
130+
expected bool
131+
}{
132+
{
133+
description: "empty map",
134+
annotations: map[string]string{},
135+
expected: false,
136+
},
137+
{
138+
description: "annotation set to anything but not \"cluster\" means the source is the operator",
139+
annotations: map[string]string{
140+
NRTAPIDefinitionAnnotation: "operator",
141+
},
142+
expected: false,
143+
},
144+
{
145+
description: "skipping NRT CRD reconciliation",
146+
annotations: map[string]string{
147+
NRTAPIDefinitionAnnotation: NRTAPIFromCluster,
148+
},
149+
expected: true,
150+
},
151+
}
152+
for _, tc := range testcases {
153+
t.Run(tc.description, func(t *testing.T) {
154+
if got := IsNRTAPIDefinitionCluster(tc.annotations); got != tc.expected {
155+
t.Errorf("expected %v got %v", tc.expected, got)
156+
}
157+
})
158+
}
159+
}

internal/api/features/_topics.json

+2-1
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
"schedattrwatch",
2020
"ngpoolname",
2121
"nganns",
22-
"metrics"
22+
"metrics",
23+
"nrtcrdanns"
2324
]
2425
}

internal/controller/numaresourcesoperator_controller.go

+5
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,11 @@ func (r *NUMAResourcesOperatorReconciler) degradeStatus(ctx context.Context, ins
224224
}
225225

226226
func (r *NUMAResourcesOperatorReconciler) reconcileResourceAPI(ctx context.Context, instance *nropv1.NUMAResourcesOperator, trees []nodegroupv1.Tree) intreconcile.Step {
227+
if annotations.IsNRTAPIDefinitionCluster(instance.Annotations) {
228+
// should never happen, so let's be vocal. Very vocal.
229+
r.Recorder.Eventf(instance, corev1.EventTypeWarning, "SkipCRDInstall", "Skipped to install Node Resource Topology CRD: caused by annotation")
230+
return intreconcile.StepSuccess()
231+
}
227232
applied, err := r.syncNodeResourceTopologyAPI(ctx)
228233
if err != nil {
229234
r.Recorder.Eventf(instance, corev1.EventTypeWarning, "FailedCRDInstall", "Failed to install Node Resource Topology CRD: %v", err)

internal/controller/numaresourcesoperator_controller_test.go

+174-1
Original file line numberDiff line numberDiff line change
@@ -444,7 +444,6 @@ var _ = Describe("Test NUMAResourcesOperator Reconcile", func() {
444444
By(fmt.Sprintf("Check that container %s image did not change", cnt.Name))
445445
Expect(cnt.Image).To(Equal("madeup-image:1"))
446446
})
447-
448447
})
449448
})
450449

@@ -1122,6 +1121,180 @@ var _ = Describe("Test NUMAResourcesOperator Reconcile", func() {
11221121
Expect(nro).To(BeDegradedWithReason(validation.NodeGroupsError))
11231122
})
11241123
})
1124+
1125+
When("the NRT API definition annotation is provided", Label("feature:nrtcrdanns"), func() {
1126+
var nro *nropv1.NUMAResourcesOperator
1127+
var mcp1 *machineconfigv1.MachineConfigPool
1128+
var mcp1Selector *metav1.LabelSelector
1129+
var nroKey client.ObjectKey
1130+
1131+
var reconciler *NUMAResourcesOperatorReconciler
1132+
var ng1 nropv1.NodeGroup
1133+
1134+
pn1 := "test1"
1135+
1136+
BeforeEach(func() {
1137+
mcp1Selector = &metav1.LabelSelector{
1138+
MatchLabels: map[string]string{
1139+
pn1: pn1,
1140+
},
1141+
}
1142+
ng1 = nropv1.NodeGroup{
1143+
PoolName: &pn1,
1144+
}
1145+
1146+
nro = testobjs.NewNUMAResourcesOperator(objectnames.DefaultNUMAResourcesOperatorCrName, ng1)
1147+
nroKey = client.ObjectKeyFromObject(nro)
1148+
1149+
mcp1 = testobjs.NewMachineConfigPool(pn1, mcp1Selector.MatchLabels, mcp1Selector, mcp1Selector)
1150+
1151+
var err error
1152+
reconciler, err = NewFakeNUMAResourcesOperatorReconciler(platf, defaultOCPVersion, nro, mcp1)
1153+
Expect(err).ToNot(HaveOccurred())
1154+
})
1155+
1156+
It("should be NOT create the CRD object while the annotation is present", func() {
1157+
Eventually(func() error {
1158+
nroUpdated := &nropv1.NUMAResourcesOperator{}
1159+
Expect(reconciler.Client.Get(context.TODO(), nroKey, nroUpdated)).To(Succeed())
1160+
if nroUpdated.Annotations == nil {
1161+
nroUpdated.Annotations = make(map[string]string)
1162+
}
1163+
nroUpdated.Annotations[annotations.NRTAPIDefinitionAnnotation] = annotations.NRTAPIFromCluster
1164+
return reconciler.Client.Update(context.TODO(), nroUpdated)
1165+
}).WithPolling(1 * time.Second).WithTimeout(30 * time.Second).Should(Succeed())
1166+
1167+
Expect(reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: nroKey})).ToNot(CauseRequeue())
1168+
1169+
nroUpdated := &nropv1.NUMAResourcesOperator{}
1170+
Expect(reconciler.Client.Get(context.TODO(), nroKey, nroUpdated)).To(Succeed())
1171+
1172+
availableCondition := getConditionByType(nroUpdated.Status.Conditions, status.ConditionAvailable)
1173+
Expect(availableCondition).ToNot(BeNil())
1174+
Expect(availableCondition.Status).To(Equal(metav1.ConditionTrue))
1175+
1176+
crd := &apiextensionsv1.CustomResourceDefinition{}
1177+
crdKey := client.ObjectKey{
1178+
Name: "noderesourcetopologies.topology.node.k8s.io",
1179+
}
1180+
err := reconciler.Client.Get(context.TODO(), crdKey, crd)
1181+
Expect(apierrors.IsNotFound(err)).To(BeTrue(), "unexpected error: %v", err)
1182+
})
1183+
1184+
It("should stop reconciling the CRD object while the annotation is present", func() {
1185+
// ensure no annotations. Just to be sure
1186+
Eventually(func() error {
1187+
nroUpdated := &nropv1.NUMAResourcesOperator{}
1188+
Expect(reconciler.Client.Get(context.TODO(), nroKey, nroUpdated)).To(Succeed())
1189+
nroUpdated.Annotations = nil
1190+
return reconciler.Client.Update(context.TODO(), nroUpdated)
1191+
}).WithPolling(1 * time.Second).WithTimeout(30 * time.Second).Should(Succeed())
1192+
1193+
Expect(reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: nroKey})).ToNot(CauseRequeue())
1194+
1195+
var nroUpdated nropv1.NUMAResourcesOperator
1196+
Expect(reconciler.Client.Get(context.TODO(), nroKey, &nroUpdated)).To(Succeed())
1197+
1198+
availableCondition := getConditionByType(nroUpdated.Status.Conditions, status.ConditionAvailable)
1199+
Expect(availableCondition).ToNot(BeNil())
1200+
Expect(availableCondition.Status).To(Equal(metav1.ConditionTrue))
1201+
1202+
var crd apiextensionsv1.CustomResourceDefinition
1203+
crdKey := client.ObjectKey{
1204+
Name: "noderesourcetopologies.topology.node.k8s.io",
1205+
}
1206+
Expect(reconciler.Client.Get(context.TODO(), crdKey, &crd)).To(Succeed())
1207+
1208+
Eventually(func() error {
1209+
nroUpdated := &nropv1.NUMAResourcesOperator{}
1210+
Expect(reconciler.Client.Get(context.TODO(), nroKey, nroUpdated)).To(Succeed())
1211+
if nroUpdated.Annotations == nil {
1212+
nroUpdated.Annotations = make(map[string]string)
1213+
}
1214+
nroUpdated.Annotations[annotations.NRTAPIDefinitionAnnotation] = annotations.NRTAPIFromCluster
1215+
return reconciler.Client.Update(context.TODO(), nroUpdated)
1216+
}).WithPolling(1 * time.Second).WithTimeout(30 * time.Second).Should(Succeed())
1217+
1218+
Expect(reconciler.Client.Delete(context.TODO(), &crd)).To(Succeed())
1219+
1220+
Expect(reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: nroKey})).ToNot(CauseRequeue())
1221+
1222+
Expect(reconciler.Client.Get(context.TODO(), nroKey, &nroUpdated)).To(Succeed())
1223+
1224+
availableCondition = getConditionByType(nroUpdated.Status.Conditions, status.ConditionAvailable)
1225+
Expect(availableCondition).ToNot(BeNil())
1226+
Expect(availableCondition.Status).To(Equal(metav1.ConditionTrue))
1227+
1228+
err := reconciler.Client.Get(context.TODO(), crdKey, &crd)
1229+
Expect(apierrors.IsNotFound(err)).To(BeTrue(), "unexpected error: %v", err)
1230+
})
1231+
1232+
It("should reconcile again the CRD object when the annotation is removed", func() {
1233+
// ensure no annotations. Just to be sure
1234+
Eventually(func() error {
1235+
nroUpdated := &nropv1.NUMAResourcesOperator{}
1236+
Expect(reconciler.Client.Get(context.TODO(), nroKey, nroUpdated)).To(Succeed())
1237+
nroUpdated.Annotations = nil
1238+
return reconciler.Client.Update(context.TODO(), nroUpdated)
1239+
}).WithPolling(1 * time.Second).WithTimeout(30 * time.Second).Should(Succeed())
1240+
1241+
Expect(reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: nroKey})).ToNot(CauseRequeue())
1242+
1243+
var nroUpdated nropv1.NUMAResourcesOperator
1244+
Expect(reconciler.Client.Get(context.TODO(), nroKey, &nroUpdated)).To(Succeed())
1245+
1246+
availableCondition := getConditionByType(nroUpdated.Status.Conditions, status.ConditionAvailable)
1247+
Expect(availableCondition).ToNot(BeNil())
1248+
Expect(availableCondition.Status).To(Equal(metav1.ConditionTrue))
1249+
1250+
var crd apiextensionsv1.CustomResourceDefinition
1251+
crdKey := client.ObjectKey{
1252+
Name: "noderesourcetopologies.topology.node.k8s.io",
1253+
}
1254+
Expect(reconciler.Client.Get(context.TODO(), crdKey, &crd)).To(Succeed())
1255+
1256+
Eventually(func() error {
1257+
nroUpdated := &nropv1.NUMAResourcesOperator{}
1258+
Expect(reconciler.Client.Get(context.TODO(), nroKey, nroUpdated)).To(Succeed())
1259+
if nroUpdated.Annotations == nil {
1260+
nroUpdated.Annotations = make(map[string]string)
1261+
}
1262+
nroUpdated.Annotations[annotations.NRTAPIDefinitionAnnotation] = annotations.NRTAPIFromCluster
1263+
return reconciler.Client.Update(context.TODO(), nroUpdated)
1264+
}).WithPolling(1 * time.Second).WithTimeout(30 * time.Second).Should(Succeed())
1265+
1266+
Expect(reconciler.Client.Delete(context.TODO(), &crd)).To(Succeed())
1267+
1268+
Expect(reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: nroKey})).ToNot(CauseRequeue())
1269+
1270+
Expect(reconciler.Client.Get(context.TODO(), nroKey, &nroUpdated)).To(Succeed())
1271+
1272+
availableCondition = getConditionByType(nroUpdated.Status.Conditions, status.ConditionAvailable)
1273+
Expect(availableCondition).ToNot(BeNil())
1274+
Expect(availableCondition.Status).To(Equal(metav1.ConditionTrue))
1275+
1276+
err := reconciler.Client.Get(context.TODO(), crdKey, &crd)
1277+
Expect(apierrors.IsNotFound(err)).To(BeTrue(), "unexpected error: %v", err)
1278+
1279+
Eventually(func() error {
1280+
nroUpdated := &nropv1.NUMAResourcesOperator{}
1281+
Expect(reconciler.Client.Get(context.TODO(), nroKey, nroUpdated)).To(Succeed())
1282+
nroUpdated.Annotations = nil
1283+
return reconciler.Client.Update(context.TODO(), nroUpdated)
1284+
}).WithPolling(1 * time.Second).WithTimeout(30 * time.Second).Should(Succeed())
1285+
1286+
Expect(reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: nroKey})).ToNot(CauseRequeue())
1287+
1288+
Expect(reconciler.Client.Get(context.TODO(), nroKey, &nroUpdated)).To(Succeed())
1289+
1290+
availableCondition = getConditionByType(nroUpdated.Status.Conditions, status.ConditionAvailable)
1291+
Expect(availableCondition).ToNot(BeNil())
1292+
Expect(availableCondition.Status).To(Equal(metav1.ConditionTrue))
1293+
1294+
Expect(reconciler.Client.Get(context.TODO(), crdKey, &crd)).To(Succeed())
1295+
})
1296+
})
1297+
11251298
},
11261299
Entry("Openshift Platform", platform.OpenShift),
11271300
Entry("Hypershift Platform", platform.HyperShift),

0 commit comments

Comments
 (0)