Skip to content

Commit 887a8b5

Browse files
aledbfaledbf
authored
Avoid ws-manager-mk2 metrics duplication (#18625)
1 parent 7909192 commit 887a8b5

File tree

4 files changed

+63
-80
lines changed

4 files changed

+63
-80
lines changed

‎components/ws-manager-mk2/controllers/metrics.go

Lines changed: 7 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -139,24 +139,14 @@ func (m *controllerMetrics) countWorkspaceStartFailures(log *logr.Logger, ws *wo
139139
class := ws.Spec.Class
140140
tpe := string(ws.Spec.Type)
141141

142-
counter, err := m.totalStartsFailureCounterVec.GetMetricWithLabelValues(tpe, class)
143-
if err != nil {
144-
log.Error(err, "could not count workspace startup failure", "type", tpe, "class", class)
145-
}
146-
147-
counter.Inc()
142+
m.totalStartsFailureCounterVec.WithLabelValues(tpe, class).Inc()
148143
}
149144

150145
func (m *controllerMetrics) countWorkspaceFailure(log *logr.Logger, ws *workspacev1.Workspace) {
151146
class := ws.Spec.Class
152147
tpe := string(ws.Spec.Type)
153148

154-
counter, err := m.totalFailuresCounterVec.GetMetricWithLabelValues(tpe, class)
155-
if err != nil {
156-
log.Error(err, "could not count workspace failure", "type", tpe, "class", class)
157-
}
158-
159-
counter.Inc()
149+
m.totalFailuresCounterVec.WithLabelValues(tpe, class).Inc()
160150
}
161151

162152
func (m *controllerMetrics) countWorkspaceStop(log *logr.Logger, ws *workspacev1.Workspace) {
@@ -182,60 +172,35 @@ func (m *controllerMetrics) countWorkspaceStop(log *logr.Logger, ws *workspacev1
182172
class := ws.Spec.Class
183173
tpe := string(ws.Spec.Type)
184174

185-
counter, err := m.totalStopsCounterVec.GetMetricWithLabelValues(reason, tpe, class)
186-
if err != nil {
187-
log.Error(err, "could not count workspace stop", "reason", "unknown", "type", tpe, "class", class)
188-
}
189-
190-
counter.Inc()
175+
m.totalStopsCounterVec.WithLabelValues(reason, tpe, class).Inc()
191176
}
192177

193178
func (m *controllerMetrics) countTotalBackups(log *logr.Logger, ws *workspacev1.Workspace) {
194179
class := ws.Spec.Class
195180
tpe := string(ws.Spec.Type)
196181

197-
counter, err := m.totalBackupCounterVec.GetMetricWithLabelValues(tpe, class)
198-
if err != nil {
199-
log.Error(err, "could not count workspace backup", "type", tpe, "class", class)
200-
}
201-
202-
counter.Inc()
182+
m.totalBackupCounterVec.WithLabelValues(tpe, class).Inc()
203183
}
204184

205185
func (m *controllerMetrics) countTotalBackupFailures(log *logr.Logger, ws *workspacev1.Workspace) {
206186
class := ws.Spec.Class
207187
tpe := string(ws.Spec.Type)
208188

209-
counter, err := m.totalBackupFailureCounterVec.GetMetricWithLabelValues(tpe, class)
210-
if err != nil {
211-
log.Error(err, "could not count workspace backup failure", "type", tpe, "class", class)
212-
}
213-
214-
counter.Inc()
189+
m.totalBackupFailureCounterVec.WithLabelValues(tpe, class).Inc()
215190
}
216191

217192
func (m *controllerMetrics) countTotalRestores(log *logr.Logger, ws *workspacev1.Workspace) {
218193
class := ws.Spec.Class
219194
tpe := string(ws.Spec.Type)
220195

221-
counter, err := m.totalRestoreCounterVec.GetMetricWithLabelValues(tpe, class)
222-
if err != nil {
223-
log.Error(err, "could not count workspace restore", "type", tpe, "class", class)
224-
}
225-
226-
counter.Inc()
196+
m.totalRestoreCounterVec.WithLabelValues(tpe, class).Inc()
227197
}
228198

229199
func (m *controllerMetrics) countTotalRestoreFailures(log *logr.Logger, ws *workspacev1.Workspace) {
230200
class := ws.Spec.Class
231201
tpe := string(ws.Spec.Type)
232202

233-
counter, err := m.totalRestoreFailureCounterVec.GetMetricWithLabelValues(tpe, class)
234-
if err != nil {
235-
log.Error(err, "could not count workspace restore failure", "type", tpe, "class", class)
236-
}
237-
238-
counter.Inc()
203+
m.totalRestoreFailureCounterVec.WithLabelValues(tpe, class).Inc()
239204
}
240205

241206
func (m *controllerMetrics) containsWorkspace(ws *workspacev1.Workspace) bool {

‎components/ws-manager-mk2/controllers/suite_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,8 @@ var _ = BeforeSuite(func() {
102102
})
103103
Expect(err).ToNot(HaveOccurred())
104104

105+
SetupIndexer(k8sManager)
106+
105107
conf := newTestConfig()
106108
maintenance := &fakeMaintenance{enabled: false}
107109
wsReconciler, err := NewWorkspaceReconciler(k8sManager.GetClient(), k8sManager.GetScheme(), k8sManager.GetEventRecorderFor("workspace"), &conf, metrics.Registry, maintenance)

‎components/ws-manager-mk2/controllers/workspace_controller.go

Lines changed: 35 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,11 @@ import (
88
"context"
99
"fmt"
1010
"strings"
11+
"sync"
1112
"time"
1213

1314
corev1 "k8s.io/api/core/v1"
1415
"k8s.io/apimachinery/pkg/api/equality"
15-
"k8s.io/apimachinery/pkg/api/errors"
1616
apierrors "k8s.io/apimachinery/pkg/api/errors"
1717
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1818
"k8s.io/apimachinery/pkg/runtime"
@@ -95,7 +95,7 @@ func (r *WorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
9595

9696
var workspace workspacev1.Workspace
9797
if err := r.Get(ctx, req.NamespacedName, &workspace); err != nil {
98-
if !errors.IsNotFound(err) {
98+
if !apierrors.IsNotFound(err) {
9999
log.Error(err, "unable to fetch workspace")
100100
}
101101
// we'll ignore not-found errors, since they can't be fixed by an immediate
@@ -184,7 +184,7 @@ func (r *WorkspaceReconciler) actOnStatus(ctx context.Context, workspace *worksp
184184
}
185185

186186
err = r.Create(ctx, pod)
187-
if errors.IsAlreadyExists(err) {
187+
if apierrors.IsAlreadyExists(err) {
188188
// pod exists, we're good
189189
} else if err != nil {
190190
log.Error(err, "unable to create Pod for Workspace", "pod", pod)
@@ -257,7 +257,7 @@ func (r *WorkspaceReconciler) actOnStatus(ctx context.Context, workspace *worksp
257257
err := r.Client.Delete(ctx, pod, &client.DeleteOptions{
258258
GracePeriodSeconds: gracePeriodSeconds,
259259
})
260-
if errors.IsNotFound(err) {
260+
if apierrors.IsNotFound(err) {
261261
// pod is gone - nothing to do here
262262
} else {
263263
return ctrl.Result{Requeue: true}, err
@@ -282,7 +282,7 @@ func (r *WorkspaceReconciler) actOnStatus(ctx context.Context, workspace *worksp
282282
// Workspace was requested to be deleted, propagate by deleting the Pod.
283283
// The Pod deletion will then trigger workspace disposal steps.
284284
err := r.Client.Delete(ctx, pod)
285-
if errors.IsNotFound(err) {
285+
if apierrors.IsNotFound(err) {
286286
// pod is gone - nothing to do here
287287
} else {
288288
return ctrl.Result{Requeue: true}, err
@@ -396,7 +396,7 @@ func (r *WorkspaceReconciler) deleteWorkspacePod(ctx context.Context, pod *corev
396396
// Workspace was requested to be deleted, propagate by deleting the Pod.
397397
// The Pod deletion will then trigger workspace disposal steps.
398398
err := r.Client.Delete(ctx, pod)
399-
if errors.IsNotFound(err) {
399+
if apierrors.IsNotFound(err) {
400400
// pod is gone - nothing to do here
401401
} else {
402402
return ctrl.Result{Requeue: true}, err
@@ -441,7 +441,7 @@ func (r *WorkspaceReconciler) deleteSecret(ctx context.Context, name, namespace
441441
}, func(ctx context.Context) (bool, error) {
442442
var secret corev1.Secret
443443
err := r.Client.Get(ctx, types.NamespacedName{Name: name, Namespace: namespace}, &secret)
444-
if errors.IsNotFound(err) {
444+
if apierrors.IsNotFound(err) {
445445
// nothing to delete
446446
return true, nil
447447
}
@@ -452,7 +452,7 @@ func (r *WorkspaceReconciler) deleteSecret(ctx context.Context, name, namespace
452452
}
453453

454454
err = r.Client.Delete(ctx, &secret)
455-
if err != nil && !errors.IsNotFound(err) {
455+
if err != nil && !apierrors.IsNotFound(err) {
456456
log.Error(err, "cannot delete secret", "secret", name)
457457
return false, nil
458458
}
@@ -468,7 +468,7 @@ func (r *WorkspaceReconciler) deleteSecret(ctx context.Context, name, namespace
468468
// For conflicts, instead a result with `Requeue: true` is returned, which has the same requeuing
469469
// behaviour as returning an error.
470470
func errorResultLogConflict(log logr.Logger, err error) (ctrl.Result, error) {
471-
if errors.IsConflict(err) {
471+
if apierrors.IsConflict(err) {
472472
return ctrl.Result{RequeueAfter: 100 * time.Millisecond}, nil
473473
} else {
474474
return ctrl.Result{}, err
@@ -482,26 +482,6 @@ var (
482482

483483
// SetupWithManager sets up the controller with the Manager.
484484
func (r *WorkspaceReconciler) SetupWithManager(mgr ctrl.Manager) error {
485-
idx := func(rawObj client.Object) []string {
486-
// grab the job object, extract the owner...
487-
job := rawObj.(*corev1.Pod)
488-
owner := metav1.GetControllerOf(job)
489-
if owner == nil {
490-
return nil
491-
}
492-
// ...make sure it's a workspace...
493-
if owner.APIVersion != apiGVStr || owner.Kind != "Workspace" {
494-
return nil
495-
}
496-
497-
// ...and if so, return it
498-
return []string{owner.Name}
499-
}
500-
err := mgr.GetFieldIndexer().IndexField(context.Background(), &corev1.Pod{}, wsOwnerKey, idx)
501-
if err != nil {
502-
return err
503-
}
504-
505485
return ctrl.NewControllerManagedBy(mgr).
506486
Named("workspace").
507487
WithOptions(controller.Options{
@@ -538,3 +518,29 @@ func (r *WorkspaceReconciler) SetupWithManager(mgr ctrl.Manager) error {
538518
}).
539519
Complete(r)
540520
}
521+
522+
func SetupIndexer(mgr ctrl.Manager) error {
523+
var err error
524+
var once sync.Once
525+
once.Do(func() {
526+
idx := func(rawObj client.Object) []string {
527+
// grab the job object, extract the owner...
528+
job := rawObj.(*corev1.Pod)
529+
owner := metav1.GetControllerOf(job)
530+
if owner == nil {
531+
return nil
532+
}
533+
// ...make sure it's a workspace...
534+
if owner.APIVersion != apiGVStr || owner.Kind != "Workspace" {
535+
return nil
536+
}
537+
538+
// ...and if so, return it
539+
return []string{owner.Name}
540+
}
541+
542+
err = mgr.GetFieldIndexer().IndexField(context.Background(), &corev1.Pod{}, wsOwnerKey, idx)
543+
})
544+
545+
return err
546+
}

‎components/ws-manager-mk2/main.go

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -143,13 +143,6 @@ func main() {
143143
os.Exit(1)
144144
}
145145

146-
workspaceReconciler, err := controllers.NewWorkspaceReconciler(
147-
mgr.GetClient(), mgr.GetScheme(), mgr.GetEventRecorderFor("workspace"), &cfg.Manager, metrics.Registry, maintenanceReconciler)
148-
if err != nil {
149-
setupLog.Error(err, "unable to create controller", "controller", "Workspace")
150-
os.Exit(1)
151-
}
152-
153146
timeoutReconciler, err := controllers.NewTimeoutReconciler(mgr.GetClient(), mgr.GetEventRecorderFor("workspace"), cfg.Manager, maintenanceReconciler)
154147
if err != nil {
155148
setupLog.Error(err, "unable to create timeout controller", "controller", "Timeout")
@@ -175,11 +168,28 @@ func main() {
175168
os.Exit(1)
176169
}
177170

178-
if err = workspaceReconciler.SetupWithManager(mgr); err != nil {
179-
setupLog.Error(err, "unable to setup workspace controller with manager", "controller", "Workspace")
171+
err = controllers.SetupIndexer(mgr)
172+
if err != nil {
173+
setupLog.Error(err, "unable to configure field indexer")
180174
os.Exit(1)
181175
}
182176

177+
go func() {
178+
<-mgr.Elected()
179+
180+
workspaceReconciler, err := controllers.NewWorkspaceReconciler(
181+
mgr.GetClient(), mgr.GetScheme(), mgr.GetEventRecorderFor("workspace"), &cfg.Manager, metrics.Registry, maintenanceReconciler)
182+
if err != nil {
183+
setupLog.Error(err, "unable to create controller", "controller", "Workspace")
184+
os.Exit(1)
185+
}
186+
187+
if err = workspaceReconciler.SetupWithManager(mgr); err != nil {
188+
setupLog.Error(err, "unable to setup workspace controller with manager", "controller", "Workspace")
189+
os.Exit(1)
190+
}
191+
}()
192+
183193
if err = timeoutReconciler.SetupWithManager(mgr); err != nil {
184194
setupLog.Error(err, "unable to setup timeout controller with manager", "controller", "Timeout")
185195
os.Exit(1)

0 commit comments

Comments
 (0)