Skip to content

Commit

Permalink
Use old restmapper
Browse files Browse the repository at this point in the history
- Latest restmapper implementation doesnot handle cases where an API Group / GroupVersion is not present in the system. In such cases Clients expect No{Kind,Resource}MatchErrors.
- Upstream fix is already open kubernetes-sigs/controller-runtime#2425.
- Revert this commit once the fix is available.
  • Loading branch information
ary1992 committed Aug 10, 2023
1 parent 19f74f2 commit 1bd269f
Show file tree
Hide file tree
Showing 16 changed files with 629 additions and 34 deletions.
2 changes: 1 addition & 1 deletion cmd/gardener-apiserver/.import-restrictions
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@ rules:
# make sure apiserver can build without these packages
- github.com/gardener/gardener/charts
- github.com/gardener/gardener/pkg/operation
- github.com/gardener/gardener/third_party
- github.com/gardener/gardener/third_party/apiserver
8 changes: 5 additions & 3 deletions cmd/gardener-resource-manager/app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/go-logr/logr"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
"golang.org/x/time/rate"
corev1 "k8s.io/api/core/v1"
eventsv1 "k8s.io/api/events/v1"
eventsv1beta1 "k8s.io/api/events/v1beta1"
Expand All @@ -38,7 +39,6 @@ import (
"k8s.io/klog/v2"
"k8s.io/utils/pointer"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
"sigs.k8s.io/controller-runtime/pkg/cluster"
controllerconfig "sigs.k8s.io/controller-runtime/pkg/config"
"sigs.k8s.io/controller-runtime/pkg/healthz"
Expand All @@ -56,6 +56,7 @@ import (
resourcemanagerclient "github.com/gardener/gardener/pkg/resourcemanager/client"
"github.com/gardener/gardener/pkg/resourcemanager/controller"
"github.com/gardener/gardener/pkg/resourcemanager/webhook"
thirdpartyapiutil "github.com/gardener/gardener/third_party/controller-runtime/pkg/apiutil"
)

// Name is a const for the name of this component.
Expand Down Expand Up @@ -206,9 +207,10 @@ func run(ctx context.Context, log logr.Logger, cfg *config.ResourceManagerConfig
// use dynamic rest mapper for target cluster, which will automatically rediscover resources on NoMatchErrors
// but is rate-limited to not issue to many discovery calls (rate-limit shared across all reconciliations)
opts.MapperProvider = func(config *rest.Config, httpClient *http.Client) (meta.RESTMapper, error) {
return apiutil.NewDynamicRESTMapper(
return thirdpartyapiutil.NewDynamicRESTMapper(
config,
httpClient,
thirdpartyapiutil.WithLazyDiscovery,
thirdpartyapiutil.WithLimiter(rate.NewLimiter(rate.Every(1*time.Minute), 1)), // rediscover at maximum every minute
)
}

Expand Down
20 changes: 20 additions & 0 deletions cmd/gardenlet/app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"context"
"fmt"
"net"
"net/http"
"os"
goruntime "runtime"
"strconv"
Expand All @@ -26,11 +27,13 @@ import (
"github.com/go-logr/logr"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
"golang.org/x/time/rate"
certificatesv1 "k8s.io/api/certificates/v1"
coordinationv1 "k8s.io/api/coordination/v1"
corev1 "k8s.io/api/core/v1"
eventsv1 "k8s.io/api/events/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/fields"
"k8s.io/apimachinery/pkg/util/wait"
Expand Down Expand Up @@ -72,6 +75,7 @@ import (
"github.com/gardener/gardener/pkg/utils/flow"
gardenerutils "github.com/gardener/gardener/pkg/utils/gardener"
kubernetesutils "github.com/gardener/gardener/pkg/utils/kubernetes"
thirdpartyapiutil "github.com/gardener/gardener/third_party/controller-runtime/pkg/apiutil"
)

// Name is a const for the name of this component.
Expand Down Expand Up @@ -162,6 +166,14 @@ func run(ctx context.Context, cancel context.CancelFunc, log logr.Logger, cfg *c
RecoverPanic: pointer.Bool(true),
},

MapperProvider: func(config *rest.Config, httpClient *http.Client) (meta.RESTMapper, error) {
return thirdpartyapiutil.NewDynamicRESTMapper(
config,
thirdpartyapiutil.WithLazyDiscovery,
thirdpartyapiutil.WithLimiter(rate.NewLimiter(rate.Every(1*time.Minute), 1)), // rediscover at maximum every minute
)
},

Client: client.Options{
Cache: &client.CacheOptions{
DisableFor: []client.Object{
Expand Down Expand Up @@ -330,6 +342,14 @@ func (g *garden) Start(ctx context.Context) error {
Reader: uncachedClient,
}, nil
}

opts.MapperProvider = func(config *rest.Config, httpClient *http.Client) (meta.RESTMapper, error) {
return thirdpartyapiutil.NewDynamicRESTMapper(
config,
thirdpartyapiutil.WithLazyDiscovery,
thirdpartyapiutil.WithLimiter(rate.NewLimiter(rate.Every(1*time.Minute), 1)), // rediscover at maximum every minute
)
}
})
if err != nil {
return fmt.Errorf("failed creating garden cluster object: %w", err)
Expand Down
1 change: 1 addition & 0 deletions extensions/.import-restrictions
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,4 @@ rules:
- github.com/gardener/gardener/pkg/mock
- github.com/gardener/gardener/pkg/resourcemanager/controller/garbagecollector/references
- github.com/gardener/gardener/pkg/utils
- github.com/gardener/gardener/third_party/controller-runtime/pkg/apiutil
9 changes: 2 additions & 7 deletions extensions/pkg/util/shoot_clients.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,14 @@ import (
"k8s.io/client-go/rest"
"k8s.io/utils/pointer"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"

extensionsconfig "github.com/gardener/gardener/extensions/pkg/apis/config"
v1beta1constants "github.com/gardener/gardener/pkg/apis/core/v1beta1/constants"
"github.com/gardener/gardener/pkg/chartrenderer"
kubernetesclient "github.com/gardener/gardener/pkg/client/kubernetes"
kubernetesutils "github.com/gardener/gardener/pkg/utils/kubernetes"
"github.com/gardener/gardener/pkg/utils/secrets"
thirdpartyapiutil "github.com/gardener/gardener/third_party/controller-runtime/pkg/apiutil"
)

// ShootClients bundles together several clients for the shoot cluster.
Expand Down Expand Up @@ -108,12 +108,7 @@ func NewClientForShoot(ctx context.Context, c client.Client, namespace string, o
ApplyRESTOptions(shootRESTConfig, restOptions)

if opts.Mapper == nil {
httpClient, err := rest.HTTPClientFor(shootRESTConfig)
if err != nil {
return nil, nil, fmt.Errorf("failed to get HTTP client for config: %w", err)
}

mapper, err := apiutil.NewDynamicRESTMapper(shootRESTConfig, httpClient)
mapper, err := thirdpartyapiutil.NewDynamicRESTMapper(shootRESTConfig)
if err != nil {
return nil, nil, fmt.Errorf("failed to create new DynamicRESTMapper: %w", err)
}
Expand Down
3 changes: 1 addition & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,10 @@ require (
sigs.k8s.io/controller-runtime v0.15.0
sigs.k8s.io/controller-runtime/tools/setup-envtest v0.0.0-20221212190805-d4f1e822ca11 // v0.14.1
sigs.k8s.io/controller-tools v0.11.3
sigs.k8s.io/structured-merge-diff/v4 v4.2.3
sigs.k8s.io/yaml v1.3.0
)

require sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect

require (
github.com/BurntSushi/toml v1.0.0 // indirect
github.com/Masterminds/goutils v1.1.1 // indirect
Expand Down
12 changes: 5 additions & 7 deletions pkg/client/kubernetes/runtime_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"time"

"github.com/go-logr/logr"
"golang.org/x/time/rate"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/client-go/rest"
Expand All @@ -29,6 +30,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"

kubernetescache "github.com/gardener/gardener/pkg/client/kubernetes/cache"
thirdpartyapiutil "github.com/gardener/gardener/third_party/controller-runtime/pkg/apiutil"
)

const (
Expand All @@ -55,15 +57,11 @@ func setCacheOptionsDefaults(options *cache.Options) error {

func setClientOptionsDefaults(config *rest.Config, options *client.Options) error {
if options.Mapper == nil {
httpClient, err := rest.HTTPClientFor(config)
if err != nil {
return fmt.Errorf("failed to get HTTP client for config: %w", err)
}

// default the client's REST mapper to a dynamic REST mapper (automatically rediscovers resources on NoMatchErrors)
mapper, err := apiutil.NewDynamicRESTMapper(
mapper, err := thirdpartyapiutil.NewDynamicRESTMapper(
config,
httpClient,
thirdpartyapiutil.WithLazyDiscovery,
thirdpartyapiutil.WithLimiter(rate.NewLimiter(rate.Every(5*time.Second), 1)),
)
if err != nil {
return fmt.Errorf("failed to create new DynamicRESTMapper: %w", err)
Expand Down
2 changes: 2 additions & 0 deletions skaffold-operator.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ build:
- pkg/utils/version
- plugin/pkg
- third_party/gopkg.in/yaml.v2
- third_party/controller-runtime/pkg/apiutil
- vendor
- VERSION
ldflags:
Expand Down Expand Up @@ -312,6 +313,7 @@ build:
- pkg/utils/validation/features
- pkg/utils/validation/kubernetesversion
- pkg/utils/version
- third_party/controller-runtime/pkg/apiutil
- vendor
- VERSION
ldflags:
Expand Down
7 changes: 7 additions & 0 deletions skaffold.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ build:
- plugin/pkg/shoot/validator
- plugin/pkg/shoot/vpa
- plugin/pkg/utils
- third_party/controller-runtime/pkg/apiutil
- vendor
- VERSION
ldflags:
Expand Down Expand Up @@ -306,6 +307,7 @@ build:
- pkg/utils/validation/kubernetesversion
- pkg/utils/version
- third_party/gopkg.in/yaml.v2
- third_party/controller-runtime/pkg/apiutil
- vendor
- VERSION
ldflags:
Expand Down Expand Up @@ -369,6 +371,7 @@ build:
- pkg/utils/validation/cidr
- pkg/utils/validation/kubernetesversion
- pkg/utils/version
- third_party/controller-runtime/pkg/apiutil
- vendor
- VERSION
ldflags:
Expand Down Expand Up @@ -446,6 +449,7 @@ build:
- pkg/utils/version
- third_party/apiserver/pkg/apis/audit/v1alpha1
- third_party/apiserver/pkg/apis/audit/v1beta1
- third_party/controller-runtime/pkg/apiutil
- vendor
- VERSION
ldflags:
Expand Down Expand Up @@ -672,6 +676,7 @@ build:
- pkg/utils/validation/kubernetesversion
- pkg/utils/version
- third_party/gopkg.in/yaml.v2
- third_party/controller-runtime/pkg/apiutil
- vendor
- VERSION
ldflags:
Expand Down Expand Up @@ -980,6 +985,7 @@ build:
- pkg/utils/validation/kubernetesversion
- pkg/utils/version
- third_party/gopkg.in/yaml.v2
- third_party/controller-runtime/pkg/apiutil
- vendor
- VERSION
ldflags:
Expand Down Expand Up @@ -1081,6 +1087,7 @@ build:
- pkg/utils/validation/features
- pkg/utils/validation/kubernetesversion
- pkg/utils/version
- third_party/controller-runtime/pkg/apiutil
- vendor
- VERSION
ldflags:
Expand Down
6 changes: 6 additions & 0 deletions test/integration/gardenlet/seed/care/care_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package care_test

import (
"context"
"net/http"
"path/filepath"
"testing"
"time"
Expand All @@ -24,6 +25,7 @@ import (
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"
Expand All @@ -44,6 +46,7 @@ import (
"github.com/gardener/gardener/pkg/gardenlet/features"
"github.com/gardener/gardener/pkg/logger"
. "github.com/gardener/gardener/pkg/utils/test/matchers"
thirdpartyapiutil "github.com/gardener/gardener/third_party/controller-runtime/pkg/apiutil"
)

func TestCare(t *testing.T) {
Expand Down Expand Up @@ -135,6 +138,9 @@ var _ = BeforeSuite(func() {
},
},
},
MapperProvider: func(config *rest.Config, httpClient *http.Client) (meta.RESTMapper, error) {
return thirdpartyapiutil.NewDynamicRESTMapper(config)
},
})
Expect(err).NotTo(HaveOccurred())
mgrClient = mgr.GetClient()
Expand Down
7 changes: 2 additions & 5 deletions test/integration/gardenlet/seed/seed/seed_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,9 @@ import (
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/client-go/rest"
"k8s.io/utils/pointer"
"sigs.k8s.io/controller-runtime/pkg/cache"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
"sigs.k8s.io/controller-runtime/pkg/manager"

"github.com/gardener/gardener/pkg/api/indexer"
Expand All @@ -58,6 +56,7 @@ import (
"github.com/gardener/gardener/pkg/utils/test"
. "github.com/gardener/gardener/pkg/utils/test/matchers"
"github.com/gardener/gardener/test/utils/namespacefinalizer"
thirdpartyapiutil "github.com/gardener/gardener/third_party/controller-runtime/pkg/apiutil"
)

var _ = Describe("Seed controller tests", func() {
Expand Down Expand Up @@ -88,9 +87,7 @@ var _ = Describe("Seed controller tests", func() {
})

By("Setup manager")
httpClient, err := rest.HTTPClientFor(restConfig)
Expect(err).NotTo(HaveOccurred())
mapper, err := apiutil.NewDynamicRESTMapper(restConfig, httpClient)
mapper, err := thirdpartyapiutil.NewDynamicRESTMapper(restConfig)
Expect(err).NotTo(HaveOccurred())

mgr, err := manager.New(restConfig, manager.Options{
Expand Down
6 changes: 2 additions & 4 deletions test/integration/operator/garden/care/care_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import (
"k8s.io/client-go/rest"
"sigs.k8s.io/controller-runtime/pkg/cache"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
"sigs.k8s.io/controller-runtime/pkg/envtest"
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/log/zap"
Expand All @@ -45,6 +44,7 @@ import (
"github.com/gardener/gardener/pkg/operator/controller/garden/care"
"github.com/gardener/gardener/pkg/operator/features"
. "github.com/gardener/gardener/pkg/utils/test/matchers"
thirdpartyapiutil "github.com/gardener/gardener/third_party/controller-runtime/pkg/apiutil"
)

func TestGarden(t *testing.T) {
Expand Down Expand Up @@ -127,9 +127,7 @@ var _ = BeforeSuite(func() {
})

By("Setup manager")
httpClient, err := rest.HTTPClientFor(restConfig)
Expect(err).NotTo(HaveOccurred())
mapper, err := apiutil.NewDynamicRESTMapper(restConfig, httpClient)
mapper, err := thirdpartyapiutil.NewDynamicRESTMapper(restConfig)
Expect(err).NotTo(HaveOccurred())

mgr, err := manager.New(restConfig, manager.Options{
Expand Down
7 changes: 2 additions & 5 deletions test/integration/operator/garden/garden/garden_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,11 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/rest"
clientcmdlatest "k8s.io/client-go/tools/clientcmd/api/latest"
clientcmdv1 "k8s.io/client-go/tools/clientcmd/api/v1"
"k8s.io/utils/pointer"
"sigs.k8s.io/controller-runtime/pkg/cache"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
"sigs.k8s.io/controller-runtime/pkg/manager"

gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1"
Expand All @@ -62,6 +60,7 @@ import (
"github.com/gardener/gardener/pkg/utils/test"
. "github.com/gardener/gardener/pkg/utils/test/matchers"
"github.com/gardener/gardener/test/utils/operationannotation"
thirdpartyapiutil "github.com/gardener/gardener/third_party/controller-runtime/pkg/apiutil"
)

var _ = Describe("Garden controller tests", func() {
Expand Down Expand Up @@ -110,9 +109,7 @@ var _ = Describe("Garden controller tests", func() {
})

By("Setup manager")
httpClient, err := rest.HTTPClientFor(restConfig)
Expect(err).NotTo(HaveOccurred())
mapper, err := apiutil.NewDynamicRESTMapper(restConfig, httpClient)
mapper, err := thirdpartyapiutil.NewDynamicRESTMapper(restConfig)
Expect(err).NotTo(HaveOccurred())

mgr, err := manager.New(restConfig, manager.Options{
Expand Down

0 comments on commit 1bd269f

Please sign in to comment.