Skip to content

Commit 7c75aaf

Browse files
hiddecohiddeco
authored
Merge pull request #155 from fluxcd/resource-statuses
Prevent resources getting stuck on transient err
2 parents c1fb9b7 + b9576d5 commit 7c75aaf

9 files changed

+80
-37
lines changed

‎api/v1alpha1/artifact.go renamed to ‎api/v1alpha1/artifact_types.go

File renamed without changes.

‎api/v1alpha1/condition_types.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,3 +76,13 @@ const (
7676
// is underway.
7777
ProgressingReason string = "Progressing"
7878
)
79+
80+
// InReadyCondition returns if the given SourceCondition slice has a ReadyCondition with
81+
// a 'True' status.
82+
func InReadyCondition(conditions []SourceCondition) bool {
83+
condition := getCondition(conditions, ReadyCondition)
84+
if condition == nil {
85+
return false
86+
}
87+
return condition.Status == corev1.ConditionTrue
88+
}

‎api/v1alpha1/source.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,3 +26,15 @@ func filterOutSourceCondition(conditions []SourceCondition, condition string) []
2626
}
2727
return newConditions
2828
}
29+
30+
// getCondition returns the SourceCondition from the given slice that matches
31+
// the given condition.
32+
func getCondition(conditions []SourceCondition, condition string) *SourceCondition {
33+
for i := range conditions {
34+
c := conditions[i]
35+
if c.Type == condition {
36+
return &c
37+
}
38+
}
39+
return nil
40+
}

