Skip to content

Commit

Permalink
Upgrade to controller-runtime v0.15.0 (#420)
Browse files Browse the repository at this point in the history
Remove the logger in the filter types as those functions now get a
context.
Reimplement the way we read the operator config since the
controller-runtime library code for that is deprecated.
Upgrade to Kubernetes v1.27.2.
Bump other dependencies.
  • Loading branch information
qbarrand committed May 30, 2023
1 parent df29c3d commit 8c8dc55
Show file tree
Hide file tree
Showing 22 changed files with 307 additions and 187 deletions.
7 changes: 4 additions & 3 deletions api-hub/v1beta1/managedclustermodule_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
ctrl "sigs.k8s.io/controller-runtime"
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/webhook"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
)

var modulelog = logf.Log.WithName("managed-cluster-module-resource")
Expand All @@ -39,7 +40,7 @@ func (mcm *ManagedClusterModule) SetupWebhookWithManager(mgr ctrl.Manager) error
var _ webhook.Validator = &ManagedClusterModule{}

// ValidateCreate implements webhook.Validator so a webhook will be registered for the type
func (mcm *ManagedClusterModule) ValidateCreate() error {
func (mcm *ManagedClusterModule) ValidateCreate() (admission.Warnings, error) {
modulelog.Info("Validating ManagedClusterModule creation", "name", mcm.Name, "namespace", mcm.Namespace)

module := &kmmv1beta1.Module{
Expand All @@ -49,7 +50,7 @@ func (mcm *ManagedClusterModule) ValidateCreate() error {
}

// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
func (mcm *ManagedClusterModule) ValidateUpdate(obj runtime.Object) error {
func (mcm *ManagedClusterModule) ValidateUpdate(obj runtime.Object) (admission.Warnings, error) {
modulelog.Info("Validating ManagedClusterModule update", "name", mcm.Name, "namespace", mcm.Namespace)

module := &kmmv1beta1.Module{
Expand All @@ -59,7 +60,7 @@ func (mcm *ManagedClusterModule) ValidateUpdate(obj runtime.Object) error {
}

// ValidateDelete implements webhook.Validator so a webhook will be registered for the type
func (mcm *ManagedClusterModule) ValidateDelete() error {
func (mcm *ManagedClusterModule) ValidateDelete() (admission.Warnings, error) {
modulelog.Info("Validating ManagedClusterModule delete", "name", mcm.Name, "namespace", mcm.Namespace)

module := &kmmv1beta1.Module{
Expand Down
17 changes: 9 additions & 8 deletions api/v1beta1/module_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
ctrl "sigs.k8s.io/controller-runtime"
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/webhook"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
)

// log is for logging in this package.
Expand All @@ -44,30 +45,30 @@ func (m *Module) SetupWebhookWithManager(mgr ctrl.Manager) error {
var _ webhook.Validator = &Module{}

// ValidateCreate implements webhook.Validator so a webhook will be registered for the type
func (m *Module) ValidateCreate() error {
func (m *Module) ValidateCreate() (admission.Warnings, error) {
modulelog.Info("Validating Module creation", "name", m.Name, "namespace", m.Namespace)

if err := m.validateKernelMapping(); err != nil {
return fmt.Errorf("failed to validate kernel mappings: %v", err)
return nil, fmt.Errorf("failed to validate kernel mappings: %v", err)
}

return m.validateModprobe()
return nil, m.validateModprobe()
}

// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
func (m *Module) ValidateUpdate(_ runtime.Object) error {
func (m *Module) ValidateUpdate(_ runtime.Object) (admission.Warnings, error) {
modulelog.Info("Validating Module update", "name", m.Name, "namespace", m.Namespace)

if err := m.validateKernelMapping(); err != nil {
return fmt.Errorf("failed to validate kernel mappings: %v", err)
return nil, fmt.Errorf("failed to validate kernel mappings: %v", err)
}

return m.validateModprobe()
return nil, m.validateModprobe()
}

// ValidateDelete implements webhook.Validator so a webhook will be registered for the type
func (m *Module) ValidateDelete() error {
return nil
func (m *Module) ValidateDelete() (admission.Warnings, error) {
return nil, nil
}

func (m *Module) validateKernelMapping() error {
Expand Down
25 changes: 14 additions & 11 deletions api/v1beta1/module_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -373,8 +373,8 @@ var _ = Describe("ValidateCreate", func() {
},
}

e := mod.ValidateCreate()
Expect(e).ToNot(HaveOccurred())
_, err := mod.ValidateCreate()
Expect(err).ToNot(HaveOccurred())
})

It("should fail when validating kernel mappings regexps", func() {
Expand All @@ -390,9 +390,9 @@ var _ = Describe("ValidateCreate", func() {
},
}

e := mod.ValidateCreate()
Expect(e).To(HaveOccurred())
Expect(e.Error()).To(ContainSubstring("failed to validate kernel mappings"))
_, err := mod.ValidateCreate()
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("failed to validate kernel mappings"))
})
})

Expand Down Expand Up @@ -434,8 +434,11 @@ var _ = Describe("ValidateUpdate", func() {
},
}

Expect(mod1.ValidateUpdate(nil)).ToNot(HaveOccurred())
Expect(mod2.ValidateUpdate(nil)).ToNot(HaveOccurred())
_, err1 := mod1.ValidateUpdate(nil)
Expect(err1).ToNot(HaveOccurred())

_, err2 := mod2.ValidateUpdate(nil)
Expect(err2).ToNot(HaveOccurred())
})

It("should fail when validating kernel mappings regexps", func() {
Expand All @@ -451,15 +454,15 @@ var _ = Describe("ValidateUpdate", func() {
},
}

e := mod.ValidateUpdate(nil)
Expect(e).To(HaveOccurred())
Expect(e.Error()).To(ContainSubstring("failed to validate kernel mappings"))
_, err := mod.ValidateUpdate(nil)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("failed to validate kernel mappings"))
})
})

var _ = Describe("ValidateDelete", func() {
It("should do nothing and return always nil", func() {
module := &Module{}
Expect(module.ValidateDelete()).ToNot(HaveOccurred())
Expect(module.ValidateDelete()).To(BeEmpty())
})
})
15 changes: 8 additions & 7 deletions cmd/manager-hub/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"flag"
"time"

"github.com/kubernetes-sigs/kernel-module-management/internal/config"
"k8s.io/apimachinery/pkg/runtime"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
Expand All @@ -29,7 +30,6 @@ import (
workv1 "open-cluster-management.io/api/work/v1"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/healthz"

// Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.)
// to ensure that exec-entrypoint and run can make use of them.
_ "k8s.io/client-go/plugin/pkg/client/auth"
Expand Down Expand Up @@ -87,21 +87,22 @@ func main() {

setupLogger.Info("Creating manager", "git commit", commit)

options := ctrl.Options{Scheme: scheme}

options, err = options.AndFrom(ctrl.ConfigFile().AtPath(configFile))
cfg, err := config.ParseFile(configFile)
if err != nil {
cmd.FatalError(setupLogger, err, "unable to load the config file")
cmd.FatalError(setupLogger, err, "could not parse the configuration file", "path", configFile)
}

mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), options)
options := cfg.ManagerOptions()
options.Scheme = scheme

mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), *options)
if err != nil {
cmd.FatalError(setupLogger, err, "unable to create manager")
}

client := mgr.GetClient()

filterAPI := filter.New(client, mgr.GetLogger())
filterAPI := filter.New(client)

metricsAPI := metrics.New()
metricsAPI.Register()
Expand Down
14 changes: 8 additions & 6 deletions cmd/manager/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"os"
"strconv"

"github.com/kubernetes-sigs/kernel-module-management/internal/config"
"k8s.io/apimachinery/pkg/runtime"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
Expand Down Expand Up @@ -93,21 +94,22 @@ func main() {

setupLogger.Info("Creating manager", "git commit", commit)

options := ctrl.Options{Scheme: scheme}

options, err = options.AndFrom(ctrl.ConfigFile().AtPath(configFile))
cfg, err := config.ParseFile(configFile)
if err != nil {
cmd.FatalError(setupLogger, err, "unable to load the config file")
cmd.FatalError(setupLogger, err, "could not parse the configuration file", "path", configFile)
}

mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), options)
options := cfg.ManagerOptions()
options.Scheme = scheme

mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), *options)
if err != nil {
cmd.FatalError(setupLogger, err, "unable to create manager")
}

client := mgr.GetClient()

filterAPI := filter.New(client, mgr.GetLogger())
filterAPI := filter.New(client)

metricsAPI := metrics.New()
metricsAPI.Register()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,8 @@ spec:
of compute resources required. If Requests is omitted
for a container, it defaults to Limits if that is
explicitly specified, otherwise to an implementation-defined
value. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/'
value. Requests cannot exceed Limits. More info:
https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/'
type: object
type: object
volumeMounts:
Expand Down Expand Up @@ -728,7 +729,7 @@ spec:
be the minimum value between the SizeLimit specified
here and the sum of memory limits of all containers
in a pod. The default is nil which means that
the limit is undefined. More info: http://kubernetes.io/docs/user-guide/volumes#emptydir'
the limit is undefined. More info: https://kubernetes.io/docs/concepts/storage/volumes#emptydir'
pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
x-kubernetes-int-or-string: true
type: object
Expand Down Expand Up @@ -967,7 +968,8 @@ spec:
a container, it defaults to Limits
if that is explicitly specified, otherwise
to an implementation-defined value.
More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/'
Requests cannot exceed Limits. More
info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/'
type: object
type: object
selector:
Expand Down
7 changes: 4 additions & 3 deletions config/crd/bases/kmm.sigs.x-k8s.io_modules.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ spec:
compute resources required. If Requests is omitted for
a container, it defaults to Limits if that is explicitly
specified, otherwise to an implementation-defined value.
More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/'
Requests cannot exceed Limits. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/'
type: object
type: object
volumeMounts:
Expand Down Expand Up @@ -698,7 +698,7 @@ spec:
value between the SizeLimit specified here and the
sum of memory limits of all containers in a pod. The
default is nil which means that the limit is undefined.
More info: http://kubernetes.io/docs/user-guide/volumes#emptydir'
More info: https://kubernetes.io/docs/concepts/storage/volumes#emptydir'
pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
x-kubernetes-int-or-string: true
type: object
Expand Down Expand Up @@ -925,7 +925,8 @@ spec:
If Requests is omitted for a container,
it defaults to Limits if that is explicitly
specified, otherwise to an implementation-defined
value. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/'
value. Requests cannot exceed Limits.
More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/'
type: object
type: object
selector:
Expand Down
16 changes: 5 additions & 11 deletions config/manager-hub/controller_manager_config.yaml
Original file line number Diff line number Diff line change
@@ -1,12 +1,6 @@
apiVersion: controller-runtime.sigs.k8s.io/v1alpha1
kind: ControllerManagerConfig
health:
healthProbeBindAddress: :8081
metrics:
bindAddress: 127.0.0.1:8080
webhook:
port: 9443
healthProbeBindAddress: :8081
metricsBindAddress: 127.0.0.1:8080
webhookPort: 9443
leaderElection:
leaderElect: true
resourceName: kmm-hub.sigs.x-k8s.io

