feat(gctx): retry logic (kyverno#10796)

Signed-off-by: Khaled Emara <[email protected]>
TnGadaria · Aug 21, 2024 · aceb7d5 · aceb7d5
1 parent 4287f8c
commit aceb7d5
Show file tree

Hide file tree

Showing 22 changed files with 176 additions and 38 deletions.
diff --git a/api/kyverno/v2alpha1/global_context_entry_types.go b/api/kyverno/v2alpha1/global_context_entry_types.go
@@ -157,6 +157,12 @@ type ExternalAPICall struct {
 	// +kubebuilder:validation:Format=duration
 	// +kubebuilder:default=`10m`
 	RefreshInterval *metav1.Duration `json:"refreshInterval,omitempty"`
+	// RetryLimit defines the number of times the APICall should be retried in case of failure.
+	// +kubebuilder:validation:Minimum=1
+	// +kubebuilder:default=3
+	// +kubebuilder:validation:Optional
+	// +optional
+	RetryLimit int `json:"retryLimit,omitempty"`
 }
 
 // Validate implements programmatic validation

diff --git a/charts/kyverno/charts/crds/templates/kyverno.io/kyverno.io_globalcontextentries.yaml b/charts/kyverno/charts/crds/templates/kyverno.io/kyverno.io_globalcontextentries.yaml
@@ -105,6 +105,12 @@ spec:
                       such as "300ms", "1.5h" or "2h45m". Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h".
                     format: duration
                     type: string
+                  retryLimit:
+                    default: 3
+                    description: RetryLimit defines the number of times the APICall
+                      should be retried in case of failure.
+                    minimum: 1
+                    type: integer
                   service:
                     description: |-
                       Service is an API call to a JSON web service.

diff --git a/config/crds/kyverno/kyverno.io_globalcontextentries.yaml b/config/crds/kyverno/kyverno.io_globalcontextentries.yaml
@@ -99,6 +99,12 @@ spec:
                       such as "300ms", "1.5h" or "2h45m". Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h".
                     format: duration
                     type: string
+                  retryLimit:
+                    default: 3
+                    description: RetryLimit defines the number of times the APICall
+                      should be retried in case of failure.
+                    minimum: 1
+                    type: integer
                   service:
                     description: |-
                       Service is an API call to a JSON web service.

diff --git a/config/install-latest-testing.yaml b/config/install-latest-testing.yaml
@@ -24369,6 +24369,12 @@ spec:
                       such as "300ms", "1.5h" or "2h45m". Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h".
                     format: duration
                     type: string
+                  retryLimit:
+                    default: 3
+                    description: RetryLimit defines the number of times the APICall
+                      should be retried in case of failure.
+                    minimum: 1
+                    type: integer
                   service:
                     description: |-
                       Service is an API call to a JSON web service.

diff --git a/docs/user/crd/index.html b/docs/user/crd/index.html
@@ -7112,6 +7112,18 @@ <h3 id="kyverno.io/v2alpha1.ExternalAPICall">ExternalAPICall
 such as &ldquo;300ms&rdquo;, &ldquo;1.5h&rdquo; or &ldquo;2h45m&rdquo;. Valid time units are &ldquo;ns&rdquo;, &ldquo;us&rdquo; (or &ldquo;µs&rdquo;), &ldquo;ms&rdquo;, &ldquo;s&rdquo;, &ldquo;m&rdquo;, &ldquo;h&rdquo;.</p>
 </td>
 </tr>
+<tr>
+<td>
+<code>retryLimit</code><br/>
+<em>
+int
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>RetryLimit defines the number of times the APICall should be retried in case of failure.</p>
+</td>
+</tr>
 </tbody>
 </table>
 <hr />

diff --git a/docs/user/crd/kyverno.v2alpha1.html b/docs/user/crd/kyverno.v2alpha1.html
@@ -337,6 +337,33 @@ <H3 id="kyverno-io-v2alpha1-ExternalAPICall">ExternalAPICall
       </tr>
 
 
+
+
+      <tr>
+        <td><code>retryLimit</code>
+
+          </br>
+
+
+
+
+              <span style="font-family: monospace">int</span>
+
+
+        </td>
+        <td>
+
+
+          <p>RetryLimit defines the number of times the APICall should be retried in case of failure.</p>
+
+
+
+
+
+        </td>
+      </tr>
+
+
 
 
       </tbody>

diff --git a/pkg/client/applyconfigurations/kyverno/v2alpha1/externalapicall.go b/pkg/client/applyconfigurations/kyverno/v2alpha1/externalapicall.go
diff --git a/pkg/globalcontext/externalapi/entry.go b/pkg/globalcontext/externalapi/entry.go
@@ -59,7 +59,7 @@ func New(
 		caller := apicall.NewExecutor(logger, "globalcontext", client, config)
 
 		wait.UntilWithContext(ctx, func(ctx context.Context) {
-			if data, err := doCall(ctx, caller, call); err != nil {
+			if data, err := doCall(ctx, caller, call, gce.Spec.APICall.RetryLimit); err != nil {
 				e.setData(nil, err)
 
 				logger.Error(err, "failed to get data from api caller")
@@ -127,8 +127,24 @@ func (e *entry) setData(data any, err error) {
 	}
 }
 
-func doCall(ctx context.Context, caller apicall.Executor, call kyvernov1.APICall) (any, error) {
-	return caller.Execute(ctx, &call)
+func doCall(ctx context.Context, caller apicall.Executor, call kyvernov1.APICall, retryLimit int) (any, error) {
+	var result any
+	backoff := wait.Backoff{
+		Duration: retry.DefaultBackoff.Duration,
+		Factor:   retry.DefaultBackoff.Factor,
+		Jitter:   retry.DefaultBackoff.Jitter,
+		Steps:    retryLimit,
+	}
+
+	retryError := retry.OnError(backoff, func(err error) bool {
+		return err != nil
+	}, func() error {
+		var exeErr error
+		result, exeErr = caller.Execute(ctx, &call)
+		return exeErr
+	})
+
+	return result, retryError
 }
 
 func updateStatus(ctx context.Context, gceName string, kyvernoClient versioned.Interface, ready bool, reason string) error {

diff --git a/test/conformance/chainsaw/globalcontext/apicall-correct/chainsaw-test.yaml b/test/conformance/chainsaw/globalcontext/apicall-correct/chainsaw-test.yaml
@@ -14,9 +14,11 @@ spec:
     - apply:
         file: gctxentry.yaml
     - sleep:
-        duration: 15s
+        duration: 3s
     - apply:
         file: clusterpolicy.yaml
+    - sleep:
+        duration: 3s
     - assert:
         file: clusterpolicy-ready.yaml
     - apply:

diff --git a/test/conformance/chainsaw/globalcontext/apicall-correct/main-deployment.yaml b/test/conformance/chainsaw/globalcontext/apicall-correct/main-deployment.yaml
@@ -16,7 +16,13 @@ spec:
         app: main-deployment
     spec:
       containers:
-      - name: nginx
-        image: nginx:1.14.2
-        ports:
-        - containerPort: 80
+        - name: pause
+          image: registry.k8s.io/pause:latest
+          resources:
+            requests:
+              cpu: 10m
+              memory: 10Mi
+            limits:
+              cpu: 10m
+              memory: 10Mi
+      terminationGracePeriodSeconds: 0
diff --git a/test/conformance/chainsaw/globalcontext/apicall-correct/new-deployment.yaml b/test/conformance/chainsaw/globalcontext/apicall-correct/new-deployment.yaml
@@ -16,7 +16,13 @@ spec:
         app: new-deployment
     spec:
       containers:
-      - name: nginx
-        image: nginx:1.14.2
-        ports:
-        - containerPort: 80
+        - name: pause
+          image: registry.k8s.io/pause:latest
+          resources:
+            requests:
+              cpu: 10m
+              memory: 10Mi
+            limits:
+              cpu: 10m
+              memory: 10Mi
+      terminationGracePeriodSeconds: 0
diff --git a/test/conformance/chainsaw/globalcontext/apicall-failed/gctxentry.yaml b/test/conformance/chainsaw/globalcontext/apicall-failed/gctxentry.yaml
@@ -5,4 +5,4 @@ metadata:
 spec:
   apiCall:
     urlPath: "/apis/apps/v1/namespaces/default/unknown"
-    refreshInterval: 10s
+    refreshInterval: 1h
diff --git a/test/conformance/chainsaw/globalcontext/gctxentry-not-exist/chainsaw-test.yaml b/test/conformance/chainsaw/globalcontext/gctxentry-not-exist/chainsaw-test.yaml
@@ -15,5 +15,7 @@ spec:
     try:
     - apply:
         file: clusterpolicy.yaml
+    - sleep:
+        duration: 3s
     - assert:
         file: clusterpolicy-assert.yaml
diff --git a/test/conformance/chainsaw/globalcontext/gctxentry-not-exist/main-deployment.yaml b/test/conformance/chainsaw/globalcontext/gctxentry-not-exist/main-deployment.yaml
@@ -16,7 +16,13 @@ spec:
         app: main-deployment
     spec:
       containers:
-      - name: nginx
-        image: nginx:1.14.2
-        ports:
-        - containerPort: 80
+        - name: pause
+          image: registry.k8s.io/pause:latest
+          resources:
+            requests:
+              cpu: 10m
+              memory: 10Mi
+            limits:
+              cpu: 10m
+              memory: 10Mi
+      terminationGracePeriodSeconds: 0
diff --git a/test/conformance/chainsaw/globalcontext/not-ready/chainsaw-test.yaml b/test/conformance/chainsaw/globalcontext/not-ready/chainsaw-test.yaml
@@ -14,9 +14,11 @@ spec:
     - apply:
         file: gctxentry.yaml
     - sleep:
-        duration: 15s
+        duration: 3s
     - apply:
         file: clusterpolicy.yaml
+    - sleep:
+        duration: 3s
     - assert:
         file: clusterpolicy-ready.yaml
     - delete:
@@ -25,7 +27,7 @@ spec:
           kind: GlobalContextEntry
           name: gctx-not-ready
     - sleep:
-        duration: 5s
+        duration: 3s
     - assert:
         file: clusterpolicy-failed.yaml
     - apply:

diff --git a/test/conformance/chainsaw/globalcontext/not-ready/gctxentry.yaml b/test/conformance/chainsaw/globalcontext/not-ready/gctxentry.yaml
@@ -5,4 +5,4 @@ metadata:
 spec:
   apiCall:
     urlPath: "/apis/apps/v1/namespaces/test-globalcontext-not-ready/deployments"
-    refreshInterval: 10s
+    refreshInterval: 1h
diff --git a/test/conformance/chainsaw/globalcontext/not-ready/main-deployment.yaml b/test/conformance/chainsaw/globalcontext/not-ready/main-deployment.yaml
@@ -16,7 +16,13 @@ spec:
         app: main-deployment
     spec:
       containers:
-      - name: nginx
-        image: nginx:1.14.2
-        ports:
-        - containerPort: 80
+        - name: pause
+          image: registry.k8s.io/pause:latest
+          resources:
+            requests:
+              cpu: 10m
+              memory: 10Mi
+            limits:
+              cpu: 10m
+              memory: 10Mi
+      terminationGracePeriodSeconds: 0
diff --git a/test/conformance/chainsaw/globalcontext/not-ready/new-deployment.yaml b/test/conformance/chainsaw/globalcontext/not-ready/new-deployment.yaml
@@ -16,7 +16,13 @@ spec:
         app: new-deployment
     spec:
       containers:
-      - name: nginx
-        image: nginx:1.14.2
-        ports:
-        - containerPort: 80
+        - name: pause
+          image: registry.k8s.io/pause:latest
+          resources:
+            requests:
+              cpu: 10m
+              memory: 10Mi
+            limits:
+              cpu: 10m
+              memory: 10Mi
+      terminationGracePeriodSeconds: 0
diff --git a/test/conformance/chainsaw/globalcontext/resource-correct/chainsaw-test.yaml b/test/conformance/chainsaw/globalcontext/resource-correct/chainsaw-test.yaml
@@ -14,9 +14,11 @@ spec:
     - apply:
         file: gctxentry.yaml
     - sleep:
-        duration: 5s
+        duration: 3s
     - apply:
         file: clusterpolicy.yaml
+    - sleep:
+        duration: 3s
     - assert:
         file: clusterpolicy-ready.yaml
     - apply:

diff --git a/test/conformance/chainsaw/globalcontext/resource-correct/main-deployment.yaml b/test/conformance/chainsaw/globalcontext/resource-correct/main-deployment.yaml
@@ -16,7 +16,13 @@ spec:
         app: main-deployment
     spec:
       containers:
-      - name: nginx
-        image: nginx:1.14.2
-        ports:
-        - containerPort: 80
+        - name: pause
+          image: registry.k8s.io/pause:latest
+          resources:
+            requests:
+              cpu: 10m
+              memory: 10Mi
+            limits:
+              cpu: 10m
+              memory: 10Mi
+      terminationGracePeriodSeconds: 0
diff --git a/test/conformance/chainsaw/globalcontext/resource-correct/new-deployment.yaml b/test/conformance/chainsaw/globalcontext/resource-correct/new-deployment.yaml
@@ -16,7 +16,13 @@ spec:
         app: new-deployment
     spec:
       containers:
-      - name: nginx
-        image: nginx:1.14.2
-        ports:
-        - containerPort: 80
+        - name: pause
+          image: registry.k8s.io/pause:latest
+          resources:
+            requests:
+              cpu: 10m
+              memory: 10Mi
+            limits:
+              cpu: 10m
+              memory: 10Mi
+      terminationGracePeriodSeconds: 0
diff --git a/test/conformance/chainsaw/globalcontext/validate-crd/valid-context.yaml b/test/conformance/chainsaw/globalcontext/validate-crd/valid-context.yaml
@@ -11,4 +11,4 @@ spec:
         -----BEGIN CERTIFICATE-----
         -----REDACTED-----
         -----END CERTIFICATE-----
-    refreshInterval: 10ns
+    refreshInterval: 1h