Skip to content

Commit d4b8418

Browse files
anandrkskdanandrkskd
authored
Remove hard dependency on OCP CRDs (#1157)
* remove openshift CRD as hard dependency Signed-off-by: Anand Kumar Singh <anandrkskd@gmail.com> * add a check for argocd-metrics to skip if monitoring api is not available Signed-off-by: Anand Kumar Singh <anandrkskd@gmail.com> * fix failure and integrate review comment assisted-by: claude-code for code,planning remove duplicate import, add checks before gitopsService and other OCP specific controller Signed-off-by: Anand Kumar Singh <anandrkskd@gmail.com> * remove unit tests Signed-off-by: Anand Kumar Singh <anandrkskd@gmail.com> * run all api check before returning, add olm api check before register, keep the check and register pattern same for all conditional api registration assisted-by: claude-code for code,planning Signed-off-by: Anand Kumar Singh <anandrkskd@gmail.com> * incorporate review comments remove non openshift check before config.openshift.io api check to make sure CRDs are not skipped, and not gate gitopsservice Signed-off-by: Anand Kumar Singh <anandrkskd@gmail.com> * update tests Signed-off-by: Anand Kumar Singh <anandrkskd@gmail.com> * fix failure to return accumulated error Signed-off-by: Anand Kumar Singh <anandrkskd@gmail.com> --------- Signed-off-by: Anand Kumar Singh <anandrkskd@gmail.com>
1 parent aacca24 commit d4b8418

6 files changed

Lines changed: 290 additions & 41 deletions

File tree

‎cmd/main.go‎

Lines changed: 65 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ func main() {
133133
ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts)))
134134

135135
if err := util.InspectCluster(); err != nil {
136-
setupLog.Info("unable to inspect cluster")
136+
setupLog.Error(err, "unable to inspect cluster")
137137
}
138138

