[K8s] Implement retryable k8s client #3723
Conversation
# Conflicts: # pkg/containerimagebuilderpusher/kaniko.go
There was a problem hiding this comment.
A suggestion regarding the location and structuring of the code:
In Nuclio we don't have a clients package that includes all, but we do have client packages in pkg - for example dockerclient, cmdrunner, processwaiter etc.
common is more of a place for helper functions, which are split into domains.
I suggest moving the files in this PR to a separate k8sclient or kubeclient that will have the following structure:
pkg
├── kubeclient
│ └── client.go // the actual implementation of the client struct, with its methods
│ └── types.go // where the interface is defined, along with constants if any
│ └── retry.go // the retry helpers
There was a problem hiding this comment.
@TomerShor yeah, I agree here. That makes me think whether new package platform/kube/clients would be a good place for keeping both new kube retry client + nuclio custom client for CRDs. Wdyt?
There was a problem hiding this comment.
We actually already have a kube/client package. you can add the retry client there
There was a problem hiding this comment.
@TomerShor I moved it to be platform/kube/clients/nuclio as this is nuclio CRDs client
pkg/common/k8s/types.go
Outdated
| // ClientWithRetry provides resilient methods to interact with various Kubernetes resources | ||
| type ClientWithRetry interface { |
There was a problem hiding this comment.
The interface is just Client. The retry is the specific implementation
There was a problem hiding this comment.
Also - isn't this needed for all Nuclio CRDs as well?
There was a problem hiding this comment.
@TomerShor I replaced all the usages of kubernetes.Interface. For CRDs we use
nuclio/pkg/platform/kube/client/clientset/versioned/clientset.go
Lines 31 to 34 in 3709646
pkg/common/k8s/types.go
Outdated
| DeleteService(ctx context.Context, namespace string, name string, deleteOptions metav1.DeleteOptions) error | ||
|
|
||
| // PatchService applies a patch to a Service. | ||
| PatchService(ctx context.Context, namespace string, name string, pt types.PatchType, data []byte) (*corev1.Service, error) |
There was a problem hiding this comment.
| PatchService(ctx context.Context, namespace string, name string, pt types.PatchType, data []byte) (*corev1.Service, error) | |
| PatchService(ctx context.Context, namespace string, name string, patchType types.PatchType, data []byte) (*corev1.Service, error) |
pkg/common/k8s/types.go
Outdated
| // StreamPodLogs returns a stream of logs for a Pod. | ||
| StreamPodLogs(ctx context.Context, namespace, podName string, options *corev1.PodLogOptions) (io.ReadCloser, error) | ||
|
|
||
| //DeletePod deletes a Pod by name. |
There was a problem hiding this comment.
| //DeletePod deletes a Pod by name. | |
| // DeletePod deletes a Pod by name. |
| type clientWithRetry struct { | ||
| kubernetes.Interface | ||
| retries int | ||
| delay time.Duration | ||
| } | ||
|
|
||
| func NewClientWithRetryFromConfig(config *rest.Config) (Client, error) { | ||
| client, err := kubernetes.NewForConfig(config) | ||
| if err != nil { | ||
| return nil, errors.Wrap(err, "Failed to create Kubernetes client") | ||
| } | ||
| return &clientWithRetry{ | ||
| Interface: client, | ||
| retries: clients.MaxRetries, | ||
| delay: clients.Delay, | ||
| }, nil | ||
| } | ||
|
|
||
| func NewClientWithRetryFromClient(client kubernetes.Interface) Client { | ||
| return &clientWithRetry{ | ||
| Interface: client, | ||
| retries: clients.MaxRetries, | ||
| delay: clients.Delay, | ||
| } | ||
| } |
There was a problem hiding this comment.
Move these and all of the methods below to client.go
### Description This PR wraps `kubernetes.Interface` into Nuclio's custom retryable client to be able to retry on certain errors and avoid many problems that temporarily k8s issues may cause. ### Testing Long CI passed ✅ + while review is in progress, I'm going to test basic functionality ### References - Ticket link: https://iguazio.atlassian.net/browse/NUC-522 (cherry picked from commit bec9e2f)
### Description backport #3723
Description
This PR wraps
kubernetes.Interfaceinto Nuclio's custom retryable client to be able to retry on certain errors and avoid many problems that temporarily k8s issues may cause.Testing
Long CI passed ✅ +basic functionality tested
References