Skip to content

Commit 878085e

Browse files
dejanzeledejanzele
committed
remove duplicated code and fix minor bugs
1 parent e3bcf44 commit 878085e

18 files changed

+567
-514
lines changed

‎go.mod

+1
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ require (
2323
github.com/cespare/xxhash/v2 v2.3.0 // indirect
2424
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
2525
github.com/emicklei/go-restful/v3 v3.12.1 // indirect
26+
github.com/evanphx/json-patch v4.12.0+incompatible // indirect
2627
github.com/evanphx/json-patch/v5 v5.9.0 // indirect
2728
github.com/fsnotify/fsnotify v1.7.0 // indirect
2829
github.com/go-logr/zapr v1.3.0 // indirect

‎internal/controller/install/armadaserver_controller.go

+21-60
Original file line numberDiff line numberDiff line change
@@ -63,68 +63,35 @@ type ArmadaServerReconciler struct {
6363
// Reconcile is part of the main kubernetes reconciliation loop which aims to
6464
// move the current state of the cluster closer to the desired state.
6565
func (r *ArmadaServerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
66-
logger := log.FromContext(ctx).WithValues("namespace", req.Namespace, "name", req.Name)
66+
logger := log.FromContext(ctx).WithValues("kind", "ArmadaServer", "namespace", req.Namespace, "name", req.Name)
67+
6768
started := time.Now()
68-
logger.Info("Reconciling ArmadaServer object")
69-
70-
logger.Info("Fetching ArmadaServer object from cache")
71-
var as installv1alpha1.ArmadaServer
72-
if err := r.Client.Get(ctx, req.NamespacedName, &as); err != nil {
73-
if k8serrors.IsNotFound(err) {
74-
logger.Info("ArmadaServer not found in cache, ending reconcile...", "namespace", req.Namespace, "name", req.Name)
75-
return ctrl.Result{}, nil
76-
}
69+
70+
logger.Info("Reconciling object")
71+
72+
var server installv1alpha1.ArmadaServer
73+
if miss, err := getObjectFromCache(ctx, r.Client, &server, req.NamespacedName, logger); err != nil || miss {
7774
return ctrl.Result{}, err
7875
}
7976

80-
pc, err := installv1alpha1.BuildPortConfig(as.Spec.ApplicationConfig)
77+
pc, err := installv1alpha1.BuildPortConfig(server.Spec.ApplicationConfig)
8178
if err != nil {
8279
return ctrl.Result{}, err
8380
}
84-
as.Spec.PortConfig = pc
81+
server.Spec.PortConfig = pc
8582

8683
var components *CommonComponents
87-
components, err = generateArmadaServerInstallComponents(&as, r.Scheme)
84+
components, err = generateArmadaServerInstallComponents(&server, r.Scheme)
8885
if err != nil {
8986
return ctrl.Result{}, err
9087
}
9188

92-
deletionTimestamp := as.ObjectMeta.DeletionTimestamp
93-
// examine DeletionTimestamp to determine if object is under deletion
94-
if deletionTimestamp.IsZero() {
95-
// The object is not being deleted, so if it does not have our finalizer,
96-
// then lets add the finalizer and update the object. This is equivalent
97-
// registering our finalizer.
98-
if !controllerutil.ContainsFinalizer(&as, operatorFinalizer) {
99-
logger.Info("Attaching finalizer to As object", "finalizer", operatorFinalizer)
100-
controllerutil.AddFinalizer(&as, operatorFinalizer)
101-
if err := r.Update(ctx, &as); err != nil {
102-
return ctrl.Result{}, err
103-
}
104-
}
105-
} else {
106-
logger.Info("ArmadaServer object is being deleted", "finalizer", operatorFinalizer)
107-
logger.Info("Namespace-scoped resources will be deleted by Kubernetes based on their OwnerReference")
108-
// The object is being deleted
109-
if controllerutil.ContainsFinalizer(&as, operatorFinalizer) {
110-
// our finalizer is present, so lets handle any external dependency
111-
logger.Info("Running cleanup function for ArmadaServer cluster-scoped components", "finalizer", operatorFinalizer)
112-
if err := r.deleteExternalResources(ctx, components, logger); err != nil {
113-
// if fail to delete the external dependency here, return with error
114-
// so that it can be retried
115-
return ctrl.Result{}, err
116-
}
117-
118-
// remove our finalizer from the list and update it.
119-
logger.Info("Removing finalizer from ArmadaServer object", "finalizer", operatorFinalizer)
120-
controllerutil.RemoveFinalizer(&as, operatorFinalizer)
121-
if err := r.Update(ctx, &as); err != nil {
122-
return ctrl.Result{}, err
123-
}
124-
}
125-
126-
// Stop reconciliation as the item is being deleted
127-
return ctrl.Result{}, nil
89+
cleanupF := func(ctx context.Context) error {
90+
return r.deleteExternalResources(ctx, components, logger)
91+
}
92+
finish, err := checkAndHandleResourceDeletion(ctx, r.Client, &server, operatorFinalizer, cleanupF, logger)
93+
if err != nil || finish {
94+
return ctrl.Result{}, err
12895
}
12996

13097
componentsCopy := components.DeepCopy()
@@ -134,21 +101,15 @@ func (r *ArmadaServerReconciler) Reconcile(ctx context.Context, req ctrl.Request
134101
return nil
135102
}
136103

137-
if components.ServiceAccount != nil {
138-
logger.Info("Upserting ArmadaServer ServiceAccount object")
139-
if _, err := controllerutil.CreateOrUpdate(ctx, r.Client, components.ServiceAccount, mutateFn); err != nil {
140-
return ctrl.Result{}, err
141-
}
104+
if err := upsertObject(ctx, r.Client, components.ServiceAccount, server.Kind, mutateFn, logger); err != nil {
105+
return ctrl.Result{}, err
142106
}
143107

144-
if components.Secret != nil {
145-
logger.Info("Upserting ArmadaServer Secret object")
146-
if _, err := controllerutil.CreateOrUpdate(ctx, r.Client, components.Secret, mutateFn); err != nil {
147-
return ctrl.Result{}, err
148-
}
108+
if err := upsertObject(ctx, r.Client, components.Secret, server.Kind, mutateFn, logger); err != nil {
109+
return ctrl.Result{}, err
149110
}
150111

151-
if as.Spec.PulsarInit {
112+
if server.Spec.PulsarInit {
152113
for idx := range components.Jobs {
153114
err = func() error {
154115
if components.Jobs[idx] != nil {

‎internal/controller/install/binoculars_controller.go

+31-82
Original file line numberDiff line numberDiff line change
@@ -60,18 +60,14 @@ type BinocularsReconciler struct {
6060
// For more details, check Reconcile and its Result here:
6161
// - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.13.0/pkg/reconcile
6262
func (r *BinocularsReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
63-
logger := log.FromContext(ctx).WithValues("namespace", req.Namespace, "name", req.Name)
63+
logger := log.FromContext(ctx).WithValues("kind", "Binoculars", "namespace", req.Namespace, "name", req.Name)
64+
6465
started := time.Now()
65-
logger.Info("Reconciling Binoculars object")
6666

67-
logger.Info("Fetching Binoculars object from cache")
67+
logger.Info("Reconciling Binoculars object")
6868

6969
var binoculars installv1alpha1.Binoculars
70-
if err := r.Client.Get(ctx, req.NamespacedName, &binoculars); err != nil {
71-
if k8serrors.IsNotFound(err) {
72-
logger.Info("Binoculars not found in cache, ending reconcile...", "namespace", req.Namespace, "name", req.Name)
73-
return ctrl.Result{}, nil
74-
}
70+
if miss, err := getObjectFromCache(ctx, r.Client, &binoculars, req.NamespacedName, logger); err != nil || miss {
7571
return ctrl.Result{}, err
7672
}
7773

@@ -87,41 +83,12 @@ func (r *BinocularsReconciler) Reconcile(ctx context.Context, req ctrl.Request)
8783
return ctrl.Result{}, err
8884
}
8985

90-
deletionTimestamp := binoculars.ObjectMeta.DeletionTimestamp
91-
// examine DeletionTimestamp to determine if object is under deletion
92-
if deletionTimestamp.IsZero() {
93-
// The object is not being deleted, so if it does not have our finalizer,
94-
// then lets add the finalizer and update the object. This is equivalent
95-
// registering our finalizer.
96-
if !controllerutil.ContainsFinalizer(&binoculars, operatorFinalizer) {
97-
logger.Info("Attaching finalizer to Binoculars object", "finalizer", operatorFinalizer)
98-
controllerutil.AddFinalizer(&binoculars, operatorFinalizer)
99-
if err := r.Update(ctx, &binoculars); err != nil {
100-
return ctrl.Result{}, err
101-
}
102-
}
103-
} else {
104-
logger.Info("Binoculars object is being deleted", "finalizer", operatorFinalizer)
105-
// The object is being deleted
106-
if controllerutil.ContainsFinalizer(&binoculars, operatorFinalizer) {
107-
// our finalizer is present, so lets handle any external dependency
108-
logger.Info("Running cleanup function for Binoculars object", "finalizer", operatorFinalizer)
109-
if err := r.deleteExternalResources(ctx, components); err != nil {
110-
// if fail to delete the external dependency here, return with error
111-
// so that it can be retried
112-
return ctrl.Result{}, err
113-
}
114-
115-
// remove our finalizer from the list and update it.
116-
logger.Info("Removing finalizer from Binoculars object", "finalizer", operatorFinalizer)
117-
controllerutil.RemoveFinalizer(&binoculars, operatorFinalizer)
118-
if err := r.Update(ctx, &binoculars); err != nil {
119-
return ctrl.Result{}, err
120-
}
121-
}
122-
123-
// Stop reconciliation as the item is being deleted
124-
return ctrl.Result{}, nil
86+
cleanupF := func(ctx context.Context) error {
87+
return r.deleteExternalResources(ctx, components)
88+
}
89+
finish, err := checkAndHandleResourceDeletion(ctx, r.Client, &binoculars, operatorFinalizer, cleanupF, logger)
90+
if err != nil || finish {
91+
return ctrl.Result{}, err
12592
}
12693

12794
componentsCopy := components.DeepCopy()
@@ -131,58 +98,40 @@ func (r *BinocularsReconciler) Reconcile(ctx context.Context, req ctrl.Request)
13198
return nil
13299
}
133100

134-
if components.ServiceAccount != nil {
135-
logger.Info("Upserting Binoculars ServiceAccount object")
136-
if _, err := controllerutil.CreateOrUpdate(ctx, r.Client, components.ServiceAccount, mutateFn); err != nil {
137-
return ctrl.Result{}, err
138-
}
101+
if err := upsertObject(ctx, r.Client, components.ServiceAccount, binoculars.Kind, mutateFn, logger); err != nil {
102+
return ctrl.Result{}, err
139103
}
140104

141-
if components.ClusterRole != nil {
142-
logger.Info("Upserting Binoculars ClusterRole object")
143-
if _, err := controllerutil.CreateOrUpdate(ctx, r.Client, components.ClusterRole, mutateFn); err != nil {
144-
return ctrl.Result{}, err
145-
}
105+
if err := upsertObject(ctx, r.Client, components.ClusterRole, binoculars.Kind, mutateFn, logger); err != nil {
106+
return ctrl.Result{}, err
146107
}
147108

148-
if components.ClusterRoleBindings != nil && len(components.ClusterRoleBindings) > 0 {
149-
logger.Info("Upserting Binoculars ClusterRoleBinding object")
150-
if _, err := controllerutil.CreateOrUpdate(ctx, r.Client, components.ClusterRoleBindings[0], mutateFn); err != nil {
151-
return ctrl.Result{}, err
109+
if len(components.ClusterRoleBindings) > 0 {
110+
for _, crb := range components.ClusterRoleBindings {
111+
if err := upsertObject(ctx, r.Client, crb, binoculars.Kind, mutateFn, logger); err != nil {
112+
return ctrl.Result{}, err
113+
}
152114
}
153115
}
154116

155-
if components.Secret != nil {
156-
logger.Info("Upserting Binoculars Secret object")
157-
if _, err := controllerutil.CreateOrUpdate(ctx, r.Client, components.Secret, mutateFn); err != nil {
158-
return ctrl.Result{}, err
159-
}
117+
if err := upsertObject(ctx, r.Client, components.Secret, binoculars.Kind, mutateFn, logger); err != nil {
118+
return ctrl.Result{}, err
160119
}
161120

162-
if components.Deployment != nil {
163-
logger.Info("Upserting Binoculars Deployment object")
164-
if _, err := controllerutil.CreateOrUpdate(ctx, r.Client, components.Deployment, mutateFn); err != nil {
165-
return ctrl.Result{}, err
166-
}
121+
if err := upsertObject(ctx, r.Client, components.Deployment, binoculars.Kind, mutateFn, logger); err != nil {
122+
return ctrl.Result{}, err
167123
}
168124

169-
if components.Service != nil {
170-
logger.Info("Upserting Binoculars Service object")
171-
if _, err := controllerutil.CreateOrUpdate(ctx, r.Client, components.Service, mutateFn); err != nil {
172-
return ctrl.Result{}, err
173-
}
125+
if err := upsertObject(ctx, r.Client, components.Service, binoculars.Kind, mutateFn, logger); err != nil {
126+
return ctrl.Result{}, err
174127
}
175-
if components.IngressGrpc != nil {
176-
logger.Info("Upserting GRPC Ingress object")
177-
if _, err := controllerutil.CreateOrUpdate(ctx, r.Client, components.IngressGrpc, mutateFn); err != nil {
178-
return ctrl.Result{}, err
179-
}
128+
129+
if err := upsertObject(ctx, r.Client, components.IngressGrpc, binoculars.Kind, mutateFn, logger); err != nil {
130+
return ctrl.Result{}, err
180131
}
181-
if components.IngressHttp != nil {
182-
logger.Info("Upserting REST Ingress object")
183-
if _, err := controllerutil.CreateOrUpdate(ctx, r.Client, components.IngressHttp, mutateFn); err != nil {
184-
return ctrl.Result{}, err
185-
}
132+
133+
if err := upsertObject(ctx, r.Client, components.IngressHttp, binoculars.Kind, mutateFn, logger); err != nil {
134+
return ctrl.Result{}, err
186135
}
187136

188137
logger.Info("Successfully reconciled Binoculars object", "durationMillis", time.Since(started).Milliseconds())

‎internal/controller/install/binoculars_controller_test.go

+7-2
Original file line numberDiff line numberDiff line change
@@ -161,14 +161,19 @@ func TestBinocularsReconciler_Reconcile(t *testing.T) {
161161
t.Fatal("We should not fail on generating binoculars")
162162
}
163163

164+
// Binoculars
164165
mockK8sClient := k8sclient.NewMockClient(mockCtrl)
165166
mockK8sClient.
166167
EXPECT().
167168
Get(gomock.Any(), expectedNamespacedName, gomock.AssignableToTypeOf(&v1alpha1.Binoculars{})).
168169
Return(nil).
169170
SetArg(2, expectedBinoculars)
170-
// Binoculars finalizer
171-
mockK8sClient.EXPECT().Update(gomock.Any(), gomock.AssignableToTypeOf(&installv1alpha1.Binoculars{})).Return(nil)
171+
172+
// Finalizer
173+
mockK8sClient.
174+
EXPECT().
175+
Update(gomock.Any(), gomock.AssignableToTypeOf(&installv1alpha1.Binoculars{})).
176+
Return(nil)
172177

173178
mockK8sClient.
174179
EXPECT().

0 commit comments

Comments
 (0)