139139
disableHTTP2 := func(c *tls.Config) {
@@ -198,18 +198,52 @@ func main() {
198198
client = mgr.GetClient()
199199
}
200200

201-
registerComponentOrExit(mgr, console.AddToScheme)
202-
registerComponentOrExit(mgr, routev1.AddToScheme) // Adding the routev1 api
203-
registerComponentOrExit(mgr, operatorsv1.AddToScheme)
204-
registerComponentOrExit(mgr, operatorsv1alpha1.AddToScheme)
201+
// Setup Scheme for OpenShift Console if available (verified by InspectCluster)
202+
if util.IsConsoleAPIFound() {
203+
registerComponentOrExit(mgr, console.AddToScheme)
204+
}
205+
206+
// Setup Scheme for OpenShift Route if available (verified by InspectCluster)
207+
if util.IsRouteAPIFound() {
208+
registerComponentOrExit(mgr, routev1.AddToScheme)
209+
}
210+
211+
// Setup Scheme for Prometheus Operator if available (verified by InspectCluster)
212+
if util.IsMonitoringAPIFound() {
213+
registerComponentOrExit(mgr, monitoringv1.AddToScheme)
214+
}
215+
216+
// Setup Scheme for OLM if available (verified by InspectCluster)
217+
if util.IsOLMAPIFound() {
218+
registerComponentOrExit(mgr, operatorsv1.AddToScheme)
219+
registerComponentOrExit(mgr, operatorsv1alpha1.AddToScheme)
220+
}
221+
205222
registerComponentOrExit(mgr, argov1alpha1api.AddToScheme)
206223
registerComponentOrExit(mgr, argov1beta1api.AddToScheme)
207-
registerComponentOrExit(mgr, configv1.AddToScheme)
208-
registerComponentOrExit(mgr, monitoringv1.AddToScheme)
224+
225+
// Setup Scheme for OpenShift Config if available
226+
if util.IsConfigAPIFound() {
227+
registerComponentOrExit(mgr, configv1.AddToScheme)
228+
}
229+
209230
registerComponentOrExit(mgr, rolloutManagerApi.AddToScheme)
210-
registerComponentOrExit(mgr, templatev1.AddToScheme)
211-
registerComponentOrExit(mgr, appsv1.AddToScheme)
212-
registerComponentOrExit(mgr, oauthv1.AddToScheme)
231+
232+
// Setup Scheme for OpenShift Template if available (verified by InspectCluster)
233+
if util.IsTemplateAPIFound() {
234+
registerComponentOrExit(mgr, templatev1.AddToScheme)
235+
}
236+
237+
// Setup Scheme for OpenShift Apps if available (verified by InspectCluster)
238+
if util.IsAppsAPIFound() {
239+
registerComponentOrExit(mgr, appsv1.AddToScheme)
240+
}
241+
242+
// Setup Scheme for OpenShift OAuth if available (verified by InspectCluster)
243+
if util.IsOAuthAPIFound() {
244+
registerComponentOrExit(mgr, oauthv1.AddToScheme)
245+
}
246+
213247
registerComponentOrExit(mgr, crdv1.AddToScheme)
214248

215249
// Start webhook only if ENABLE_CONVERSION_WEBHOOK is set
@@ -229,21 +263,30 @@ func main() {
229263
os.Exit(1)
230264
}
231265

232-
if err = (&controllers.ReconcileArgoCDRoute{
233-
Client: client,
234-
Scheme: mgr.GetScheme(),
235-
}).SetupWithManager(mgr); err != nil {
236-
setupLog.Error(err, "unable to create controller", "controller", "Argo CD route")
237-
os.Exit(1)
266+
if util.IsRouteAPIFound() {
267+
if err = (&controllers.ReconcileArgoCDRoute{
268+
Client: client,
269+
Scheme: mgr.GetScheme(),
270+
}).SetupWithManager(mgr); err != nil {
271+
setupLog.Error(err, "unable to create controller", "controller", "Argo CD route")
272+
os.Exit(1)
273+
}
274+
} else {
275+
setupLog.Info("Route API not found, skipping ReconcileArgoCDRoute controller setup")
238276
}
239277

240-
if err = (&controllers.ArgoCDMetricsReconciler{
241-
Client: client,
242-
Scheme: mgr.GetScheme(),
243-
}).SetupWithManager(mgr); err != nil {
244-
setupLog.Error(err, "unable to create controller", "controller", "Argo CD metrics")
245-
os.Exit(1)
278+
if util.IsMonitoringAPIFound() {
279+
if err = (&controllers.ArgoCDMetricsReconciler{
280+
Client: client,
281+
Scheme: mgr.GetScheme(),
282+
}).SetupWithManager(mgr); err != nil {
283+
setupLog.Error(err, "unable to create controller", "controller", "Argo CD metrics")
284+
os.Exit(1)
285+
}
286+
} else {
287+
setupLog.Info("Monitoring API not found, skipping Argo CD metrics controller setup")
246288
}
289+
247290
// Check the label selector format eg. "foo=bar"
248291
if _, err := labels.Parse(labelSelectorFlag); err != nil {
249292
setupLog.Error(err, "error parsing the labelSelector '%s'.", labelSelectorFlag)

‎controllers/argocd_controller.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,9 @@ func isConsoleLinkDisabled() bool {
7171

7272
// SetupWithManager sets up the controller with the Manager.
7373
func (r *ReconcileArgoCDRoute) SetupWithManager(mgr ctrl.Manager) error {
74-
// Watch for changes to argocd-server route in the default argocd instance namespace
75-
// The ConsoleLink holds the route URL and should be regenerated when route is updated
74+
if !util.IsRouteAPIFound() {
75+
return nil
76+
}
7677

7778
return ctrl.NewControllerManagedBy(mgr).
7879
For(&routev1.Route{}, builder.WithPredicates(filterPredicate(filterArgoCDRoute))).

‎controllers/gitopsservice_controller.go‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -101,15 +101,20 @@ func (r *ReconcileGitopsService) SetupWithManager(mgr ctrl.Manager) error {
101101
reqLogger.Error(err, "Failed to create GitOps service instance")
102102
}
103103

104-
return ctrl.NewControllerManagedBy(mgr).
104+
bldr := ctrl.NewControllerManagedBy(mgr).
105105
For(&pipelinesv1alpha1.GitopsService{}, builder.WithPredicates(pred)).
106106
Owns(&rbacv1.ClusterRoleBinding{}).
107107
Owns(&rbacv1.ClusterRole{}).
108108
Owns(&corev1.ServiceAccount{}).
109109
Owns(&corev1.ConfigMap{}).
110110
Owns(&appsv1.Deployment{}, builder.WithPredicates(pred)).
111-
Owns(&corev1.Service{}, builder.WithPredicates(pred)).
112-
Owns(&routev1.Route{}, builder.WithPredicates(pred)).
111+
Owns(&corev1.Service{}, builder.WithPredicates(pred))
112+
113+
if util.IsRouteAPIFound() {
114+
bldr = bldr.Owns(&routev1.Route{}, builder.WithPredicates(pred))
115+
}
116+
117+
return bldr.
113118
Watches(
114119
&corev1.Namespace{},
115120
&handler.EnqueueRequestForObject{},
@@ -345,14 +350,16 @@ func (r *ReconcileGitopsService) cleanKAMResources(ctx context.Context, reqLogge
345350
}
346351

347352
// KAM Route
348-
cleanupKAMRoute := &routev1.Route{}
349-
if err := r.Client.Get(ctx, types.NamespacedName{Name: kamResourceName, Namespace: serviceNamespace}, cleanupKAMRoute); err == nil {
350-
reqLogger.Info("Detected unsupported KAM Route, deleting", "Name", kamResourceName, "Namespace", serviceNamespace)
351-
if err := r.Client.Delete(ctx, cleanupKAMRoute); err != nil && !errors.IsNotFound(err) {
352-
reqLogger.Error(err, "Failed to delete KAM Route", "Name", kamResourceName, "Namespace", serviceNamespace)
353+
if util.IsRouteAPIFound() {
354+
cleanupKAMRoute := &routev1.Route{}
355+
if err := r.Client.Get(ctx, types.NamespacedName{Name: kamResourceName, Namespace: serviceNamespace}, cleanupKAMRoute); err == nil {
356+
reqLogger.Info("Detected unsupported KAM Route, deleting", "Name", kamResourceName, "Namespace", serviceNamespace)
357+
if err := r.Client.Delete(ctx, cleanupKAMRoute); err != nil && !errors.IsNotFound(err) {
358+
reqLogger.Error(err, "Failed to delete KAM Route", "Name", kamResourceName, "Namespace", serviceNamespace)
359+
}
360+
} else if !errors.IsNotFound(err) {
361+
reqLogger.Error(err, "Failed to retrieve KAM Route", "Name", kamResourceName, "Namespace", serviceNamespace)
353362
}
354-
} else if !errors.IsNotFound(err) {
355-
reqLogger.Error(err, "Failed to retrieve KAM Route", "Name", kamResourceName, "Namespace", serviceNamespace)
356363
}
357364

358365
}

‎controllers/gitopsservice_controller_test.go‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package controllers
1919
import (
2020
"context"
2121
"fmt"
22+
"os"
2223
"testing"
2324

2425
argoapp "github.com/argoproj-labs/argocd-operator/api/v1beta1"
@@ -48,6 +49,14 @@ import (
4849
"sigs.k8s.io/controller-runtime/pkg/reconcile"
4950
)
5051

52+
func TestMain(m *testing.M) {
53+
util.SetConfigAPIFound(true)
54+
util.SetMonitoringAPIFound(true)
55+
util.SetRouteAPIFound(true)
56+
util.SetOLMAPIFound(true)
57+
os.Exit(m.Run())
58+
}
59+
5160
func TestImageFromEnvVariable(t *testing.T) {
5261
ns := types.NamespacedName{Name: "test", Namespace: "test"}
5362
t.Run("Image present as env variable", func(t *testing.T) {
@@ -927,6 +936,8 @@ func TestCleanKAMResources_ServiceExist(t *testing.T) {
927936
// Route exist
928937
func TestCleanKAMResources_RouteExist(t *testing.T) {
929938
logf.SetLogger(zap.New(zap.UseDevMode(true)))
939+
defer util.SetRouteAPIFound(util.IsRouteAPIFound())
940+
util.SetRouteAPIFound(true)
930941
s := scheme.Scheme
931942
addKnownTypesToScheme(s)
932943
kamRoute := &routev1.Route{
@@ -947,6 +958,8 @@ func TestCleanKAMResources_RouteExist(t *testing.T) {
947958
// All Resources exist
948959
func TestCleanKAMResources_AllResourcesExist(t *testing.T) {
949960
logf.SetLogger(zap.New(zap.UseDevMode(true)))
961+
defer util.SetRouteAPIFound(util.IsRouteAPIFound())
962+
util.SetRouteAPIFound(true)
950963
s := scheme.Scheme
951964
addKnownTypesToScheme(s)
952965
kamDeploy := &appsv1.Deployment{

‎controllers/util/test_util.go‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
/*
2+
Copyright 2021.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package util
18+
19+
// *** THIS SHOULD ONLY BE USED FOR UNIT TESTING ***
20+
func SetConfigAPIFound(found bool) {
21+
configAPIFound = found
22+
}
23+
24+
func SetMonitoringAPIFound(found bool) {
25+
monitoringAPIFound = found
26+
}
27+
28+
// *** THIS SHOULD ONLY BE USED FOR UNIT TESTING ***
29+
func SetConsoleAPIFound(found bool) {
30+
consoleAPIFound = found
31+
}
32+
33+
// *** THIS SHOULD ONLY BE USED FOR UNIT TESTING ***
34+
func SetRouteAPIFound(found bool) {
35+
routeAPIFound = found
36+
}
37+
38+
// *** THIS SHOULD ONLY BE USED FOR UNIT TESTING ***
39+
func SetTemplateAPIFound(found bool) {
40+
templateAPIFound = found
41+
}
42+
43+
// *** THIS SHOULD ONLY BE USED FOR UNIT TESTING ***
44+
func SetAppsAPIFound(found bool) {
45+
appsAPIFound = found
46+
}
47+
48+
// *** THIS SHOULD ONLY BE USED FOR UNIT TESTING ***
49+
func SetOAuthAPIFound(found bool) {
50+
oauthAPIFound = found
51+
}
52+
53+
// *** THIS SHOULD ONLY BE USED FOR UNIT TESTING ***
54+
func SetOLMAPIFound(found bool) {
55+
olmAPIFound = found
56+
}

0 commit comments

Comments
 (0)