Skip to content

Commit 36567c5

Browse files
TomerNewmankubernetes-prow[bot]
TomerNewman
authored and
kubernetes-prow[bot]
committed
Gate DRA reconciler on Kubernetes >= 1.34
The DRA reconciler was registered unconditionally, which would crash the entire operator on clusters older than 1.34 because the resource.k8s.io/v1 DeviceClass API does not exist there. Introduce an internal/version package with KubeVersion, DiscoverKubeVersion, and an AtLeast method to centralise version discovery and comparison. The manager now checks the cluster version at startup and only registers the DRA controller when the minimum version is met. The webhook package is updated to use the shared version package instead of its own local types. Also fixes a pre-existing bug where /run was missing from the allowed hostPath prefixes error message.
1 parent dfccbc7 commit 36567c5

7 files changed

Lines changed: 194 additions & 101 deletions

File tree

‎cmd/manager/main.go‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ import (
4242
"github.com/kubernetes-sigs/kernel-module-management/internal/metrics"
4343
"github.com/kubernetes-sigs/kernel-module-management/internal/module"
4444
"github.com/kubernetes-sigs/kernel-module-management/internal/nmc"
45+
"github.com/kubernetes-sigs/kernel-module-management/internal/version"
4546
resourcev1 "k8s.io/api/resource/v1"
4647
"k8s.io/apimachinery/pkg/runtime"
4748
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
@@ -113,7 +114,16 @@ func main() {
113114
}
114115

115116
options := cg.GetManagerOptionsFromConfig(cfg, scheme)
116-
mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), options)
117+
restCfg := ctrl.GetConfigOrDie()
118+
119+
kubeVersion, err := version.DiscoverKubeVersion(restCfg)
120+
if err != nil {
121+
cmd.FatalError(setupLogger, err, "unable to discover Kubernetes version")
122+
}
123+
124+
setupLogger.Info("Detected Kubernetes version", "major", kubeVersion.Major, "minor", kubeVersion.Minor)
125+
126+
mgr, err := ctrl.NewManager(restCfg, options)
117127
if err != nil {
118128
cmd.FatalError(setupLogger, err, "unable to create manager")
119129
}
@@ -170,8 +180,16 @@ func main() {
170180
cmd.FatalError(setupLogger, err, "unable to create controller", "name", controllers.PodNodeLabelReconcilerName)
171181
}
172182

