From 2da7ffbedb4115d23777ab3574a6a727624766ef Mon Sep 17 00:00:00 2001 From: Yuriy Losev Date: Fri, 20 Oct 2023 15:16:34 +0400 Subject: [PATCH] Fix cache invalidation (#12) Fix cache invalidation on APIResource discovery --- .github/workflows/code-checks.yaml | 4 +- .github/workflows/lint.yaml | 6 +-- .github/workflows/tests.yaml | 4 +- .golangci.yaml | 13 ++--- client/client.go | 80 +++++++++++++++++++----------- 5 files changed, 64 insertions(+), 43 deletions(-) diff --git a/.github/workflows/code-checks.yaml b/.github/workflows/code-checks.yaml index 9ac5485..9611f43 100644 --- a/.github/workflows/code-checks.yaml +++ b/.github/workflows/code-checks.yaml @@ -10,10 +10,10 @@ jobs: name: Go code style runs-on: ubuntu-latest steps: - - name: Set up Go 1.19 + - name: Set up Go 1.20 uses: actions/setup-go@v4 with: - go-version: 1.19 + go-version: '1.20' - uses: actions/checkout@v3 diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml index a71930f..c34e0d2 100644 --- a/.github/workflows/lint.yaml +++ b/.github/workflows/lint.yaml @@ -11,10 +11,10 @@ jobs: name: Run Go linters runs-on: ubuntu-latest steps: - - name: Set up Go 1.19 + - name: Set up Go 1.20 uses: actions/setup-go@v4 with: - go-version: 1.19 + go-version: '1.20' id: go - name: Check out addon-operator code @@ -38,7 +38,7 @@ jobs: - name: Run golangci-lint run: | - curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b . v1.52.0 + curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b . v1.54.2 ./golangci-lint run --sort-results codespell: diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index f34a524..a7ed34e 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -10,10 +10,10 @@ jobs: name: Run unit tests runs-on: ubuntu-latest steps: - - name: Set up Go 1.19 + - name: Set up Go 1.20 uses: actions/setup-go@v4 with: - go-version: 1.19 + go-version: '1.20' id: go - name: Check out addon-operator code diff --git a/.golangci.yaml b/.golangci.yaml index 02c9e4c..4fd7a1a 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -37,12 +37,13 @@ linters-settings: goimports: local-prefixes: github.com/flant/ depguard: - list-type: blacklist - include-go-root: true - packages: - - github.com/evanphx/json-patch - packages-with-error-message: - - github.com/evanphx/json-patch: "The 'github.com/evanphx/json-patch' package is superseded. Use pkg/utils/jsonpatch.go instead." + rules: + Main: + files: + - $all + deny: + - pkg: github.com/evanphx/json-patch + desc: "The 'github.com/evanphx/json-patch' package is superseded. Use pkg/utils/jsonpatch.go instead." issues: exclude: # Using underscores is a common practice, refactor in the future diff --git a/client/client.go b/client/client.go index 611d398..fcd95ee 100644 --- a/client/client.go +++ b/client/client.go @@ -6,8 +6,10 @@ import ( "strings" "time" + "github.com/pkg/errors" log "github.com/sirupsen/logrus" apixv1client "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1" + apiErrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" @@ -326,6 +328,21 @@ func getInClusterConfig() (config *rest.Config, defaultNs string, err error) { // NOTE that fetching all preferred resources can give errors if there are non-working // api controllers in cluster. func (c *Client) APIResourceList(apiVersion string) (lists []*metav1.APIResourceList, err error) { + lists, err = c.apiResourceList(apiVersion) + if err != nil { + if errors.Cause(err).Error() == "not found" { + // *errors.errorString type is here, we can't check it another way + c.cachedDiscovery.Invalidate() + return c.apiResourceList(apiVersion) + } + + return nil, err + } + + return lists, nil +} + +func (c *Client) apiResourceList(apiVersion string) (lists []*metav1.APIResourceList, err error) { if apiVersion == "" { // Get all preferred resources. // Can return errors if api controllers are not available. @@ -348,29 +365,38 @@ func (c *Client) APIResourceList(apiVersion string) (lists []*metav1.APIResource list, err := c.discovery().ServerResourcesForGroupVersion(gv.String()) if err != nil { - return nil, fmt.Errorf("apiVersion '%s' has no supported resources in cluster: %v", apiVersion, err) + // if not found, err has type *errors.errorString here + return nil, errors.Wrapf(err, "apiVersion '%s' has no supported resources in cluster", apiVersion) } lists = []*metav1.APIResourceList{list} } - // TODO should it copy group and version into each resource? - - // TODO create debug command to output this from cli - // Debug mode will list all available CRDs for apiVersion - // for _, r := range list.APIResources { - // log.Debugf("GVR: %30s %30s %30s", list.GroupVersion, r.Kind, - // fmt.Sprintf("%+v", append([]string{r.Name}, r.ShortNames...)), - // ) - // } - return } // APIResource fetches APIResource object from cluster that specifies the name of a resource and whether it is namespaced. +// if resource not found, we try to invalidate cache and // // NOTE that fetching with empty apiVersion can give errors if there are non-working // api controllers in cluster. -func (c *Client) APIResource(apiVersion, kind string) (res *metav1.APIResource, err error) { +func (c *Client) APIResource(apiVersion, kind string) (*metav1.APIResource, error) { + resource, err := c.apiResource(apiVersion, kind) + if err != nil { + if apiErrors.IsNotFound(err) { + c.cachedDiscovery.Invalidate() + resource, err = c.apiResource(apiVersion, kind) + } else { + return nil, fmt.Errorf("apiVersion '%s', kind '%s' is not supported by cluster: %w", apiVersion, kind, err) + } + } + if err != nil { + return nil, fmt.Errorf("apiVersion '%s', kind '%s' is not supported by cluster: %w", apiVersion, kind, err) + } + + return resource, nil +} + +func (c *Client) apiResource(apiVersion, kind string) (res *metav1.APIResource, err error) { lists, err := c.APIResourceList(apiVersion) if err != nil && len(lists) == 0 { // apiVersion is defined and there is a ServerResourcesForGroupVersion error @@ -378,26 +404,12 @@ func (c *Client) APIResource(apiVersion, kind string) (res *metav1.APIResource, } resource := getApiResourceFromResourceLists(kind, lists) - if resource != nil { - return resource, nil - } - - if c.cachedDiscovery != nil { - c.cachedDiscovery.Invalidate() - } - - resource = getApiResourceFromResourceLists(kind, lists) - if resource != nil { - return resource, nil + if resource == nil { + gv, _ := schema.ParseGroupVersion(apiVersion) + return nil, apiErrors.NewNotFound(schema.GroupResource{Group: gv.Group, Resource: kind}, "") } - // If resource is not found, append additional error, may be the custom API of the resource is not available. - additionalErr := "" - if err != nil { - additionalErr = fmt.Sprintf(", additional error: %s", err.Error()) - } - err = fmt.Errorf("apiVersion '%s', kind '%s' is not supported by cluster%s", apiVersion, kind, additionalErr) - return nil, err + return resource, nil } // GroupVersionResource returns a GroupVersionResource object to use with dynamic informer. @@ -424,6 +436,14 @@ func (c *Client) discovery() discovery.DiscoveryInterface { return c.Discovery() } +// InvalidateDiscoveryCache allows you to invalidate cache manually, for example, when you are deploying CRD +// KubeClient tries to invalidate cache automatically when needed, but you can save a few resources to call this manually +func (c *Client) InvalidateDiscoveryCache() { + if c.cachedDiscovery != nil { + c.cachedDiscovery.Invalidate() + } +} + func equalLowerCasedToOneOf(term string, choices ...string) bool { if len(choices) == 0 { return false