Skip to content

Commit 2fb2e00

Browse files
varunrsekarvarunrsekar
committed
add check for resourceclaim CRD presence
Signed-off-by: Varun Ramachandra Sekar <[email protected]>
1 parent 67f2a25 commit 2fb2e00

File tree

5 files changed

+173
-6
lines changed

5 files changed

+173
-6
lines changed

‎internal/controller/nimservice_controller.go

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,7 @@ func (r *NIMServiceReconciler) SetupWithManager(mgr ctrl.Manager) error {
245245
return err
246246
}
247247

248-
return ctrl.NewControllerManagedBy(mgr).
248+
nimServiceBuilder := ctrl.NewControllerManagedBy(mgr).
249249
For(&appsv1alpha1.NIMService{}).
250250
Owns(&appsv1.Deployment{}).
251251
Owns(&appsv1.StatefulSet{}).
@@ -278,13 +278,21 @@ func (r *NIMServiceReconciler) SetupWithManager(mgr ctrl.Manager) error {
278278
&appsv1alpha1.NIMCache{},
279279
handler.EnqueueRequestsFromMapFunc(r.mapNIMCacheToNIMService),
280280
builder.WithPredicates(predicate.ResourceVersionChangedPredicate{}),
281-
).
282-
Watches(
281+
)
282+
283+
crdExists, err := k8sutil.CRDExists(r.discoveryClient, resourcev1beta2.SchemeGroupVersion.WithResource("resourceclaims"))
284+
if err != nil {
285+
return err
286+
}
287+
if crdExists {
288+
nimServiceBuilder = nimServiceBuilder.Watches(
283289
&resourcev1beta2.ResourceClaim{},
284290
handler.EnqueueRequestsFromMapFunc(r.mapResourceClaimToNIMService),
285291
builder.WithPredicates(predicate.ResourceVersionChangedPredicate{}),
286-
).
287-
Complete(r)
292+
)
293+
}
294+
295+
return nimServiceBuilder.Complete(r)
288296
}
289297