enabled: true
resourceID: kmm-hub.sigs.x-k8s.io
16 changes: 5 additions & 11 deletions config/manager/controller_manager_config.yaml
Original file line number Diff line number Diff line change
@@ -1,12 +1,6 @@
apiVersion: controller-runtime.sigs.k8s.io/v1alpha1
kind: ControllerManagerConfig
health:
healthProbeBindAddress: :8081
metrics:
bindAddress: 127.0.0.1:8080
webhook:
port: 9443
healthProbeBindAddress: :8081
metricsBindAddress: 127.0.0.1:8080
webhookPort: 9443
leaderElection:
leaderElect: true
resourceName: kmm.sigs.x-k8s.io

enabled: true
resourceID: kmm.sigs.x-k8s.io
14 changes: 6 additions & 8 deletions controllers/hub/managedclustermodule_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ import (
"context"
"fmt"

hubv1beta1 "github.com/kubernetes-sigs/kernel-module-management/api-hub/v1beta1"
"github.com/kubernetes-sigs/kernel-module-management/internal/cluster"
"github.com/kubernetes-sigs/kernel-module-management/internal/filter"
"github.com/kubernetes-sigs/kernel-module-management/internal/manifestwork"
"github.com/kubernetes-sigs/kernel-module-management/internal/statusupdater"
batchv1 "k8s.io/api/batch/v1"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -31,13 +36,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/handler"
"sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/source"

hubv1beta1 "github.com/kubernetes-sigs/kernel-module-management/api-hub/v1beta1"
"github.com/kubernetes-sigs/kernel-module-management/internal/cluster"
"github.com/kubernetes-sigs/kernel-module-management/internal/filter"
"github.com/kubernetes-sigs/kernel-module-management/internal/manifestwork"
"github.com/kubernetes-sigs/kernel-module-management/internal/statusupdater"
)

