Merge pull request #102 from fluxcd/helm-repository-timeout

stefanprodan · web-flow · commit 6b12d18615fd · 2020-08-12T16:58:44.000+03:00
Implement Helm repository timeout
diff --git a/api/v1alpha1/gitrepository_types.go b/api/v1alpha1/gitrepository_types.go
@@ -23,7 +23,10 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-const GitRepositoryKind = "GitRepository"
+const (
+	GitRepositoryKind    = "GitRepository"
+	GitRepositoryTimeout = time.Second * 20
+)
 
 // GitRepositorySpec defines the desired state of a Git repository.
 type GitRepositorySpec struct {
@@ -194,7 +197,7 @@ func (in *GitRepository) GetTimeout() time.Duration {
 	if in.Spec.Timeout != nil {
 		return in.Spec.Timeout.Duration
 	}
-	return time.Second * 20
+	return GitRepositoryTimeout
 }
 
 // GetArtifact returns the latest artifact from the source
diff --git a/api/v1alpha1/helmrepository_types.go b/api/v1alpha1/helmrepository_types.go
@@ -17,11 +17,16 @@ limitations under the License.
 package v1alpha1
 
 import (
+	"time"
+
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-const HelmRepositoryKind = "HelmRepository"
+const (
+	HelmRepositoryKind    = "HelmRepository"
+	HelmRepositoryTimeout = time.Second * 60
+)
 
 // HelmRepositorySpec defines the reference to a Helm repository.
 type HelmRepositorySpec struct {
@@ -41,6 +46,10 @@ type HelmRepositorySpec struct {
 	// The interval at which to check the upstream for updates.
 	// +required
 	Interval metav1.Duration `json:"interval"`
+
+	// The timeout of index downloading, defaults to 60s.
+	// +optional
+	Timeout *metav1.Duration `json:"timeout,omitempty"`
 }
 
 // HelmRepositoryStatus defines the observed state of the HelmRepository.
@@ -148,6 +157,14 @@ func (in *HelmRepository) GetInterval() metav1.Duration {
 	return in.Spec.Interval
 }
 
+// GetTimeout returns the configured timeout or the default.
+func (in *HelmRepository) GetTimeout() time.Duration {
+	if in.Spec.Timeout != nil {
+		return in.Spec.Timeout.Duration
+	}
+	return HelmRepositoryTimeout
+}
+
 // +genclient
 // +genclient:Namespaced
 // +kubebuilder:object:root=true
diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go
diff --git a/config/crd/bases/source.toolkit.fluxcd.io_helmrepositories.yaml b/config/crd/bases/source.toolkit.fluxcd.io_helmrepositories.yaml
@@ -63,6 +63,9 @@ spec:
                       TODO: Add other useful fields. apiVersion, kind, uid?'
                     type: string
                 type: object
+              timeout:
+                description: The timeout of index downloading, defaults to 60s.
+                type: string
               url:
                 description: The Helm repository URL, a valid URL contains at least
                   a protocol and host.
diff --git a/controllers/helmrepository_controller.go b/controllers/helmrepository_controller.go
@@ -204,8 +204,8 @@ func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, repository sou
 		clientOpts = opts
 	}
 
-	// TODO(hidde): implement timeout from the HelmRepository
-	//  https://github.com/helm/helm/pull/7950
+	clientOpts = append(clientOpts, getter.WithTimeout(repository.GetTimeout()))
+
 	res, err := c.Get(u.String(), clientOpts...)
 	if err != nil {
 		return sourcev1.HelmRepositoryNotReady(repository, sourcev1.IndexationFailedReason, err.Error()), err
diff --git a/controllers/helmrepository_controller_test.go b/controllers/helmrepository_controller_test.go
@@ -37,9 +37,10 @@ import (
 var _ = Describe("HelmRepositoryReconciler", func() {
 
 	const (
-		timeout       = time.Second * 30
-		interval      = time.Second * 1
-		indexInterval = time.Second * 2
+		timeout           = time.Second * 30
+		interval          = time.Second * 1
+		indexInterval     = time.Second * 2
+		repositoryTimeout = time.Second * 5
 	)
 
 	Context("HelmRepository", func() {
@@ -87,6 +88,7 @@ var _ = Describe("HelmRepositoryReconciler", func() {
 				Spec: sourcev1.HelmRepositorySpec{
 					URL:      helmServer.URL(),
 					Interval: metav1.Duration{Duration: indexInterval},
+					Timeout:  &metav1.Duration{Duration: repositoryTimeout},
 				},
 			}
 			Expect(k8sClient.Create(context.Background(), created)).Should(Succeed())
@@ -150,6 +152,57 @@ var _ = Describe("HelmRepositoryReconciler", func() {
 			Eventually(exists(got.Status.Artifact.Path), timeout, interval).ShouldNot(BeTrue())
 		})
 
+		It("Handles timeout", func() {
+			helmServer.Start()
+
+			Expect(helmServer.PackageChart(path.Join("testdata/helmchart"))).Should(Succeed())
+			Expect(helmServer.GenerateIndex()).Should(Succeed())
+
+			key := types.NamespacedName{
+				Name:      "helmrepository-sample-" + randStringRunes(5),
+				Namespace: namespace.Name,
+			}
+			created := &sourcev1.HelmRepository{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      key.Name,
+					Namespace: key.Namespace,
+				},
+				Spec: sourcev1.HelmRepositorySpec{
+					URL:      helmServer.URL(),
+					Interval: metav1.Duration{Duration: indexInterval},
+				},
+			}
+			Expect(k8sClient.Create(context.Background(), created)).Should(Succeed())
+
+			By("Expecting index download to succeed")
+			Eventually(func() bool {
+				got := &sourcev1.HelmRepository{}
+				_ = k8sClient.Get(context.Background(), key, got)
+				for _, condition := range got.Status.Conditions {
+					if condition.Reason == sourcev1.IndexationSucceededReason {
+						return true
+					}
+				}
+				return false
+			}, timeout, interval).Should(BeTrue())
+
+			By("Expecting index download to timeout")
+			updated := &sourcev1.HelmRepository{}
+			Expect(k8sClient.Get(context.Background(), key, updated)).Should(Succeed())
+			updated.Spec.Timeout = &metav1.Duration{Duration: time.Microsecond}
+			Expect(k8sClient.Update(context.Background(), updated)).Should(Succeed())
+			Eventually(func() string {
+				got := &sourcev1.HelmRepository{}
+				_ = k8sClient.Get(context.Background(), key, got)
+				for _, condition := range got.Status.Conditions {
+					if condition.Reason == sourcev1.IndexationFailedReason {
+						return condition.Message
+					}
+				}
+				return ""
+			}, timeout, interval).Should(MatchRegexp("(?i)timeout"))
+		})
+
 		It("Authenticates when basic auth credentials are provided", func() {
 			helmServer, err = testserver.NewTempHelmServer()
 			Expect(err).NotTo(HaveOccurred())
diff --git a/docs/api/source.md b/docs/api/source.md
@@ -418,6 +418,20 @@ Kubernetes meta/v1.Duration
 <p>The interval at which to check the upstream for updates.</p>
 </td>
 </tr>
+<tr>
+<td>
+<code>timeout</code><br>
+<em>
+<a href="https://godoc.org/k8s.io/apimachinery/pkg/apis/meta/v1#Duration">
+Kubernetes meta/v1.Duration
+</a>
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>The timeout of index downloading, defaults to 60s.</p>
+</td>
+</tr>
 </table>
 </td>
 </tr>
@@ -998,6 +1012,20 @@ Kubernetes meta/v1.Duration
 <p>The interval at which to check the upstream for updates.</p>
 </td>
 </tr>
+<tr>
+<td>
+<code>timeout</code><br>
+<em>
+<a href="https://godoc.org/k8s.io/apimachinery/pkg/apis/meta/v1#Duration">
+Kubernetes meta/v1.Duration
+</a>
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>The timeout of index downloading, defaults to 60s.</p>
+</td>
+</tr>
 </tbody>
 </table>
 </div>
diff --git a/docs/spec/v1alpha1/helmrepositories.md b/docs/spec/v1alpha1/helmrepositories.md
@@ -27,6 +27,10 @@ type HelmRepositorySpec struct {
 	// The interval at which to check the upstream for updates.
 	// +required
 	Interval metav1.Duration `json:"interval"`
+
+	// The timeout of index downloading, defaults to 60s.
+	// +optional
+	Timeout *metav1.Duration `json:"timeout,omitempty"`
 }
 ```
 
diff --git a/go.mod b/go.mod
@@ -11,8 +11,8 @@ require (
 	github.com/onsi/ginkgo v1.12.1
 	github.com/onsi/gomega v1.10.1
 	github.com/sosedoff/gitkit v0.2.1-0.20191202022816-7182d43c6254
-	go.uber.org/zap v1.10.0
-	helm.sh/helm/v3 v3.2.4
+	go.uber.org/zap v1.13.0
+	helm.sh/helm/v3 v3.3.0
 	k8s.io/api v0.18.4
 	k8s.io/apimachinery v0.18.4
 	k8s.io/client-go v0.18.4
diff --git a/go.sum b/go.sum

Original file line number	Diff line number	Diff line change
`@@ -23,7 +23,10 @@ import (`
`23`	`23`	`metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"`
`24`	`24`	`)`
`25`	`25`
`26`		`-const GitRepositoryKind = "GitRepository"`
	`26`	`+const (`
	`27`	`+ GitRepositoryKind = "GitRepository"`
	`28`	`+ GitRepositoryTimeout = time.Second * 20`
	`29`	`+)`
`27`	`30`
`28`	`31`	`// GitRepositorySpec defines the desired state of a Git repository.`
`29`	`32`	`type GitRepositorySpec struct {`
`@@ -194,7 +197,7 @@ func (in *GitRepository) GetTimeout() time.Duration {`
`194`	`197`	`if in.Spec.Timeout != nil {`
`195`	`198`	`return in.Spec.Timeout.Duration`
`196`	`199`	`}`
`197`		`- return time.Second * 20`
	`200`	`+ return GitRepositoryTimeout`
`198`	`201`	`}`
`199`	`202`
`200`	`203`	`// GetArtifact returns the latest artifact from the source`