290298
func (r *NIMServiceReconciler) mapNIMCacheToNIMService(ctx context.Context, obj client.Object) []ctrl.Request {

‎internal/controller/platform/standalone/nimservice.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030
corev1 "k8s.io/api/core/v1"
3131
networkingv1 "k8s.io/api/networking/v1"
3232
rbacv1 "k8s.io/api/rbac/v1"
33+
resourcev1beta2 "k8s.io/api/resource/v1beta2"
3334
k8serrors "k8s.io/apimachinery/pkg/api/errors"
3435
"k8s.io/apimachinery/pkg/api/meta"
3536
apiResource "k8s.io/apimachinery/pkg/api/resource"
@@ -118,6 +119,18 @@ func (r *NIMServiceReconciler) validateDRAResources(ctx context.Context, nimServ
118119
return false, msg, nil
119120
}
120121

122+
// Check if the resource claim CRD exists
123+
crdExists, err := k8sutil.CRDExists(r.GetDiscoveryClient(), resourcev1beta2.SchemeGroupVersion.WithResource("resourceclaims"))
124+
if err != nil {
125+
logger.Error(err, "failed to check if resource claim CRD exists")
126+
return false, "", err
127+
}
128+
if !crdExists {
129+
msg := "DRA resources are not supported by NIM-Operator on this cluster, please ensure resource.k8s.io/v1beta2 API group is enabled"
130+
logger.Error(fmt.Errorf("%s", msg), msg, "nimService", nimService.Name)
131+
return false, msg, nil
132+
}
133+
121134
// Check if duplicate resource claim names are provided
122135
resourceClaimNames := make(map[string]bool)
123136
for idx, resource := range nimService.Spec.DRAResources {

‎internal/controller/platform/standalone/nimservice_test.go

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import (
3838
corev1 "k8s.io/api/core/v1"
3939
networkingv1 "k8s.io/api/networking/v1"
4040
rbacv1 "k8s.io/api/rbac/v1"
41+
resourcev1beta2 "k8s.io/api/resource/v1beta2"
4142
"k8s.io/apimachinery/pkg/api/errors"
4243
"k8s.io/apimachinery/pkg/api/resource"
4344
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -153,6 +154,14 @@ var _ = Describe("NIMServiceReconciler for a standalone platform", func() {
153154

154155
// Create mock discovery client
155156
discoveryClient = &discoveryfake.FakeDiscovery{Fake: &testing.Fake{}}
157+
discoveryClient.Resources = []*metav1.APIResourceList{
158+
{
159+
GroupVersion: resourcev1beta2.SchemeGroupVersion.String(),
160+
APIResources: []metav1.APIResource{
161+
{Name: "resourceclaims"},
162+
},
163+
},
164+
}
156165
discoveryClient.FakedServerVersion = &version.Info{
157166
GitVersion: "v1.33.0",
158167
}
@@ -687,6 +696,36 @@ var _ = Describe("NIMServiceReconciler for a standalone platform", func() {
687696
Expect(failedCondition.Message).To(Equal("DRA resources are not supported by NIM-Operator on this cluster, please upgrade to k8s version 'v1.33.0' or higher"))
688697
})
689698

699+
It("should mark NIMService as failed when resource claim CRD is not enabled", func() {
700+
reconciler.discoveryClient = &discoveryfake.FakeDiscovery{
701+
Fake: &testing.Fake{},
702+
FakedServerVersion: &version.Info{
703+
GitVersion: "v1.33.0",
704+
},
705+
}
706+
nimService.Spec.DRAResources = []appsv1alpha1.DRAResource{
707+
{
708+
ResourceClaimName: ptr.To("test-resource-claim"),
709+
},
710+
}
711+
nimServiceKey := types.NamespacedName{Name: nimService.Name, Namespace: nimService.Namespace}
712+
err := client.Create(context.TODO(), nimService)
713+
Expect(err).NotTo(HaveOccurred())
714+
715+
_, err = reconciler.reconcileNIMService(context.TODO(), nimService)
716+
Expect(err).NotTo(HaveOccurred())
717+
718+
obj := &appsv1alpha1.NIMService{}
719+
err = client.Get(context.TODO(), nimServiceKey, obj)
720+
Expect(err).NotTo(HaveOccurred())
721+
Expect(obj.Status.State).To(Equal(appsv1alpha1.NIMServiceStatusFailed))
722+
failedCondition := getCondition(obj, conditions.Failed)
723+
Expect(failedCondition).NotTo(BeNil())
724+
Expect(failedCondition.Status).To(Equal(metav1.ConditionTrue))
725+
Expect(failedCondition.Reason).To(Equal(conditions.ReasonDRAResourcesUnsupported))
726+
Expect(failedCondition.Message).To(Equal("DRA resources are not supported by NIM-Operator on this cluster, please ensure resource.k8s.io/v1beta2 API group is enabled"))
727+
})
728+
690729
It("should mark NIMService as failed when resource claim name is duplicated", func() {
691730
nimService.Spec.DRAResources = []appsv1alpha1.DRAResource{
692731
{

‎internal/k8sutil/k8sutil.go

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
appsv1 "k8s.io/api/apps/v1"
2525
corev1 "k8s.io/api/core/v1"
2626
"k8s.io/apimachinery/pkg/api/errors"
27+
"k8s.io/apimachinery/pkg/runtime/schema"
2728
"k8s.io/apimachinery/pkg/types"
2829
"k8s.io/client-go/discovery"
2930
"k8s.io/utils/ptr"
@@ -296,11 +297,31 @@ func GetRawYAMLFromConfigMap(ctx context.Context, k8sClient client.Client, names
296297

297298
func GetClusterVersion(discoveryClient discovery.DiscoveryInterface) (string, error) {
298299
if discoveryClient == nil {
299-
return "", fmt.Errorf("client not provided")
300+
return "", fmt.Errorf("discoveryClient not provided")
300301
}
301302
versionInfo, err := discoveryClient.ServerVersion()
302303
if err != nil {
303304
return "", err
304305
}
305306
return versionInfo.GitVersion, nil
306307
}
308+
309+
func CRDExists(discoveryClient discovery.DiscoveryInterface, gvr schema.GroupVersionResource) (bool, error) {
310+
if discoveryClient == nil {
311+
return false, fmt.Errorf("discoveryClient not provided")
312+
}
313+
resources, err := discoveryClient.ServerResourcesForGroupVersion(gvr.GroupVersion().String())
314+
if err != nil {
315+
// If the resource is not found, then the CRD is not enabled.
316+
if errors.IsNotFound(err) {
317+
return false, nil
318+
}
319+
return false, err
320+
}
321+
for _, resource := range resources.APIResources {
322+
if resource.Name == gvr.Resource {
323+
return true, nil
324+
}
325+
}
326+
return false, nil
327+
}

‎internal/k8sutil/k8sutil_test.go

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
. "github.com/onsi/gomega"
2525
corev1 "k8s.io/api/core/v1"
2626
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
27+
"k8s.io/apimachinery/pkg/runtime/schema"
2728
"k8s.io/apimachinery/pkg/version"
2829
discoveryfake "k8s.io/client-go/discovery/fake"
2930
testing "k8s.io/client-go/testing"
@@ -82,4 +83,89 @@ var _ = Describe("K8s util tests", func() {
8283
Expect(version).To(Equal("v1.33.0"))
8384
})
8485
})
86+
87+
Context("CRDExists", func() {
88+
var fakeDiscovery *discoveryfake.FakeDiscovery
89+
90+
BeforeEach(func() {
91+
fakeDiscovery = &discoveryfake.FakeDiscovery{
92+
Fake: &testing.Fake{
93+
RWMutex: sync.RWMutex{},
94+
},
95+
}
96+
})
97+
98+
It("should return error when discovery client is nil", func() {
99+
exists, err := CRDExists(nil, schema.GroupVersionResource{
100+
Group: "test.example.com",
101+
Version: "v1",
102+
Resource: "testresources",
103+
})
104+
Expect(err).To(HaveOccurred())
105+
Expect(exists).To(BeFalse())
106+
})
107+
108+
It("should return false when CRD is not found", func() {
109+
fakeDiscovery.Resources = []*metav1.APIResourceList{
110+
{
111+
GroupVersion: "test.example.com/v1",
112+
APIResources: []metav1.APIResource{
113+
{
114+
Name: "otherresources",
115+
},
116+
},
117+
},
118+
}
119+
120+
exists, err := CRDExists(fakeDiscovery, schema.GroupVersionResource{
121+
Group: "test.example.com",
122+
Version: "v1",
123+
Resource: "testresources",
124+
})
125+
Expect(err).NotTo(HaveOccurred())
126+
Expect(exists).To(BeFalse())
127+
})
128+
129+
It("should return true when CRD exists", func() {
130+
fakeDiscovery.Resources = []*metav1.APIResourceList{
131+
{
132+
GroupVersion: "test.example.com/v1",
133+
APIResources: []metav1.APIResource{
134+
{
135+
Name: "testresources",
136+
},
137+
},
138+
},
139+
}
140+
141+
exists, err := CRDExists(fakeDiscovery, schema.GroupVersionResource{
142+
Group: "test.example.com",
143+
Version: "v1",
144+
Resource: "testresources",
145+
})
146+
Expect(err).NotTo(HaveOccurred())
147+
Expect(exists).To(BeTrue())
148+
})
149+
150+
It("should return false when group version is not found", func() {
151+
fakeDiscovery.Resources = []*metav1.APIResourceList{
152+
{
153+
GroupVersion: "other.example.com/v1",
154+
APIResources: []metav1.APIResource{
155+
{
156+
Name: "testresources",
157+
},
158+
},
159+
},
160+
}
161+
162+
exists, err := CRDExists(fakeDiscovery, schema.GroupVersionResource{
163+
Group: "test.example.com",
164+
Version: "v1",
165+
Resource: "testresources",
166+
})
167+
Expect(err).NotTo(HaveOccurred())
168+
Expect(exists).To(BeFalse())
169+
})
170+
})
85171
})

0 commit comments

Comments
 (0)