const ManagedClusterModuleReconcilerName = "ManagedClusterModule"
Expand Down Expand Up @@ -168,7 +166,7 @@ func (r *ManagedClusterModuleReconciler) SetupWithManager(mgr ctrl.Manager) erro
Owns(&workv1.ManifestWork{}).
Owns(&batchv1.Job{}).
Watches(
&source.Kind{Type: &clusterv1.ManagedCluster{}},
&clusterv1.ManagedCluster{},
handler.EnqueueRequestsFromMapFunc(r.filter.FindManagedClusterModulesForCluster),
builder.WithPredicates(
r.filter.ManagedClusterModuleReconcilerManagedClusterPredicate(),
Expand Down
3 changes: 1 addition & 2 deletions controllers/module_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/handler"
"sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/source"
)

const ModuleReconcilerName = "Module"
Expand Down Expand Up @@ -490,7 +489,7 @@ func (r *ModuleReconciler) SetupWithManager(mgr ctrl.Manager, kernelLabel string
Owns(&appsv1.DaemonSet{}).
Owns(&batchv1.Job{}).
Watches(
&source.Kind{Type: &v1.Node{}},
&v1.Node{},
handler.EnqueueRequestsFromMapFunc(r.filter.FindModulesForNode),
builder.WithPredicates(
r.filter.ModuleReconcilerNodePredicate(kernelLabel),
Expand Down
12 changes: 4 additions & 8 deletions controllers/node_kernel_clusterclaim.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"sort"
"strings"

"github.com/kubernetes-sigs/kernel-module-management/internal/constants"
"github.com/kubernetes-sigs/kernel-module-management/internal/filter"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"open-cluster-management.io/api/cluster/v1alpha1"
Expand All @@ -17,10 +19,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/predicate"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
"sigs.k8s.io/controller-runtime/pkg/source"

"github.com/kubernetes-sigs/kernel-module-management/internal/constants"
"github.com/kubernetes-sigs/kernel-module-management/internal/filter"
)

//+kubebuilder:rbac:groups="core",resources=nodes,verbs=get;patch;list;watch
Expand Down Expand Up @@ -101,10 +99,8 @@ func (r *NodeKernelClusterClaimReconciler) SetupWithManager(mgr ctrl.Manager) er
// Each time our ClusterClaim is updated, enqueue an empty reconciliation request.
// We list all nodes during reconciliation, so sending an empty request is OK.
Watches(
&source.Kind{
Type: &v1alpha1.ClusterClaim{},
},
handler.EnqueueRequestsFromMapFunc(func(_ client.Object) []reconcile.Request {
&v1alpha1.ClusterClaim{},
handler.EnqueueRequestsFromMapFunc(func(_ context.Context, _ client.Object) []reconcile.Request {
return []reconcile.Request{{}}
}),
builder.WithPredicates(
Expand Down

0 comments on commit 8c8dc55

Please sign in to comment.