173-
if err = controllers.NewDRAReconciler(client, nodeAPI, scheme).SetupWithManager(mgr); err != nil {
174-
cmd.FatalError(setupLogger, err, "unable to create controller", "name", controllers.DRAReconcilerName)
183+
if kubeVersion.AtLeast(constants.MinKubeMajorForDRA, constants.MinKubeMinorForDRA) {
184+
if err = controllers.NewDRAReconciler(client, nodeAPI, scheme).SetupWithManager(mgr); err != nil {
185+
cmd.FatalError(setupLogger, err, "unable to create controller", "name", controllers.DRAReconcilerName)
186+
}
187+
} else {
188+
setupLogger.Info(
189+
fmt.Sprintf("Skipping DRA controller; requires Kubernetes >= %d.%d", constants.MinKubeMajorForDRA, constants.MinKubeMinorForDRA),
190+
"detectedMajor", kubeVersion.Major,
191+
"detectedMinor", kubeVersion.Minor,
192+
)
175193
}
176194

177195
if err = controllers.NewNodeLabelModuleVersionReconciler(client).SetupWithManager(mgr); err != nil {

‎cmd/webhook-server/main.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"github.com/kubernetes-sigs/kernel-module-management/internal/cmd"
1010
"github.com/kubernetes-sigs/kernel-module-management/internal/config"
1111
"github.com/kubernetes-sigs/kernel-module-management/internal/constants"
12+
"github.com/kubernetes-sigs/kernel-module-management/internal/version"
1213
"github.com/kubernetes-sigs/kernel-module-management/internal/webhook"
1314
"github.com/kubernetes-sigs/kernel-module-management/internal/webhook/hub"
1415
"k8s.io/apimachinery/pkg/runtime"
@@ -85,7 +86,7 @@ func main() {
8586
if enableModule {
8687
logger.Info("Enabling Module webhook")
8788

88-
kubeVersion, err := webhook.DiscoverKubeVersion(restCfg)
89+
kubeVersion, err := version.DiscoverKubeVersion(restCfg)
8990
if err != nil {
9091
cmd.FatalError(setupLogger, err, "unable to discover Kubernetes version")
9192
}

‎internal/version/version.go‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
/*
2+
Copyright 2022.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package version
18+
19+
import (
20+
"fmt"
21+
"regexp"
22+
"strconv"
23+
24+
"k8s.io/client-go/discovery"
25+
"k8s.io/client-go/rest"
26+
)
27+
28+
var kubeVersionRe = regexp.MustCompile(`^v?(\d+)\.(\d+)`)
29+
30+
// KubeVersion holds the parsed major and minor version of a Kubernetes cluster.
31+
type KubeVersion struct {
32+
Major int
33+
Minor int
34+
}
35+
36+
// AtLeast returns true if kv is greater than or equal to the given major.minor version.
37+
func (kv KubeVersion) AtLeast(major, minor int) bool {
38+
return kv.Major > major || (kv.Major == major && kv.Minor >= minor)
39+
}
40+
41+
// DiscoverKubeVersion queries the Kubernetes API server and returns its version.
42+
func DiscoverKubeVersion(cfg *rest.Config) (KubeVersion, error) {
43+
discClient, err := discovery.NewDiscoveryClientForConfig(cfg)
44+
if err != nil {
45+
return KubeVersion{}, fmt.Errorf("failed to create discovery client: %v", err)
46+
}
47+
48+
serverVersion, err := discClient.ServerVersion()
49+
if err != nil {
50+
return KubeVersion{}, fmt.Errorf("failed to query Kubernetes server version: %v", err)
51+
}
52+
53+
return ParseKubeVersion(serverVersion.GitVersion)
54+
}
55+
56+
// ParseKubeVersion extracts the major and minor version from a Kubernetes
57+
// GitVersion string such as "v1.34.0" or "v1.34.0+k3s1".
58+
func ParseKubeVersion(gitVersion string) (KubeVersion, error) {
59+
m := kubeVersionRe.FindStringSubmatch(gitVersion)
60+
if m == nil {
61+
return KubeVersion{}, fmt.Errorf("cannot parse Kubernetes version from %q", gitVersion)
62+
}
63+
64+
major, err := strconv.Atoi(m[1])
65+
if err != nil {
66+
return KubeVersion{}, fmt.Errorf("cannot parse Kubernetes major version from %q: %w", gitVersion, err)
67+
}
68+
minor, err := strconv.Atoi(m[2])
69+
if err != nil {
70+
return KubeVersion{}, fmt.Errorf("cannot parse Kubernetes minor version from %q: %w", gitVersion, err)
71+
}
72+
return KubeVersion{Major: major, Minor: minor}, nil
73+
}

‎internal/version/version_test.go‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
/*
2+
Copyright 2022.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package version
18+
19+
import (
20+
"testing"
21+
22+
. "github.com/onsi/ginkgo/v2"
23+
. "github.com/onsi/gomega"
24+
)
25+
26+
func TestSuite(t *testing.T) {
27+
RegisterFailHandler(Fail)
28+
RunSpecs(t, "Version Suite")
29+
}
30+
31+
var _ = Describe("AtLeast", func() {
32+
DescribeTable(
33+
"should compare versions correctly",
34+
func(major, minor, minMajor, minMinor int, expected bool) {
35+
kv := KubeVersion{Major: major, Minor: minor}
36+
Expect(kv.AtLeast(minMajor, minMinor)).To(Equal(expected))
37+
},
38+
Entry("exact match", 1, 34, 1, 34, true),
39+
Entry("higher minor", 1, 35, 1, 34, true),
40+
Entry("lower minor", 1, 33, 1, 34, false),
41+
Entry("higher major, lower minor", 2, 0, 1, 34, true),
42+
Entry("lower major", 0, 99, 1, 34, false),
43+
)
44+
})
45+
46+
var _ = Describe("ParseKubeVersion", func() {
47+
DescribeTable(
48+
"should parse valid version strings",
49+
func(gitVersion string, expectedMajor, expectedMinor int) {
50+
kv, err := ParseKubeVersion(gitVersion)
51+
Expect(err).NotTo(HaveOccurred())
52+
Expect(kv.Major).To(Equal(expectedMajor))
53+
Expect(kv.Minor).To(Equal(expectedMinor))
54+
},
55+
Entry("standard version", "v1.34.0", 1, 34),
56+
Entry("with build metadata", "v1.34.0+k3s1", 1, 34),
57+
Entry("pre-release version", "v1.33.0-alpha.1", 1, 33),
58+
Entry("higher minor", "v1.35.1", 1, 35),
59+
Entry("without v prefix", "1.34.0", 1, 34),
60+
Entry("hypothetical k8s 2.0", "v2.0.0", 2, 0),
61+
Entry("hypothetical k8s 2.5", "v2.5.3", 2, 5),
62+
)
63+
64+
DescribeTable(
65+
"should return error for invalid strings",
66+
func(gitVersion string) {
67+
_, err := ParseKubeVersion(gitVersion)
68+
Expect(err).To(HaveOccurred())
69+
},
70+
Entry("empty string", ""),
71+
Entry("garbage", "invalid"),
72+
Entry("no minor", "v1"),
73+
)
74+
})

‎internal/webhook/hub/managedclustermodule_webhook.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"github.com/go-logr/logr"
2424
"github.com/kubernetes-sigs/kernel-module-management/api-hub/v1beta1"
2525
kmmv1beta1 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1"
26+
"github.com/kubernetes-sigs/kernel-module-management/internal/version"
2627
"github.com/kubernetes-sigs/kernel-module-management/internal/webhook"
2728
"k8s.io/apimachinery/pkg/runtime"
2829
ctrl "sigs.k8s.io/controller-runtime"
@@ -34,7 +35,7 @@ type ManagedClusterModuleValidator struct {
3435
m admission.CustomValidator
3536
}
3637

37-
func NewManagedClusterModuleValidator(logger logr.Logger, kubeVersion *webhook.KubeVersion) *ManagedClusterModuleValidator {
38+
func NewManagedClusterModuleValidator(logger logr.Logger, kubeVersion *version.KubeVersion) *ManagedClusterModuleValidator {
3839
return &ManagedClusterModuleValidator{
3940
logger: logger,
4041
m: webhook.NewModuleValidator(logger, kubeVersion),

‎internal/webhook/module.go‎

Lines changed: 11 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import (
2222
"fmt"
2323
"path/filepath"
2424
"regexp"
25-
"strconv"
2625
"strings"
2726

2827
corev1 "k8s.io/api/core/v1"
@@ -31,10 +30,9 @@ import (
3130
"github.com/go-logr/logr"
3231
kmmv1beta1 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1"
3332
"github.com/kubernetes-sigs/kernel-module-management/internal/constants"
33+
"github.com/kubernetes-sigs/kernel-module-management/internal/version"
3434
"k8s.io/apimachinery/pkg/runtime"
3535
"k8s.io/apimachinery/pkg/util/sets"
36-
"k8s.io/client-go/discovery"
37-
"k8s.io/client-go/rest"
3836
ctrl "sigs.k8s.io/controller-runtime"
3937
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
4038
)
@@ -43,56 +41,15 @@ import (
4341
// 63 (max label key length after slash) - len("version-schedule-plugin.") = 38
4442
const maxCombinedLength = 38
4543

46-
var kubeVersionRe = regexp.MustCompile(`^v?(\d+)\.(\d+)`)
47-
48-
// KubeVersion holds the parsed major and minor version of a Kubernetes cluster.
49-
type KubeVersion struct {
50-
Major int
51-
Minor int
52-
}
53-
5444
type ModuleValidator struct {
5545
logger logr.Logger
56-
kubeVersion *KubeVersion
46+
kubeVersion *version.KubeVersion
5747
}
5848

59-
func NewModuleValidator(logger logr.Logger, kubeVersion *KubeVersion) *ModuleValidator {
49+
func NewModuleValidator(logger logr.Logger, kubeVersion *version.KubeVersion) *ModuleValidator {
6050
return &ModuleValidator{logger: logger, kubeVersion: kubeVersion}
6151
}
6252

63-
// DiscoverKubeVersion queries the Kubernetes API server and returns its version.
64-
func DiscoverKubeVersion(cfg *rest.Config) (KubeVersion, error) {
65-
discClient, err := discovery.NewDiscoveryClientForConfig(cfg)
66-
if err != nil {
67-
return KubeVersion{}, fmt.Errorf("failed to create discovery client: %v", err)
68-
}
69-
70-
serverVersion, err := discClient.ServerVersion()
71-
if err != nil {
72-
return KubeVersion{}, fmt.Errorf("failed to query Kubernetes server version: %v", err)
73-
}
74-
75-
return parseKubeVersion(serverVersion.GitVersion)
76-
}
77-
78-
// parseKubeVersion extracts the major and minor version from a Kubernetes
79-
// GitVersion string such as "v1.34.0" or "v1.34.0+k3s1".
80-
func parseKubeVersion(gitVersion string) (KubeVersion, error) {
81-
m := kubeVersionRe.FindStringSubmatch(gitVersion)
82-
if m == nil {
83-
return KubeVersion{}, fmt.Errorf("cannot parse Kubernetes version from %q", gitVersion)
84-
}
85-
major, err := strconv.Atoi(m[1])
86-
if err != nil {
87-
return KubeVersion{}, fmt.Errorf("cannot parse Kubernetes major version from %q: %v", gitVersion, err)
88-
}
89-
minor, err := strconv.Atoi(m[2])
90-
if err != nil {
91-
return KubeVersion{}, fmt.Errorf("cannot parse Kubernetes minor version from %q: %v", gitVersion, err)
92-
}
93-
return KubeVersion{Major: major, Minor: minor}, nil
94-
}
95-
9653
func (m *ModuleValidator) SetupWebhookWithManager(mgr ctrl.Manager) error {
9754
// controller-runtime will set the path to `validate-<group>-<version>-<resource> so we
9855
// need to make sure it is set correctly in the +kubebuilder annotation below.
@@ -145,7 +102,7 @@ func (m *ModuleValidator) ValidateDelete(ctx context.Context, obj runtime.Object
145102
return nil, NotImplemented
146103
}
147104

148-
func validateModule(mod *kmmv1beta1.Module, kubeVersion *KubeVersion) (admission.Warnings, error) {
105+
func validateModule(mod *kmmv1beta1.Module, kubeVersion *version.KubeVersion) (admission.Warnings, error) {
149106
nameLength := len(mod.Name + mod.Namespace)
150107

151108
if nameLength > maxCombinedLength {
@@ -192,20 +149,18 @@ func validateModule(mod *kmmv1beta1.Module, kubeVersion *KubeVersion) (admission
192149
return nil, validateFilesToSign(mod.Spec.ModuleLoader.Container)
193150
}
194151

195-
func validateDRA(mod *kmmv1beta1.Module, kubeVersion *KubeVersion) error {
152+
func validateDRA(mod *kmmv1beta1.Module, kubeVersion *version.KubeVersion) error {
196153
if mod.Spec.DRA == nil {
197154
return nil
198155
}
199156

200157
// Skip version gating when kubeVersion is nil (e.g. hub webhook validating
201158
// a ManagedClusterModule — the spoke's own webhook will enforce its version).
202-
if kubeVersion != nil {
203-
if kubeVersion.Major < constants.MinKubeMajorForDRA || (kubeVersion.Major == constants.MinKubeMajorForDRA && kubeVersion.Minor < constants.MinKubeMinorForDRA) {
204-
return fmt.Errorf(
205-
"spec.dra requires Kubernetes %d.%d or later; current cluster version is %d.%d",
206-
constants.MinKubeMajorForDRA, constants.MinKubeMinorForDRA, kubeVersion.Major, kubeVersion.Minor,
207-
)
208-
}
159+
if kubeVersion != nil && !kubeVersion.AtLeast(constants.MinKubeMajorForDRA, constants.MinKubeMinorForDRA) {
160+
return fmt.Errorf(
161+
"spec.dra requires Kubernetes %d.%d or later; current cluster version is %d.%d",
162+
constants.MinKubeMajorForDRA, constants.MinKubeMinorForDRA, kubeVersion.Major, kubeVersion.Minor,
163+
)
209164
}
210165

211166
driverName := mod.Spec.DRA.DriverName
@@ -252,7 +207,7 @@ func validateHostPathVolumes(fieldPath string, volumes []corev1.Volume) error {
252207
for i, vol := range volumes {
253208
if vol.HostPath != nil && !isAllowedHostPath(vol.HostPath.Path) {
254209
return fmt.Errorf(
255-
"%s.volumes[%d]: hostPath %q is not allowed; only /dev, /sys, /var and /opt paths are permitted",
210+
"%s.volumes[%d]: hostPath %q is not allowed; only /dev, /sys, /var, /opt and /run paths are permitted",
256211
fieldPath, i, vol.HostPath.Path,
257212
)
258213
}

0 commit comments

Comments
 (0)