Skip to content

Commit f9e6d05

Browse files
stefanprodanstefanprodan
authored
Merge pull request #937 from fluxcd/delete-suspended
Allow deleting suspended objects
2 parents 663b6a7 + 15cdd85 commit f9e6d05

15 files changed

+346
-147
lines changed

‎controllers/bucket_controller.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -258,12 +258,6 @@ func (r *BucketReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res
258258
// Record suspended status metric
259259
r.RecordSuspend(ctx, obj, obj.Spec.Suspend)
260260

261-
// Return early if the object is suspended
262-
if obj.Spec.Suspend {
263-
log.Info("reconciliation is suspended for this object")
264-
return ctrl.Result{}, nil
265-
}
266-
267261
// Initialize the patch helper with the current version of the object.
268262
patchHelper, err := patch.NewHelper(obj, r.Client)
269263
if err != nil {
@@ -309,6 +303,13 @@ func (r *BucketReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res
309303
return
310304
}
311305

306+
// Return if the object is suspended.
307+
if obj.Spec.Suspend {
308+
log.Info("reconciliation is suspended for this object")
309+
recResult, retErr = sreconcile.ResultEmpty, nil
310+
return
311+
}
312+
312313
// Reconcile actual object
313314
reconcilers := []bucketReconcileFunc{
314315
r.reconcileStorage,

‎controllers/bucket_controller_test.go

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ import (
3030

3131
. "github.com/onsi/gomega"
3232
corev1 "k8s.io/api/core/v1"
33-
apierrors "k8s.io/apimachinery/pkg/api/errors"
3433
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3534
"k8s.io/client-go/tools/record"
3635
kstatus "sigs.k8s.io/cli-utils/pkg/kstatus/status"
@@ -85,7 +84,7 @@ func TestBucketReconciler_Reconcile(t *testing.T) {
8584
g.Expect(testEnv.Create(ctx, secret)).To(Succeed())
8685
defer testEnv.Delete(ctx, secret)
8786

88-
obj := &sourcev1.Bucket{
87+
origObj := &sourcev1.Bucket{
8988
ObjectMeta: metav1.ObjectMeta{
9089
GenerateName: "bucket-reconcile-",
9190
Namespace: "default",
@@ -102,6 +101,7 @@ func TestBucketReconciler_Reconcile(t *testing.T) {
102101
},
103102
},
104103
}
104+
obj := origObj.DeepCopy()
105105
g.Expect(testEnv.Create(ctx, obj)).To(Succeed())
106106

107107
key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace}
@@ -115,17 +115,7 @@ func TestBucketReconciler_Reconcile(t *testing.T) {
115115
}, timeout).Should(BeTrue())
116116

117117
// Wait for Bucket to be Ready
118-
g.Eventually(func() bool {
119-
if err := testEnv.Get(ctx, key, obj); err != nil {
120-
return false
121-
}
122-
if !conditions.IsReady(obj) || obj.Status.Artifact == nil {
123-
return false
124-
}
125-
readyCondition := conditions.Get(obj, meta.ReadyCondition)
126-
return obj.Generation == readyCondition.ObservedGeneration &&
127-
obj.Generation == obj.Status.ObservedGeneration
128-
}, timeout).Should(BeTrue())
118+
waitForSourceReadyWithArtifact(ctx, g, obj)
129119

130120
// Check if the object status is valid.
131121
condns := &conditionscheck.Conditions{NegativePolarity: bucketReadyCondition.NegativePolarity}
@@ -157,12 +147,11 @@ func TestBucketReconciler_Reconcile(t *testing.T) {
157147
g.Expect(testEnv.Delete(ctx, obj)).To(Succeed())
158148

159149
// Wait for Bucket to be deleted
160-
g.Eventually(func() bool {
161-
if err := testEnv.Get(ctx, key, obj); err != nil {
162-
return apierrors.IsNotFound(err)
163-
}
164-
return false
165-
}, timeout).Should(BeTrue())
150+
waitForSourceDeletion(ctx, g, obj)
151+
152+
// Check if a suspended object gets deleted.
153+
obj = origObj.DeepCopy()
154+
testSuspendedObjectDeleteWithArtifact(ctx, g, obj)
166155
}
167156

168157
func TestBucketReconciler_reconcileStorage(t *testing.T) {

‎controllers/common_test.go

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
/*
2+
Copyright 2022 The Flux authors
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 controllers
18+
19+
import (
20+
"context"
21+
22+
. "github.com/onsi/gomega"
23+
apierrors "k8s.io/apimachinery/pkg/api/errors"
24+
"sigs.k8s.io/controller-runtime/pkg/client"
25+
26+
"github.com/fluxcd/pkg/apis/meta"
27+
"github.com/fluxcd/pkg/runtime/conditions"
28+
"github.com/fluxcd/pkg/runtime/patch"
29+
30+
"github.com/fluxcd/source-controller/internal/object"
31+
)
32+
33+
// waitForSourceDeletion is a generic test helper to wait for object deletion of
34+
// any source kind.
35+
func waitForSourceDeletion(ctx context.Context, g *WithT, obj conditions.Setter) {
36+
g.THelper()
37+
38+
key := client.ObjectKeyFromObject(obj)
39+
g.Eventually(func() bool {
40+
if err := testEnv.Get(ctx, key, obj); err != nil {
41+
return apierrors.IsNotFound(err)
42+
}
43+
return false
44+
}, timeout).Should(BeTrue())
45+
}
46+
47+
// waitForSuspended is a generic test helper to wait for object to be suspended
48+
// of any source kind.
49+
func waitForSuspended(ctx context.Context, g *WithT, obj conditions.Setter) {
50+
g.THelper()
51+
52+
key := client.ObjectKeyFromObject(obj)
53+
g.Eventually(func() bool {
54+
if err := testEnv.Get(ctx, key, obj); err != nil {
55+
return false
56+
}
57+
suspended, err := object.GetSuspend(obj)
58+
if err != nil {
59+
return false
60+
}
61+
return suspended == true
62+
}, timeout).Should(BeTrue())
63+
}
64+
65+
// waitForSourceReadyWithArtifact is a generic test helper to wait for an object
66+
// to be ready of any source kind that have artifact in status when ready.
67+
func waitForSourceReadyWithArtifact(ctx context.Context, g *WithT, obj conditions.Setter) {
68+
g.THelper()
69+
waitForSourceReady(ctx, g, obj, true)
70+
}
71+
72+
// waitForSourceReadyWithoutArtifact is a generic test helper to wait for an object
73+
// to be ready of any source kind that don't have artifact in status when ready.
74+
func waitForSourceReadyWithoutArtifact(ctx context.Context, g *WithT, obj conditions.Setter) {
75+
g.THelper()
76+
waitForSourceReady(ctx, g, obj, false)
77+
}
78+
79+
// waitForSourceReady is a generic test helper to wait for an object to be
80+
// ready of any source kind.
81+
func waitForSourceReady(ctx context.Context, g *WithT, obj conditions.Setter, withArtifact bool) {
82+
g.THelper()
83+
84+
key := client.ObjectKeyFromObject(obj)
85+
g.Eventually(func() bool {
86+
if err := testEnv.Get(ctx, key, obj); err != nil {
87+
return false
88+
}
89+
if withArtifact {
90+
artifact, err := object.GetArtifact(obj)
91+
if err != nil {
92+
return false
93+
}
94+
if artifact == nil {
95+
return false
96+
}
97+
}
98+
if !conditions.IsReady(obj) {
99+
return false
100+
}
101+
readyCondition := conditions.Get(obj, meta.ReadyCondition)
102+
statusObservedGen, err := object.GetStatusObservedGeneration(obj)
103+
if err != nil {
104+
return false
105+
}
106+
return obj.GetGeneration() == readyCondition.ObservedGeneration &&
107+
obj.GetGeneration() == statusObservedGen
108+
}, timeout).Should(BeTrue())
109+
}
110+
111+
// testSuspendedObjectDeleteWithArtifact is a generic test helper to test if a
112+
// suspended object can be deleted for objects that have artifact in status when
113+
// ready.
114+
func testSuspendedObjectDeleteWithArtifact(ctx context.Context, g *WithT, obj conditions.Setter) {
115+
g.THelper()
116+
testSuspendedObjectDelete(ctx, g, obj, true)
117+
}
118+
119+
// testSuspendedObjectDeleteWithoutArtifact is a generic test helper to test if
120+
// a suspended object can be deleted for objects that don't have artifact in
121+
// status when ready.
122+
func testSuspendedObjectDeleteWithoutArtifact(ctx context.Context, g *WithT, obj conditions.Setter) {
123+
g.THelper()
124+
testSuspendedObjectDelete(ctx, g, obj, false)
125+
}
126+
127+
// testSuspendedObjectDelete is a generic test helper to test if a suspended
128+
// object can be deleted.
129+
func testSuspendedObjectDelete(ctx context.Context, g *WithT, obj conditions.Setter, withArtifact bool) {
130+
g.THelper()
131+
132+
// Create the object and wait for it to be ready.
133+
g.Expect(testEnv.Create(ctx, obj)).To(Succeed())
134+
waitForSourceReady(ctx, g, obj, withArtifact)
135+
136+
// Suspend the object.
137+
patchHelper, err := patch.NewHelper(obj, testEnv.Client)
138+
g.Expect(err).ToNot(HaveOccurred())
139+
g.Expect(object.SetSuspend(obj, true)).ToNot(HaveOccurred())
140+
g.Expect(patchHelper.Patch(ctx, obj)).ToNot(HaveOccurred())
141+
waitForSuspended(ctx, g, obj)
142+
143+
// Delete the object.
144+
g.Expect(testEnv.Delete(ctx, obj)).To(Succeed())
145+
waitForSourceDeletion(ctx, g, obj)
146+
}

‎controllers/gitrepository_controller.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -172,12 +172,6 @@ func (r *GitRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reques
172172
// Record suspended status metric
173173
r.RecordSuspend(ctx, obj, obj.Spec.Suspend)
174174

175-
// Return early if the object is suspended
176-
if obj.Spec.Suspend {
177-
log.Info("reconciliation is suspended for this object")
178-
return ctrl.Result{}, nil
179-
}
180-
181175
// Initialize the patch helper with the current version of the object.
182176
patchHelper, err := patch.NewHelper(obj, r.Client)
183177
if err != nil {
@@ -225,6 +219,13 @@ func (r *GitRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reques
225219
return
226220
}
227221

222+
// Return if the object is suspended.
223+
if obj.Spec.Suspend {
224+
log.Info("reconciliation is suspended for this object")
225+
recResult, retErr = sreconcile.ResultEmpty, nil
226+
return
227+
}
228+
228229
// Reconcile actual object
229230
reconcilers := []gitRepositoryReconcileFunc{
230231
r.reconcileStorage,

‎controllers/gitrepository_controller_test.go

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ import (
3737
. "github.com/onsi/gomega"
3838
sshtestdata "golang.org/x/crypto/ssh/testdata"
3939
corev1 "k8s.io/api/core/v1"
40-
apierrors "k8s.io/apimachinery/pkg/api/errors"
4140
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
4241
"k8s.io/apimachinery/pkg/runtime"
4342
"k8s.io/client-go/tools/record"
@@ -168,7 +167,7 @@ func TestGitRepositoryReconciler_Reconcile(t *testing.T) {
168167
_, err = initGitRepo(server, "testdata/git/repository", git.DefaultBranch, repoPath)
169168
g.Expect(err).NotTo(HaveOccurred())
170169

171-
obj := &sourcev1.GitRepository{
170+
origObj := &sourcev1.GitRepository{
172171
ObjectMeta: metav1.ObjectMeta{
173172
GenerateName: "gitrepository-reconcile-",
174173
Namespace: "default",
@@ -178,6 +177,7 @@ func TestGitRepositoryReconciler_Reconcile(t *testing.T) {
178177
URL: server.HTTPAddress() + repoPath,
179178
},
180179
}
180+
obj := origObj.DeepCopy()
181181
g.Expect(testEnv.Create(ctx, obj)).To(Succeed())
182182

183183
key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace}
@@ -191,17 +191,7 @@ func TestGitRepositoryReconciler_Reconcile(t *testing.T) {
191191
}, timeout).Should(BeTrue())
192192

193193
// Wait for GitRepository to be Ready
194-
g.Eventually(func() bool {
195-
if err := testEnv.Get(ctx, key, obj); err != nil {
196-
return false
197-
}
198-
if !conditions.IsReady(obj) || obj.Status.Artifact == nil {
199-
return false
200-
}
201-
readyCondition := conditions.Get(obj, meta.ReadyCondition)
202-
return obj.Generation == readyCondition.ObservedGeneration &&
203-
obj.Generation == obj.Status.ObservedGeneration
204-
}, timeout).Should(BeTrue())
194+
waitForSourceReadyWithArtifact(ctx, g, obj)
205195

206196
// Check if the object status is valid.
207197
condns := &conditionscheck.Conditions{NegativePolarity: gitRepositoryReadyCondition.NegativePolarity}
@@ -233,12 +223,11 @@ func TestGitRepositoryReconciler_Reconcile(t *testing.T) {
233223
g.Expect(testEnv.Delete(ctx, obj)).To(Succeed())
234224

235225
// Wait for GitRepository to be deleted
236-
g.Eventually(func() bool {
237-
if err := testEnv.Get(ctx, key, obj); err != nil {
238-
return apierrors.IsNotFound(err)
239-
}
240-
return false
241-
}, timeout).Should(BeTrue())
226+
waitForSourceDeletion(ctx, g, obj)
227+
228+
// Check if a suspended object gets deleted.
229+
obj = origObj.DeepCopy()
230+
testSuspendedObjectDeleteWithArtifact(ctx, g, obj)
242231
}
243232

244233
func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) {

‎controllers/helmchart_controller.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -194,12 +194,6 @@ func (r *HelmChartReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
194194
// Record suspended status metric
195195
r.RecordSuspend(ctx, obj, obj.Spec.Suspend)
196196

197-
// Return early if the object is suspended
198-
if obj.Spec.Suspend {
199-
log.Info("Reconciliation is suspended for this object")
200-
return ctrl.Result{}, nil
201-
}
202-
203197
// Initialize the patch helper with the current version of the object.
204198
patchHelper, err := patch.NewHelper(obj, r.Client)
205199
if err != nil {
@@ -246,6 +240,13 @@ func (r *HelmChartReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
246240
return
247241
}
248242

243+
// Return if the object is suspended.
244+
if obj.Spec.Suspend {
245+
log.Info("Reconciliation is suspended for this object")
246+
recResult, retErr = sreconcile.ResultEmpty, nil
247+
return
248+
}
249+
249250
// Reconcile actual object
250251
reconcilers := []helmChartReconcileFunc{
251252
r.reconcileStorage,

0 commit comments

Comments
 (0)