Skip to content

Commit

Permalink
Fix helmrepo reconciler unfinished worker bug
Browse files Browse the repository at this point in the history
Although all the APIs had interval as a required field, when tests
objects were created, they had the zero value of interval, which the API
server accepts. A zero interval value results in the test objects to
reconcile only once when they are created and never reconcile again
unless there's an update to the object. Most of the tests worked with
this behavior.

With HelmRepository removing the interval requirement and adding an
internal default, all the HelmRepository objects created in the tests
without any interval have a default interval value which results in
objects to reconcile automatically if they are not cleaned up after
running tests. TestHelmRepositoryReconciler_InMemoryCaching creates
HelmRepository but doesn't delete it at the end. This leads to a
reconciliation of HelmRepository outside of the test in the envtest
environment. It just happened to be that the reconciliation time matches
with the end of test time. At the end of the test run, the reconcilers
receive shutdown signal and any test server, like helmrepository server,
are stopped. A HelmRepository reconciliation triggered just before the
shutdown signal gets stuck in the reconciliation. HelmRepository can't
download the index as the test index server has stopped and hangs for
some time. The HelmRepository reconciler worker remains in active state,
unlike other reconciler workers that shut down, resulting in the test to
timeout at the end.

The is fixed by deleting the HelmRepository object created in
TestHelmRepositoryReconciler_InMemoryCaching at the end of the test
similar to other tests.

Signed-off-by: Sunny <[email protected]>
  • Loading branch information
darkowlzz committed Oct 31, 2023
1 parent d02d737 commit 778b263
Showing 1 changed file with 10 additions and 0 deletions.
10 changes: 10 additions & 0 deletions internal/controller/helmrepository_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1703,6 +1703,16 @@ func TestHelmRepositoryReconciler_InMemoryCaching(t *testing.T) {
g.Expect(err).ToNot(HaveOccurred())
_, cacheHit := testCache.Get(helmRepo.GetArtifact().Path)
g.Expect(cacheHit).To(BeTrue())

g.Expect(testEnv.Delete(ctx, helmRepo)).To(Succeed())

// Wait for HelmRepository to be deleted
g.Eventually(func() bool {
if err := testEnv.Get(ctx, key, helmRepo); err != nil {
return apierrors.IsNotFound(err)
}
return false
}, timeout).Should(BeTrue())
}

func TestHelmRepositoryReconciler_ociMigration(t *testing.T) {
Expand Down

0 comments on commit 778b263

Please sign in to comment.