‎controllers/bucket_controller.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ func (r *BucketReconciler) reconcile(ctx context.Context, bucket sourcev1.Bucket
211211

212212
// return early on unchanged revision
213213
artifact := r.Storage.NewArtifactFor(bucket.Kind, bucket.GetObjectMeta(), revision, fmt.Sprintf("%s.tar.gz", revision))
214-
if bucket.GetArtifact().HasRevision(artifact.Revision) {
214+
if sourcev1.InReadyCondition(bucket.Status.Conditions) && bucket.GetArtifact().HasRevision(artifact.Revision) {
215215
if artifact.URL != bucket.GetArtifact().URL {
216216
r.Storage.SetArtifactURL(bucket.GetArtifact())
217217
bucket.Status.URL = r.Storage.SetHostname(bucket.Status.URL)
@@ -317,7 +317,8 @@ func (r *BucketReconciler) checksum(root string) (string, error) {
317317
// resetStatus returns a modified v1alpha1.Bucket and a boolean indicating
318318
// if the status field has been reset.
319319
func (r *BucketReconciler) resetStatus(bucket sourcev1.Bucket) (sourcev1.Bucket, bool) {
320-
if bucket.GetArtifact() != nil && !r.Storage.ArtifactExist(*bucket.GetArtifact()) {
320+
// We do not have an artifact, or it does no longer exist
321+
if bucket.GetArtifact() == nil || !r.Storage.ArtifactExist(*bucket.GetArtifact()) {
321322
bucket = sourcev1.BucketProgressing(bucket)
322323
bucket.Status.Artifact = nil
323324
return bucket, true

‎controllers/gitrepository_controller.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ func (r *GitRepositoryReconciler) reconcile(ctx context.Context, repository sour
198198

199199
// return early on unchanged revision
200200
artifact := r.Storage.NewArtifactFor(repository.Kind, repository.GetObjectMeta(), revision, fmt.Sprintf("%s.tar.gz", commit.Hash.String()))
201-
if repository.GetArtifact().HasRevision(artifact.Revision) {
201+
if sourcev1.InReadyCondition(repository.Status.Conditions) && repository.GetArtifact().HasRevision(artifact.Revision) {
202202
if artifact.URL != repository.GetArtifact().URL {
203203
r.Storage.SetArtifactURL(repository.GetArtifact())
204204
repository.Status.URL = r.Storage.SetHostname(repository.Status.URL)
@@ -276,7 +276,8 @@ func (r *GitRepositoryReconciler) verify(ctx context.Context, publicKeySecret ty
276276
// resetStatus returns a modified v1alpha1.GitRepository and a boolean indicating
277277
// if the status field has been reset.
278278
func (r *GitRepositoryReconciler) resetStatus(repository sourcev1.GitRepository) (sourcev1.GitRepository, bool) {
279-
if repository.GetArtifact() != nil && !r.Storage.ArtifactExist(*repository.GetArtifact()) {
279+
// We do not have an artifact, or it does no longer exist
280+
if repository.GetArtifact() == nil || !r.Storage.ArtifactExist(*repository.GetArtifact()) {
280281
repository = sourcev1.GitRepositoryProgressing(repository)
281282
repository.Status.Artifact = nil
282283
return repository, true

‎controllers/helmchart_controller.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context,
240240
// Return early if the revision is still the same as the current artifact
241241
newArtifact := r.Storage.NewArtifactFor(chart.Kind, chart.GetObjectMeta(), cv.Version,
242242
fmt.Sprintf("%s-%s.tgz", cv.Name, cv.Version))
243-
if !force && repository.GetArtifact().HasRevision(newArtifact.Revision) {
243+
if !force && sourcev1.InReadyCondition(chart.Status.Conditions) && chart.GetArtifact().HasRevision(newArtifact.Revision) {
244244
if newArtifact.URL != repository.GetArtifact().URL {
245245
r.Storage.SetArtifactURL(chart.GetArtifact())
246246
chart.Status.URL = r.Storage.SetHostname(chart.Status.URL)
@@ -425,9 +425,9 @@ func (r *HelmChartReconciler) reconcileFromTarballArtifact(ctx context.Context,
425425
// Return early if the revision is still the same as the current chart artifact
426426
newArtifact := r.Storage.NewArtifactFor(chart.Kind, chart.ObjectMeta.GetObjectMeta(), chartMetadata.Version,
427427
fmt.Sprintf("%s-%s.tgz", chartMetadata.Name, chartMetadata.Version))
428-
if !force && chart.GetArtifact().HasRevision(newArtifact.Revision) {
428+
if !force && sourcev1.InReadyCondition(chart.Status.Conditions) && chart.GetArtifact().HasRevision(newArtifact.Revision) {
429429
if newArtifact.URL != artifact.URL {
430-
r.Storage.SetArtifactURL(&newArtifact)
430+
r.Storage.SetArtifactURL(chart.GetArtifact())
431431
chart.Status.URL = r.Storage.SetHostname(chart.Status.URL)
432432
}
433433
return chart, nil
@@ -483,8 +483,8 @@ func (r *HelmChartReconciler) reconcileFromTarballArtifact(ctx context.Context,
483483
// resetStatus returns a modified v1alpha1.HelmChart and a boolean indicating
484484
// if the status field has been reset.
485485
func (r *HelmChartReconciler) resetStatus(chart sourcev1.HelmChart) (sourcev1.HelmChart, bool) {
486-
// The artifact does no longer exist
487-
if chart.GetArtifact() != nil && !r.Storage.ArtifactExist(*chart.GetArtifact()) {
486+
// We do not have an artifact, or it does no longer exist
487+
if chart.GetArtifact() == nil || !r.Storage.ArtifactExist(*chart.GetArtifact()) {
488488
chart = sourcev1.HelmChartProgressing(chart)
489489
chart.Status.Artifact = nil
490490
return chart, true

‎controllers/helmchart_controller_test.go

Lines changed: 41 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ var _ = Describe("HelmChartReconciler", func() {
279279
})
280280
helmServer.Start()
281281

282-
Expect(helmServer.PackageChart(path.Join("testdata/helmchart"))).Should(Succeed())
282+
Expect(helmServer.PackageChartWithVersion(path.Join("testdata/helmchart"), "0.1.0")).Should(Succeed())
283283
Expect(helmServer.GenerateIndex()).Should(Succeed())
284284

285285
secretKey := types.NamespacedName{
@@ -298,6 +298,7 @@ var _ = Describe("HelmChartReconciler", func() {
298298
}
299299
Expect(k8sClient.Create(context.Background(), secret)).Should(Succeed())
300300

301+
By("Creating repository and waiting for artifact")
301302
repositoryKey := types.NamespacedName{
302303
Name: "helmrepository-sample-" + randStringRunes(5),
303304
Namespace: namespace.Name,
@@ -312,12 +313,21 @@ var _ = Describe("HelmChartReconciler", func() {
312313
SecretRef: &corev1.LocalObjectReference{
313314
Name: secretKey.Name,
314315
},
315-
Interval: metav1.Duration{Duration: time.Hour * 1},
316+
Interval: metav1.Duration{Duration: pullInterval},
316317
},
317318
}
318319
Expect(k8sClient.Create(context.Background(), repository)).Should(Succeed())
319320
defer k8sClient.Delete(context.Background(), repository)
320321

322+
Eventually(func() bool {
323+
_ = k8sClient.Get(context.Background(), repositoryKey, repository)
324+
return repository.Status.Artifact != nil
325+
}, timeout, interval).Should(BeTrue())
326+
327+
By("Deleting secret before applying HelmChart")
328+
Expect(k8sClient.Delete(context.Background(), secret)).Should(Succeed())
329+
330+
By("Applying HelmChart")
321331
key := types.NamespacedName{
322332
Name: "helmchart-sample-" + randStringRunes(5),
323333
Namespace: namespace.Name,
@@ -340,63 +350,68 @@ var _ = Describe("HelmChartReconciler", func() {
340350
Expect(k8sClient.Create(context.Background(), chart)).Should(Succeed())
341351
defer k8sClient.Delete(context.Background(), chart)
342352

343-
By("Expecting artifact")
344-
Expect(k8sClient.Update(context.Background(), secret)).Should(Succeed())
353+
By("Expecting missing secret error")
354+
got := &sourcev1.HelmChart{}
345355
Eventually(func() bool {
346-
got := &sourcev1.HelmChart{}
347356
_ = k8sClient.Get(context.Background(), key, got)
348-
return got.Status.Artifact != nil &&
349-
storage.ArtifactExist(*got.Status.Artifact)
357+
for _, c := range got.Status.Conditions {
358+
if c.Reason == sourcev1.AuthenticationFailedReason &&
359+
strings.Contains(c.Message, "auth secret error") {
360+
return true
361+
}
362+
}
363+
return false
350364
}, timeout, interval).Should(BeTrue())
351365

352-
delete(secret.Data, "username")
353-
Expect(k8sClient.Update(context.Background(), secret)).Should(Succeed())
366+
By("Applying secret with missing keys")
367+
secret.ResourceVersion = ""
368+
secret.Data["username"] = []byte{}
369+
secret.Data["password"] = []byte{}
370+
Expect(k8sClient.Create(context.Background(), secret)).Should(Succeed())
354371

355-
By("Expecting missing field error")
356-
delete(secret.Data, "username")
357-
Expect(k8sClient.Update(context.Background(), secret)).Should(Succeed())
358-
got := &sourcev1.HelmChart{}
372+
By("Expecting 401")
359373
Eventually(func() bool {
374+
got := &sourcev1.HelmChart{}
360375
_ = k8sClient.Get(context.Background(), key, got)
361376
for _, c := range got.Status.Conditions {
362-
if c.Reason == sourcev1.AuthenticationFailedReason {
377+
if c.Reason == sourcev1.ChartPullFailedReason &&
378+
strings.Contains(c.Message, "401 Unauthorized") {
363379
return true
364380
}
365381
}
366382
return false
367383
}, timeout, interval).Should(BeTrue())
368-
Expect(got.Status.Artifact).ToNot(BeNil())
369384

370-
delete(secret.Data, "password")
385+
By("Adding username key")
386+
secret.Data["username"] = []byte(username)
371387
Expect(k8sClient.Update(context.Background(), secret)).Should(Succeed())
372388

373-
By("Expecting 401")
389+
By("Expecting missing field error")
374390
Eventually(func() bool {
375-
got := &sourcev1.HelmChart{}
376391
_ = k8sClient.Get(context.Background(), key, got)
377392
for _, c := range got.Status.Conditions {
378-
if c.Reason == sourcev1.ChartPullFailedReason &&
379-
strings.Contains(c.Message, "401 Unauthorized") {
393+
if c.Reason == sourcev1.AuthenticationFailedReason {
380394
return true
381395
}
382396
}
383397
return false
384398
}, timeout, interval).Should(BeTrue())
385399

386-
By("Expecting missing secret error")
387-
Expect(k8sClient.Delete(context.Background(), secret)).Should(Succeed())
388-
got = &sourcev1.HelmChart{}
400+
By("Adding password key")
401+
secret.Data["password"] = []byte(password)
402+
Expect(k8sClient.Update(context.Background(), secret)).Should(Succeed())
403+
404+
By("Expecting artifact")
389405
Eventually(func() bool {
390406
_ = k8sClient.Get(context.Background(), key, got)
391407
for _, c := range got.Status.Conditions {
392-
if c.Reason == sourcev1.AuthenticationFailedReason &&
393-
strings.Contains(c.Message, "auth secret error") {
408+
if c.Type == sourcev1.ReadyCondition && c.Status == corev1.ConditionTrue {
394409
return true
395410
}
396411
}
397412
return false
398413
}, timeout, interval).Should(BeTrue())
399-
Expect(got.Status.Artifact).ShouldNot(BeNil())
414+
Expect(got.Status.Artifact).ToNot(BeNil())
400415
})
401416
})
402417

‎controllers/helmrepository_controller.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, repository sou
217217
// return early on unchanged generation
218218
artifact := r.Storage.NewArtifactFor(repository.Kind, repository.ObjectMeta.GetObjectMeta(), i.Generated.Format(time.RFC3339Nano),
219219
fmt.Sprintf("index-%s.yaml", url.PathEscape(i.Generated.Format(time.RFC3339Nano))))
220-
if repository.GetArtifact().HasRevision(artifact.Revision) {
220+
if sourcev1.InReadyCondition(repository.Status.Conditions) && repository.GetArtifact().HasRevision(artifact.Revision) {
221221
if artifact.URL != repository.GetArtifact().URL {
222222
r.Storage.SetArtifactURL(repository.GetArtifact())
223223
repository.Status.URL = r.Storage.SetHostname(repository.Status.URL)
@@ -266,7 +266,8 @@ func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, repository sou
266266
// resetStatus returns a modified v1alpha1.HelmRepository and a boolean indicating
267267
// if the status field has been reset.
268268
func (r *HelmRepositoryReconciler) resetStatus(repository sourcev1.HelmRepository) (sourcev1.HelmRepository, bool) {
269-
if repository.GetArtifact() != nil && !r.Storage.ArtifactExist(*repository.GetArtifact()) {
269+
// We do not have an artifact, or it does no longer exist
270+
if repository.GetArtifact() == nil || !r.Storage.ArtifactExist(*repository.GetArtifact()) {
270271
repository = sourcev1.HelmRepositoryProgressing(repository)
271272
repository.Status.Artifact = nil
272273
return repository, true

‎controllers/helmrepository_controller_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,7 @@ var _ = Describe("HelmRepositoryReconciler", func() {
174174
},
175175
}
176176
Expect(k8sClient.Create(context.Background(), created)).Should(Succeed())
177+
defer k8sClient.Delete(context.Background(), created)
177178

178179
By("Expecting index download to succeed")
179180
Eventually(func() bool {
@@ -257,6 +258,7 @@ var _ = Describe("HelmRepositoryReconciler", func() {
257258
},
258259
}
259260
Expect(k8sClient.Create(context.Background(), created)).Should(Succeed())
261+
defer k8sClient.Delete(context.Background(), created)
260262

261263
By("Expecting 401")
262264
Eventually(func() bool {
@@ -348,6 +350,7 @@ var _ = Describe("HelmRepositoryReconciler", func() {
348350
},
349351
}
350352
Expect(k8sClient.Create(context.Background(), created)).Should(Succeed())
353+
defer k8sClient.Delete(context.Background(), created)
351354

352355
By("Expecting unknown authority error")
353356
Eventually(func() bool {

0 commit comments

Comments
 (0)