Skip to content

Commit 2879d36

Browse files
ajanikowajanikow
authored
[Bugfix] Fix ArangoMember management (#760)
1 parent ddcd758 commit 2879d36

File tree

7 files changed

+158
-46
lines changed

7 files changed

+158
-46
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# Change Log
22

33
## [master](https://github.com/arangodb/kube-arangodb/tree/master) (N/A)
4+
- Fix ArangoMember race with multiple ArangoDeployments within single namespace
45

56
## [1.2.0](https://github.com/arangodb/kube-arangodb/tree/1.2.0) (2021-07-16)
67
- Enable "Operator Internal Metrics Exporter" by default

‎pkg/apis/deployment/v1/arango_member_spec.go‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,15 @@
2222

2323
package v1
2424

25-
import core "k8s.io/api/core/v1"
25+
import (
26+
core "k8s.io/api/core/v1"
27+
"k8s.io/apimachinery/pkg/types"
28+
)
2629

2730
type ArangoMemberSpec struct {
28-
Group ServerGroup `json:"group,omitempty"`
29-
ID string `json:"id,omitempty"`
31+
Group ServerGroup `json:"group,omitempty"`
32+
ID string `json:"id,omitempty"`
33+
DeploymentUID types.UID `json:"deploymentUID,omitempty"`
3034

3135
Template *core.PodTemplate `json:"template,omitempty"`
3236
TemplateChecksum string `json:"templateChecksum,omitempty"`

‎pkg/deployment/resources/member_cleanup.go‎

Lines changed: 49 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ import (
2727
"context"
2828
"time"
2929

30+
"github.com/arangodb/kube-arangodb/pkg/util/k8sutil/inspector/arangomember"
31+
3032
inspectorInterface "github.com/arangodb/kube-arangodb/pkg/util/k8sutil/inspector"
3133
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3234

@@ -171,20 +173,26 @@ func (r *Resources) EnsureArangoMembers(ctx context.Context, cachedStatus inspec
171173
s, _ := r.context.GetStatus()
172174
obj := r.context.GetAPIObject()
173175

176+
reconcileRequired := k8sutil.NewReconcile()
177+
174178
if err := s.Members.ForeachServerGroup(func(group api.ServerGroup, list api.MemberStatusList) error {
175179
for _, member := range list {
176180
name := member.ArangoMemberName(r.context.GetAPIObject().GetName(), group)
177181

178-
if _, ok := cachedStatus.ArangoMember(name); !ok {
182+
if m, ok := cachedStatus.ArangoMember(name); !ok {
179183
// Create ArangoMember
180184
a := api.ArangoMember{
181185
ObjectMeta: metav1.ObjectMeta{
182186
Name: name,
183187
Namespace: r.context.GetNamespace(),
188+
OwnerReferences: []metav1.OwnerReference{
189+
obj.AsOwner(),
190+
},
184191
},
185192
Spec: api.ArangoMemberSpec{
186-
Group: group,
187-
ID: member.ID,
193+
Group: group,
194+
ID: member.ID,
195+
DeploymentUID: obj.GetUID(),
188196
},
189197
}
190198

@@ -196,7 +204,34 @@ func (r *Resources) EnsureArangoMembers(ctx context.Context, cachedStatus inspec
196204
return err
197205
}
198206

199-
return errors.Reconcile()
207+
reconcileRequired.Required()
208+
continue
209+
} else {
210+
changed := false
211+
if len(m.OwnerReferences) == 0 {
212+
m.OwnerReferences = []metav1.OwnerReference{
213+
obj.AsOwner(),
214+
}
215+
changed = true
216+
}
217+
218+
if m.Spec.DeploymentUID == "" {
219+
m.Spec.DeploymentUID = obj.GetUID()
220+
changed = true
221+
}
222+
if changed {
223+
224+
err := k8sutil.RunWithTimeout(ctx, func(ctxChild context.Context) error {
225+
_, err := r.context.GetArangoCli().DatabaseV1().ArangoMembers(obj.GetNamespace()).Update(ctxChild, m, metav1.UpdateOptions{})
226+
return err
227+
})
228+
if err != nil {
229+
return err
230+
}
231+
232+
reconcileRequired.Required()
233+
continue
234+
}
200235
}
201236
}
202237

@@ -205,6 +240,10 @@ func (r *Resources) EnsureArangoMembers(ctx context.Context, cachedStatus inspec
205240
return err
206241
}
207242

243+
if err := reconcileRequired.Reconcile(); err != nil {
244+
return err
245+
}
246+
208247
if err := cachedStatus.IterateArangoMembers(func(member *api.ArangoMember) error {
209248
_, g, ok := s.Members.ElementByID(member.Spec.ID)
210249

@@ -220,11 +259,15 @@ func (r *Resources) EnsureArangoMembers(ctx context.Context, cachedStatus inspec
220259
}
221260
}
222261

223-
return errors.Reconcile()
262+
reconcileRequired.Required()
224263
}
225264

226265
return nil
227-
}); err != nil {
266+
}, arangomember.FilterByDeploymentUID(obj.GetUID())); err != nil {
267+
return err
268+
}
269+
270+
if err := reconcileRequired.Reconcile(); err != nil {
228271
return err
229272
}
230273

‎pkg/deployment/resources/secrets.go‎

Lines changed: 14 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,11 @@ func (r *Resources) EnsureSecrets(ctx context.Context, log zerolog.Logger, cache
8787
defer metrics.SetDuration(inspectSecretsDurationGauges.WithLabelValues(deploymentName), start)
8888
counterMetric := inspectedSecretsCounters.WithLabelValues(deploymentName)
8989

90+
reconcileRequired := k8sutil.NewReconcile()
91+
9092
if spec.IsAuthenticated() {
9193
counterMetric.Inc()
92-
if err := r.refreshCache(ctx, cachedStatus, r.ensureTokenSecret(ctx, cachedStatus, secrets, spec.Authentication.GetJWTSecretName())); err != nil {
94+
if err := reconcileRequired.WithError(r.ensureTokenSecret(ctx, cachedStatus, secrets, spec.Authentication.GetJWTSecretName())); err != nil {
9395
return errors.WithStack(err)
9496
}
9597

@@ -103,23 +105,23 @@ func (r *Resources) EnsureSecrets(ctx context.Context, log zerolog.Logger, cache
103105

104106
if spec.Metrics.IsEnabled() {
105107
if imageFound && pod.VersionHasJWTSecretKeyfolder(image.ArangoDBVersion, image.Enterprise) {
106-
if err := r.refreshCache(ctx, cachedStatus, r.ensureExporterTokenSecret(ctx, cachedStatus, secrets, spec.Metrics.GetJWTTokenSecretName(), pod.JWTSecretFolder(deploymentName))); err != nil {
108+
if err := reconcileRequired.WithError(r.ensureExporterTokenSecret(ctx, cachedStatus, secrets, spec.Metrics.GetJWTTokenSecretName(), pod.JWTSecretFolder(deploymentName))); err != nil {
107109
return errors.WithStack(err)
108110
}
109111
} else {
110-
if err := r.refreshCache(ctx, cachedStatus, r.ensureExporterTokenSecret(ctx, cachedStatus, secrets, spec.Metrics.GetJWTTokenSecretName(), spec.Authentication.GetJWTSecretName())); err != nil {
112+
if err := reconcileRequired.WithError(r.ensureExporterTokenSecret(ctx, cachedStatus, secrets, spec.Metrics.GetJWTTokenSecretName(), spec.Authentication.GetJWTSecretName())); err != nil {
111113
return errors.WithStack(err)
112114
}
113115
}
114116
}
115117
}
116118
if spec.IsSecure() {
117119
counterMetric.Inc()
118-
if err := r.refreshCache(ctx, cachedStatus, r.ensureTLSCACertificateSecret(ctx, cachedStatus, secrets, spec.TLS)); err != nil {
120+
if err := reconcileRequired.WithError(r.ensureTLSCACertificateSecret(ctx, cachedStatus, secrets, spec.TLS)); err != nil {
119121
return errors.WithStack(err)
120122
}
121123

122-
if err := r.refreshCache(ctx, cachedStatus, r.ensureSecretWithEmptyKey(ctx, cachedStatus, secrets, GetCASecretName(r.context.GetAPIObject()), "empty")); err != nil {
124+
if err := reconcileRequired.WithError(r.ensureSecretWithEmptyKey(ctx, cachedStatus, secrets, GetCASecretName(r.context.GetAPIObject()), "empty")); err != nil {
123125
return errors.WithStack(err)
124126
}
125127

@@ -165,13 +167,9 @@ func (r *Resources) EnsureSecrets(ctx context.Context, log zerolog.Logger, cache
165167
}
166168
owner := member.AsOwner()
167169
errCert := createTLSServerCertificate(ctx, log, secrets, serverNames, spec.TLS, tlsKeyfileSecretName, &owner)
168-
if err := r.refreshCache(ctx, cachedStatus, errCert); err != nil && !k8sutil.IsAlreadyExists(err) {
170+
if err := reconcileRequired.WithError(errCert); err != nil && !k8sutil.IsAlreadyExists(err) {
169171
return errors.WithStack(errors.Wrapf(err, "Failed to create TLS keyfile secret"))
170172
}
171-
172-
if err := r.refreshCache(ctx, cachedStatus, operatorErrors.Reconcile()); err != nil {
173-
return errors.WithStack(err)
174-
}
175173
}
176174
}
177175
return nil
@@ -181,47 +179,30 @@ func (r *Resources) EnsureSecrets(ctx context.Context, log zerolog.Logger, cache
181179
}
182180
if spec.RocksDB.IsEncrypted() {
183181
if i := status.CurrentImage; i != nil && features.EncryptionRotation().Supported(i.ArangoDBVersion, i.Enterprise) {
184-
if err := r.refreshCache(ctx, cachedStatus, r.ensureEncryptionKeyfolderSecret(ctx, cachedStatus, secrets, spec.RocksDB.Encryption.GetKeySecretName(), pod.GetEncryptionFolderSecretName(deploymentName))); err != nil {
182+
if err := reconcileRequired.WithError(r.ensureEncryptionKeyfolderSecret(ctx, cachedStatus, secrets, spec.RocksDB.Encryption.GetKeySecretName(), pod.GetEncryptionFolderSecretName(deploymentName))); err != nil {
185183
return errors.WithStack(err)
186184
}
187185
}
188186
}
189187
if spec.Sync.IsEnabled() {
190188
counterMetric.Inc()
191-
if err := r.refreshCache(ctx, cachedStatus, r.ensureTokenSecret(ctx, cachedStatus, secrets, spec.Sync.Authentication.GetJWTSecretName())); err != nil {
189+
if err := reconcileRequired.WithError(r.ensureTokenSecret(ctx, cachedStatus, secrets, spec.Sync.Authentication.GetJWTSecretName())); err != nil {
192190
return errors.WithStack(err)
193191
}
194192
counterMetric.Inc()
195-
if err := r.refreshCache(ctx, cachedStatus, r.ensureTokenSecret(ctx, cachedStatus, secrets, spec.Sync.Monitoring.GetTokenSecretName())); err != nil {
193+
if err := reconcileRequired.WithError(r.ensureTokenSecret(ctx, cachedStatus, secrets, spec.Sync.Monitoring.GetTokenSecretName())); err != nil {
196194
return errors.WithStack(err)
197195
}
198196
counterMetric.Inc()
199-
if err := r.refreshCache(ctx, cachedStatus, r.ensureTLSCACertificateSecret(ctx, cachedStatus, secrets, spec.Sync.TLS)); err != nil {
197+
if err := reconcileRequired.WithError(r.ensureTLSCACertificateSecret(ctx, cachedStatus, secrets, spec.Sync.TLS)); err != nil {
200198
return errors.WithStack(err)
201199
}
202200
counterMetric.Inc()
203-
if err := r.refreshCache(ctx, cachedStatus, r.ensureClientAuthCACertificateSecret(ctx, cachedStatus, secrets, spec.Sync.Authentication)); err != nil {
201+
if err := reconcileRequired.WithError(r.ensureClientAuthCACertificateSecret(ctx, cachedStatus, secrets, spec.Sync.Authentication)); err != nil {
204202
return errors.WithStack(err)
205203
}
206204
}
207-
return nil
208-
}
209-
210-
func (r *Resources) refreshCache(ctx context.Context, cachedStatus inspectorInterface.Inspector, err error) error {
211-
if err == nil {
212-
return nil
213-
}
214-
215-
if operatorErrors.IsReconcile(err) {
216-
err := cachedStatus.Refresh(ctx, r.context.GetKubeCli(), r.context.GetMonitoringV1Cli(), r.context.GetArangoCli(), r.context.GetNamespace())
217-
if err != nil {
218-
return errors.WithStack(err)
219-
}
220-
} else {
221-
return errors.WithStack(err)
222-
}
223-
224-
return nil
205+
return reconcileRequired.Reconcile()
225206
}
226207

227208
func (r *Resources) ensureTokenSecretFolder(ctx context.Context, cachedStatus inspectorInterface.Inspector, secrets k8sutil.SecretInterface, secretName, folderSecretName string) error {

‎pkg/deployment/resources/services.go‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,8 @@ func (r *Resources) EnsureServices(ctx context.Context, cachedStatus inspectorIn
6565
// Fetch existing services
6666
svcs := kubecli.CoreV1().Services(ns)
6767

68+
reconcileRequired := k8sutil.NewReconcile()
69+
6870
// Ensure member services
6971
if err := status.Members.ForeachServerGroup(func(group api.ServerGroup, list api.MemberStatusList) error {
7072
for _, m := range list {
@@ -109,7 +111,8 @@ func (r *Resources) EnsureServices(ctx context.Context, cachedStatus inspectorIn
109111
}
110112
}
111113

112-
return errors.Reconcile()
114+
reconcileRequired.Required()
115+
continue
113116
} else {
114117
spec := s.Spec.DeepCopy()
115118

@@ -136,7 +139,8 @@ func (r *Resources) EnsureServices(ctx context.Context, cachedStatus inspectorIn
136139
return err
137140
}
138141

139-
return errors.Reconcile()
142+
reconcileRequired.Required()
143+
continue
140144
}
141145
}
142146
}
@@ -229,7 +233,8 @@ func (r *Resources) EnsureServices(ctx context.Context, cachedStatus inspectorIn
229233
}
230234
}
231235
}
232-
return nil
236+
237+
return reconcileRequired.Reconcile()
233238
}
234239

235240
// EnsureServices creates all services needed to service the deployment

‎pkg/util/k8sutil/inspector/arangomember/member.go‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,10 @@
2222

2323
package arangomember
2424

25-
import api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1"
25+
import (
26+
api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1"
27+
"k8s.io/apimachinery/pkg/types"
28+
)
2629

2730
type Inspector interface {
2831
ArangoMember(name string) (*api.ArangoMember, bool)
@@ -31,3 +34,9 @@ type Inspector interface {
3134

3235
type ArangoMemberFilter func(pod *api.ArangoMember) bool
3336
type ArangoMemberAction func(pod *api.ArangoMember) error
37+
38+
func FilterByDeploymentUID(uid types.UID) ArangoMemberFilter {
39+
return func(pod *api.ArangoMember) bool {
40+
return pod.Spec.DeploymentUID == "" || pod.Spec.DeploymentUID == uid
41+
}
42+
}

‎pkg/util/k8sutil/reconcile.go‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
//
2+
// DISCLAIMER
3+
//
4+
// Copyright 2021 ArangoDB GmbH, Cologne, Germany
5+
//
6+
// Licensed under the Apache License, Version 2.0 (the "License");
7+
// you may not use this file except in compliance with the License.
8+
// You may obtain a copy of the License at
9+
//
10+
// http://www.apache.org/licenses/LICENSE-2.0
11+
//
12+
// Unless required by applicable law or agreed to in writing, software
13+
// distributed under the License is distributed on an "AS IS" BASIS,
14+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
// See the License for the specific language governing permissions and
16+
// limitations under the License.
17+
//
18+
// Copyright holder is ArangoDB GmbH, Cologne, Germany
19+
//
20+
// Author Adam Janikowski
21+
//
22+
23+
package k8sutil
24+
25+
import "github.com/arangodb/kube-arangodb/pkg/util/errors"
26+
27+
func NewReconcile() Reconcile {
28+
return &reconcile{}
29+
}
30+
31+
type Reconcile interface {
32+
Reconcile() error
33+
Required()
34+
IsRequired() bool
35+
WithError(err error) error
36+
}
37+
38+
type reconcile struct {
39+
required bool
40+
}
41+
42+
func (r *reconcile) Reconcile() error {
43+
if r.required {
44+
return errors.Reconcile()
45+
}
46+
47+
return nil
48+
}
49+
50+
func (r *reconcile) Required() {
51+
r.required = true
52+
}
53+
54+
func (r *reconcile) IsRequired() bool {
55+
return r.required
56+
}
57+
58+
func (r *reconcile) WithError(err error) error {
59+
if err == nil {
60+
return nil
61+
}
62+
63+
if errors.IsReconcile(err) {
64+
r.Required()
65+
return nil
66+
}
67+
68+
return err
69+
}

0 commit comments

Comments
 (0)