Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Upgrade to controller-runtime v0.15.0 #420

Merged
merged 1 commit into from
May 30, 2023
Merged

Upgrade to controller-runtime v0.15.0 #420

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions api-hub/v1beta1/managedclustermodule_webhook.go
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

had the new controller-runtime version removed support for parsing option? If not, then why not use its code?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It has deprecated its config parser: issue, Go doc
Users are asked to move to their own config management system, this is what this PR does.

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
View file
Open in desktop
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
8 changes: 5 additions & 3 deletions config/crd-hub/bases/hub.kmm.sigs.x-k8s.io_managedclustermodules.yaml
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
Loading
Loading