From 5def257b3373f1f1d49cff18749efcf9a08de806 Mon Sep 17 00:00:00 2001 From: cyberchen98 Date: Wed, 19 Jun 2024 17:45:24 +0800 Subject: [PATCH 1/3] refactor: moves the admission-webhook code into pkg/webhook from api --- Makefile | 14 +- api/core/v1alpha1/cnset_webhook.go | 136 ------------- api/core/v1alpha1/common_helpers.go | 3 +- api/core/v1alpha1/dnset_webhook.go | 99 ---------- api/core/v1alpha1/logset_helpers.go | 32 +++ api/core/v1alpha1/logset_types.go | 5 + api/core/v1alpha1/matrixonecluster_webhook.go | 138 ------------- api/core/v1alpha1/zz_generated.deepcopy.go | 2 +- cmd/operator/main.go | 3 +- go.mod | 3 + go.sum | 3 + pkg/webhook/cnset_webhook.go | 158 +++++++++++++++ .../webhook}/cnset_webhook_test.go | 59 +++--- .../webhook.go => pkg/webhook/common.go | 43 +--- pkg/webhook/dnset_webhook.go | 120 +++++++++++ .../webhook}/dnset_webhook_test.go | 51 ++--- .../webhook}/logset_webhook.go | 186 ++++++++++-------- .../webhook}/logset_webhook_test.go | 35 ++-- pkg/webhook/matrixonecluster_webhook.go | 174 ++++++++++++++++ .../webhook}/matrixonecluster_webhook_test.go | 175 ++++++++-------- .../v1alpha1 => pkg/webhook}/proxy_webhook.go | 45 +++-- .../webhook}/proxy_webhook_test.go | 20 +- pkg/webhook/utils.go | 97 +++++++++ pkg/webhook/webhook.go | 44 +++++ .../webhook}/webhook_suite_test.go | 12 +- .../v1alpha1 => pkg/webhook}/webui_webhook.go | 47 +++-- test/e2e/matrixonecluster_test.go | 3 +- 27 files changed, 1012 insertions(+), 695 deletions(-) delete mode 100644 api/core/v1alpha1/cnset_webhook.go delete mode 100644 api/core/v1alpha1/dnset_webhook.go delete mode 100644 api/core/v1alpha1/matrixonecluster_webhook.go create mode 100644 pkg/webhook/cnset_webhook.go rename {api/core/v1alpha1 => pkg/webhook}/cnset_webhook_test.go (71%) rename api/core/v1alpha1/webhook.go => pkg/webhook/common.go (62%) create mode 100644 pkg/webhook/dnset_webhook.go rename {api/core/v1alpha1 => pkg/webhook}/dnset_webhook_test.go (72%) rename {api/core/v1alpha1 => pkg/webhook}/logset_webhook.go (51%) rename {api/core/v1alpha1 => pkg/webhook}/logset_webhook_test.go (74%) create mode 100644 pkg/webhook/matrixonecluster_webhook.go rename {api/core/v1alpha1 => pkg/webhook}/matrixonecluster_webhook_test.go (69%) rename {api/core/v1alpha1 => pkg/webhook}/proxy_webhook.go (50%) rename {api/core/v1alpha1 => pkg/webhook}/proxy_webhook_test.go (76%) create mode 100644 pkg/webhook/utils.go create mode 100644 pkg/webhook/webhook.go rename {api/core/v1alpha1 => pkg/webhook}/webhook_suite_test.go (93%) rename {api/core/v1alpha1 => pkg/webhook}/webui_webhook.go (51%) diff --git a/Makefile b/Makefile index 5221ac72..7342f9fc 100644 --- a/Makefile +++ b/Makefile @@ -7,6 +7,7 @@ GOPROXY ?= "https://proxy.golang.org,direct" MO_VERSION ?= "nightly-d98832bb" MO_IMAGE_REPO ?= "matrixorigin/matrixone" BRANCH ?= main +ENVTEST_K8S_VERSION = 1.24.1 # Get the currently used golang install path (in GOPATH/bin, unless GOBIN is set) ifeq (,$(shell go env GOBIN)) @@ -86,12 +87,23 @@ go-lint: golangci-lint check-license: license-eye $(LICENSE_EYE) -v info -c .licenserc.yml header check +LOCALBIN ?= $(shell pwd)/bin +$(LOCALBIN): + mkdir -p $(LOCALBIN) + +ENVTEST ?= $(LOCALBIN)/setup-envtest + +.PHONY: envtest +envtest: $(ENVTEST) ## Download envtest-setup locally if necessary. +$(ENVTEST): $(LOCALBIN) + test -s $(LOCALBIN)/setup-envtest || GOBIN=$(LOCALBIN) go install sigs.k8s.io/controller-runtime/tools/setup-envtest@latest + # TODO: include E2E test: api-test unit # Run unit tests unit: generate fmt vet manifests - CGO_ENABLED=0 go test ./pkg/... -coverprofile cover.out + KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" CGO_ENABLED=0 go test ./pkg/... -coverprofile cover.out api-test: cd api && make test diff --git a/api/core/v1alpha1/cnset_webhook.go b/api/core/v1alpha1/cnset_webhook.go deleted file mode 100644 index 0cf36f99..00000000 --- a/api/core/v1alpha1/cnset_webhook.go +++ /dev/null @@ -1,136 +0,0 @@ -// Copyright 2024 Matrix Origin -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package v1alpha1 - -import ( - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/util/intstr" - "k8s.io/apimachinery/pkg/util/validation/field" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/webhook" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission" - "time" -) - -const ( - defaultStoreDrainTimeout = 5 * time.Minute - - defaultMaxSurge = 1 - defaultMaxUnavailable = 0 -) - -func (r *CNSet) setupWebhookWithManager(mgr ctrl.Manager) error { - return ctrl.NewWebhookManagedBy(mgr). - For(r). - Complete() -} - -// +kubebuilder:webhook:path=/mutate-core-matrixorigin-io-v1alpha1-cnset,mutating=true,failurePolicy=fail,sideEffects=None,groups=core.matrixorigin.io,resources=cnsets,verbs=create;update,versions=v1alpha1,name=mcnset.kb.io,admissionReviewVersions={v1,v1beta1} - -var _ webhook.Defaulter = &CNSet{} - -// Default implements webhook.Defaulter so a webhook will be registered for the type -func (r *CNSet) Default() { - r.Spec.Default() - if r.Spec.Role == "" { - r.Spec.Role = CNRoleTP - } -} - -func (r *CNSetSpec) Default() { - if r.ServiceType == "" { - r.ServiceType = corev1.ServiceTypeClusterIP - } - if r.Resources.Requests.Memory().Value() != 0 && r.SharedStorageCache.MemoryCacheSize == nil { - // default memory cache size to 50% request memory - size := r.Resources.Requests.Memory().DeepCopy() - size.Set(size.Value() / 2) - r.SharedStorageCache.MemoryCacheSize = &size - } - if r.CacheVolume != nil && r.SharedStorageCache.DiskCacheSize == nil { - // default disk cache size based on the cache volume total size - r.SharedStorageCache.DiskCacheSize = defaultDiskCacheSize(&r.CacheVolume.Size) - } - if r.ScalingConfig.StoreDrainEnabled != nil && *r.ScalingConfig.StoreDrainEnabled { - if r.ScalingConfig.StoreDrainTimeout == nil { - r.ScalingConfig.StoreDrainTimeout = &metav1.Duration{Duration: defaultStoreDrainTimeout} - } - } - if r.UpdateStrategy.MaxSurge == nil { - maxSurge := intstr.FromInt(defaultMaxSurge) - r.UpdateStrategy.MaxSurge = &maxSurge - } - if r.UpdateStrategy.MaxUnavailable == nil { - maxUnavailable := intstr.FromInt(defaultMaxUnavailable) - r.UpdateStrategy.MaxUnavailable = &maxUnavailable - } - setDefaultServiceArgs(r) - setPodSetDefaults(&r.PodSet) -} - -// +kubebuilder:webhook:path=/validate-core-matrixorigin-io-v1alpha1-cnset,mutating=false,failurePolicy=fail,sideEffects=None,groups=core.matrixorigin.io,resources=cnsets,verbs=create;update,versions=v1alpha1,name=vcnset.kb.io,admissionReviewVersions={v1,v1beta1} - -var _ webhook.Validator = &CNSet{} - -// ValidateCreate implements webhook.Validator so a webhook will be registered for the type -func (r *CNSet) ValidateCreate() (admission.Warnings, error) { - var errs field.ErrorList - errs = append(errs, validateLogSetRef(&r.Deps.LogSetRef, field.NewPath("deps"))...) - errs = append(errs, r.Spec.ValidateCreate()...) - errs = append(errs, validateMainContainer(&r.Spec.MainContainer, field.NewPath("spec"))...) - return nil, invalidOrNil(errs, r) -} - -func (r *CNSet) ValidateUpdate(old runtime.Object) (admission.Warnings, error) { - warnings, err := r.ValidateCreate() - if err != nil { - return warnings, err - } - return nil, nil -} - -func (r *CNSet) ValidateDelete() (admission.Warnings, error) { - return nil, nil -} - -func (r *CNSetSpec) ValidateCreate() field.ErrorList { - var errs field.ErrorList - if r.CacheVolume != nil { - errs = append(errs, validateVolume(r.CacheVolume, field.NewPath("spec").Child("cacheVolume"))...) - } - if r.ServiceType == corev1.ServiceTypeExternalName { - errs = append(errs, field.Invalid(field.NewPath("spec").Child("serviceType"), r.ServiceType, "must be one of [ClusterIP, NodePort, LoadBalancer]")) - } - if r.NodePort != nil && r.ServiceType == corev1.ServiceTypeClusterIP { - errs = append(errs, field.Invalid(field.NewPath("spec").Child("nodePort"), r.NodePort, "cannot set node port when serviceType is ClusterIP")) - } - for i, l := range r.Labels { - if l.Key == "" { - errs = append(errs, field.Invalid(field.NewPath("spec").Child("cnLabels").Index(i).Child("key"), r.Labels[i], "label key cannot be empty")) - } - if len(l.Values) == 0 { - errs = append(errs, field.Invalid(field.NewPath("spec").Child("cnLabels").Index(i).Child("values"), r.Labels[i], "label values cannot be empty")) - } - for j, v := range l.Values { - if v == "" { - errs = append(errs, field.Invalid(field.NewPath("spec").Child("cnLabels").Index(i).Child("values").Index(j), r.Labels[i].Values, "label value cannot be empty string")) - } - } - } - errs = append(errs, validateGoMemLimitPercent(r.MemoryLimitPercent, field.NewPath("spec").Child("memoryLimitPercent"))...) - return errs -} diff --git a/api/core/v1alpha1/common_helpers.go b/api/core/v1alpha1/common_helpers.go index 46727bf4..fbf4856b 100644 --- a/api/core/v1alpha1/common_helpers.go +++ b/api/core/v1alpha1/common_helpers.go @@ -24,6 +24,7 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + logf "sigs.k8s.io/controller-runtime/pkg/log" "strings" "time" ) @@ -239,7 +240,7 @@ func setDefaultServiceArgs(object interface{}) { obj.ServiceArgs = ServiceDefaultArgs.Proxy } default: - moLog.Error(fmt.Errorf("unknown type:%T", object), "expected types: *LogSetSpec, *DNSetSpec, *CNSetSpec") + logf.Log.WithName("mo-cluster").Error(fmt.Errorf("unknown type:%T", object), "expected types: *LogSetSpec, *DNSetSpec, *CNSetSpec") return } } diff --git a/api/core/v1alpha1/dnset_webhook.go b/api/core/v1alpha1/dnset_webhook.go deleted file mode 100644 index bdffaff3..00000000 --- a/api/core/v1alpha1/dnset_webhook.go +++ /dev/null @@ -1,99 +0,0 @@ -// Copyright 2024 Matrix Origin -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package v1alpha1 - -import ( - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/util/validation/field" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/webhook" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission" -) - -func (r *DNSet) setupWebhookWithManager(mgr ctrl.Manager) error { - return ctrl.NewWebhookManagedBy(mgr). - For(r). - Complete() -} - -// +kubebuilder:webhook:path=/mutate-core-matrixorigin-io-v1alpha1-dnset,mutating=true,failurePolicy=fail,sideEffects=None,groups=core.matrixorigin.io,resources=dnsets,verbs=create;update,versions=v1alpha1,name=mdnset.kb.io,admissionReviewVersions={v1,v1beta1} - -var _ webhook.Defaulter = &DNSet{} - -// Default implements webhook.Defaulter so a webhook will be registered for the type -func (r *DNSet) Default() { - r.Spec.Default() -} - -func (r *DNSetSpec) Default() { - if r.Resources.Requests.Memory() != nil && r.SharedStorageCache.MemoryCacheSize == nil { - // default memory cache size to 50% request memory - size := r.Resources.Requests.Memory().DeepCopy() - size.Set(size.Value() / 2) - r.SharedStorageCache.MemoryCacheSize = &size - } - if r.CacheVolume != nil && r.SharedStorageCache.DiskCacheSize == nil { - // default disk cache size based on the cache volume total size - r.SharedStorageCache.DiskCacheSize = defaultDiskCacheSize(&r.CacheVolume.Size) - } - setDefaultServiceArgs(r) - setPodSetDefaults(&r.PodSet) -} - -// +kubebuilder:webhook:path=/validate-core-matrixorigin-io-v1alpha1-dnset,mutating=false,failurePolicy=fail,sideEffects=None,groups=core.matrixorigin.io,resources=dnsets,verbs=create;update,versions=v1alpha1,name=vdnset.kb.io,admissionReviewVersions={v1,v1beta1} - -var _ webhook.Validator = &DNSet{} - -// ValidateCreate implements webhook.Validator so a webhook will be registered for the type -func (r *DNSet) ValidateCreate() (admission.Warnings, error) { - var errs field.ErrorList - errs = append(errs, validateLogSetRef(&r.Deps.LogSetRef, field.NewPath("deps"))...) - errs = append(errs, r.Spec.ValidateCreate()...) - errs = append(errs, validateMainContainer(&r.Spec.MainContainer, field.NewPath("spec"))...) - errs = append(errs, r.Spec.validateConfig(r.Spec.Config, field.NewPath("spec").Child("config"))...) - return nil, invalidOrNil(errs, r) -} - -func (r *DNSet) ValidateUpdate(old runtime.Object) (admission.Warnings, error) { - warnings, err := r.ValidateCreate() - if err != nil { - return warnings, err - } - return nil, nil -} - -func (r *DNSet) ValidateDelete() (admission.Warnings, error) { - return nil, nil -} - -func (r *DNSetSpec) ValidateCreate() field.ErrorList { - var errs field.ErrorList - if r.CacheVolume != nil { - errs = append(errs, validateVolume(r.CacheVolume, field.NewPath("spec").Child("cacheVolume"))...) - } - errs = append(errs, validateGoMemLimitPercent(r.MemoryLimitPercent, field.NewPath("spec").Child("memoryLimitPercent"))...) - return errs -} - -func (r *DNSetSpec) validateConfig(c *TomlConfig, path *field.Path) field.ErrorList { - var errs field.ErrorList - if c == nil { - return errs - } - if c.Get("tn") != nil && c.Get("dn") != nil { - errs = append(errs, field.Invalid(path, c, "[tn] and [dn] cannot be set at the same time")) - } - return errs -} diff --git a/api/core/v1alpha1/logset_helpers.go b/api/core/v1alpha1/logset_helpers.go index 9047997b..58a973eb 100644 --- a/api/core/v1alpha1/logset_helpers.go +++ b/api/core/v1alpha1/logset_helpers.go @@ -28,3 +28,35 @@ func (l *LogSet) AsDependency() LogSetRef { }, } } + +// setDefaultRetentionPolicy always set PVCRetentionPolicy, and always set S3RetentionPolicy only if S3 is not nil +// setDefaultRetentionPolicy does not change origin policy and only set default value when policy is nil +func (l *LogSetSpec) setDefaultRetentionPolicy() { + defaultDeletePolicy := PVCRetentionPolicyDelete + + if l.SharedStorage.S3 == nil { + if l.PVCRetentionPolicy == nil { + l.PVCRetentionPolicy = &defaultDeletePolicy + } + return + } + + pvcPolicy := l.PVCRetentionPolicy + s3Policy := l.SharedStorage.S3.S3RetentionPolicy + + switch { + // if both set, does not set any values + case pvcPolicy != nil && s3Policy != nil: + return + // if both not set, set to delete + case pvcPolicy == nil && s3Policy == nil: + l.PVCRetentionPolicy = &defaultDeletePolicy + l.SharedStorage.S3.S3RetentionPolicy = &defaultDeletePolicy + // if only set pvcPolicy, set it to s3Policy + case pvcPolicy != nil && s3Policy == nil: + l.SharedStorage.S3.S3RetentionPolicy = pvcPolicy + // if only set s3Policy, set it to pvcPolicy + case pvcPolicy == nil && s3Policy != nil: + l.PVCRetentionPolicy = s3Policy + } +} diff --git a/api/core/v1alpha1/logset_types.go b/api/core/v1alpha1/logset_types.go index c23d7e18..5ff1f436 100644 --- a/api/core/v1alpha1/logset_types.go +++ b/api/core/v1alpha1/logset_types.go @@ -17,6 +17,7 @@ package v1alpha1 import ( "fmt" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "time" ) const ( @@ -31,6 +32,10 @@ const ( FailedPodStrategyDelete FailedPodStrategy = "Delete" ) +const ( + defaultStoreFailureTimeout = 10 * time.Minute +) + type LogSetSpec struct { PodSet `json:",inline"` diff --git a/api/core/v1alpha1/matrixonecluster_webhook.go b/api/core/v1alpha1/matrixonecluster_webhook.go deleted file mode 100644 index 0c6732fd..00000000 --- a/api/core/v1alpha1/matrixonecluster_webhook.go +++ /dev/null @@ -1,138 +0,0 @@ -// Copyright 2024 Matrix Origin -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package v1alpha1 - -import ( - "fmt" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/util/validation" - "k8s.io/apimachinery/pkg/util/validation/field" - 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" -) - -const ( - // MatrixOneClusterNameMaxLength is the maximum length of a MatrixOneCluster name - MatrixOneClusterNameMaxLength = 46 -) - -// log is for logging in this package. -var moLog = logf.Log.WithName("mo-cluster") - -func (r *MatrixOneCluster) setupWebhookWithManager(mgr ctrl.Manager) error { - return ctrl.NewWebhookManagedBy(mgr). - For(r). - Complete() -} - -// +kubebuilder:webhook:path=/mutate-core-matrixorigin-io-v1alpha1-matrixonecluster,mutating=true,failurePolicy=fail,sideEffects=None,groups=core.matrixorigin.io,resources=matrixoneclusters,verbs=create;update,versions=v1alpha1,name=mmatrixonecluster.kb.io,admissionReviewVersions=v1;v1beta1 - -var _ webhook.Defaulter = &MatrixOneCluster{} - -// Default implements webhook.Defaulter so a webhook will be registered for the type -func (r *MatrixOneCluster) Default() { - r.Spec.LogService.Default() - if r.Spec.DN != nil { - r.Spec.DN.Default() - } - if r.Spec.TN != nil { - r.Spec.TN.Default() - } - if r.Spec.TP != nil { - r.Spec.TP.Default() - } - if r.Spec.AP != nil { - r.Spec.AP.Default() - } - for i := range r.Spec.CNGroups { - r.Spec.CNGroups[i].Default() - } -} - -// +kubebuilder:webhook:path=/validate-core-matrixorigin-io-v1alpha1-matrixonecluster,mutating=false,failurePolicy=fail,sideEffects=None,groups=core.matrixorigin.io,resources=matrixoneclusters,verbs=create;update,versions=v1alpha1,name=vmatrixonecluster.kb.io,admissionReviewVersions=v1;v1beta1 - -var _ webhook.Validator = &MatrixOneCluster{} - -// ValidateCreate implements webhook.Validator so a webhook will be registered for the type -func (r *MatrixOneCluster) ValidateCreate() (admission.Warnings, error) { - var errs field.ErrorList - if len(r.Name) > MatrixOneClusterNameMaxLength { - errs = append(errs, field.Invalid(field.NewPath("metadata").Child("name"), r.Name, fmt.Sprintf("must be no more than %d characters", MatrixOneClusterNameMaxLength))) - } - dns1035ErrorList := validation.IsDNS1035Label(r.Name) - for _, errMsg := range dns1035ErrorList { - errs = append(errs, field.Invalid(field.NewPath("metadata").Child("name"), r.Name, errMsg)) - } - if r.Spec.DN == nil && r.Spec.TN == nil { - errs = append(errs, field.Invalid(field.NewPath("spec").Child("tn"), "", ".spec.tn must be set")) - } - if r.Spec.DN != nil && r.Spec.TN != nil { - errs = append(errs, field.Invalid(field.NewPath("spec").Child("dn"), "", "legacy component .spec.dn cannot be set when .spec.tn is set")) - } - errs = append(errs, r.validateMutateCommon()...) - errs = append(errs, r.Spec.LogService.ValidateCreate(LogSetKey(r))...) - return nil, invalidOrNil(errs, r) -} - -func (r *MatrixOneCluster) ValidateUpdate(o runtime.Object) (admission.Warnings, error) { - var errs field.ErrorList - errs = append(errs, r.validateMutateCommon()...) - - old := o.(*MatrixOneCluster) - errs = append(errs, r.Spec.LogService.ValidateUpdate(&old.Spec.LogService, LogSetKey(r))...) - return nil, invalidOrNil(errs, r) -} - -func (r *MatrixOneCluster) validateMutateCommon() field.ErrorList { - var errs field.ErrorList - errs = append(errs, r.GetTN().ValidateCreate()...) - groups := map[string]bool{} - if r.Spec.TP != nil { - errs = append(errs, r.Spec.TP.ValidateCreate()...) - groups["tp"] = true - } - if r.Spec.AP != nil { - errs = append(errs, r.Spec.AP.ValidateCreate()...) - groups["ap"] = true - } - for i, cn := range r.Spec.CNGroups { - errs = append(errs, r.validateCNGroup(cn, field.NewPath("spec").Child("cnGroups").Index(i))...) - if groups[cn.Name] { - errs = append(errs, field.Invalid(field.NewPath("spec").Child("cnGroups").Index(i).Child("name"), cn.Name, "name must be unique")) - } - groups[cn.Name] = true - } - if r.Spec.Version == "" { - errs = append(errs, field.Invalid(field.NewPath("spec").Child("version"), "", "version must be set")) - } - return errs -} - -func (r *MatrixOneCluster) validateCNGroup(g CNGroup, parent *field.Path) field.ErrorList { - var errs field.ErrorList - if es := validation.IsDNS1123Subdomain(g.Name); es != nil { - for _, err := range es { - errs = append(errs, field.Invalid(parent.Child("name"), g.Name, err)) - } - } - errs = append(errs, g.CNSetSpec.ValidateCreate()...) - return errs -} - -func (r *MatrixOneCluster) ValidateDelete() (admission.Warnings, error) { - return nil, nil -} diff --git a/api/core/v1alpha1/zz_generated.deepcopy.go b/api/core/v1alpha1/zz_generated.deepcopy.go index 55ba0564..51faa971 100644 --- a/api/core/v1alpha1/zz_generated.deepcopy.go +++ b/api/core/v1alpha1/zz_generated.deepcopy.go @@ -22,7 +22,7 @@ package v1alpha1 import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" + runtime "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/intstr" ) diff --git a/cmd/operator/main.go b/cmd/operator/main.go index 6d1f301c..26310223 100644 --- a/cmd/operator/main.go +++ b/cmd/operator/main.go @@ -44,6 +44,7 @@ import ( "github.com/matrixorigin/matrixone-operator/pkg/controllers/mocluster" hookctrl "github.com/matrixorigin/matrixone-operator/pkg/controllers/webhook" "github.com/matrixorigin/matrixone-operator/pkg/controllers/webui" + mowebhook "github.com/matrixorigin/matrixone-operator/pkg/webhook" kruisev1 "github.com/openkruise/kruise-api/apps/v1beta1" "go.uber.org/zap/zapcore" controllermetrics "sigs.k8s.io/controller-runtime/pkg/metrics" @@ -144,7 +145,7 @@ func main() { if os.Getenv("ENABLE_WEBHOOKS") != "false" { v1alpha1.ServiceDefaultArgs = operatorCfg.DefaultArgs - err := v1alpha1.RegisterWebhooks(mgr) + err := mowebhook.RegisterWebhooks(mgr) exitIf(err, "unable to set up webhook") caBundle, err := os.ReadFile(fmt.Sprintf("%s/%s", webhookCertDir, caFile)) diff --git a/go.mod b/go.mod index 05aa4d4c..553d386a 100644 --- a/go.mod +++ b/go.mod @@ -17,6 +17,7 @@ require ( github.com/matrixorigin/matrixone v0.7.1-0.20240509144206-2eeef9246a17 github.com/matrixorigin/matrixone-operator/api v0.0.0-20220926063007-e629f86256d2 github.com/minio/minio-go/v7 v7.0.63 + github.com/onsi/ginkgo v1.16.5 github.com/onsi/ginkgo/v2 v2.9.5 github.com/onsi/gomega v1.27.7 github.com/openkruise/kruise-api v1.4.0 @@ -128,6 +129,7 @@ require ( github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826 // indirect github.com/mschoch/smat v0.2.0 // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect + github.com/nxadm/tail v1.4.8 // indirect github.com/opentracing/opentracing-go v1.2.1-0.20220228012449-10b1cf09e00b // indirect github.com/panjf2000/ants/v2 v2.7.4 // indirect github.com/pelletier/go-toml v1.9.5 // indirect @@ -170,6 +172,7 @@ require ( gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/ini.v1 v1.67.0 // indirect gopkg.in/natefinch/lumberjack.v2 v2.2.1 // indirect + gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect k8s.io/apiextensions-apiserver v0.27.2 // indirect diff --git a/go.sum b/go.sum index 660de491..095e1929 100644 --- a/go.sum +++ b/go.sum @@ -194,6 +194,7 @@ github.com/go-openapi/swag v0.22.3 h1:yMBqmnQ0gyZvEb/+KzuWZOXgllrXT4SADYbvDaXHv/ github.com/go-openapi/swag v0.22.3/go.mod h1:UzaqsxGiab7freDnrUUra0MwWfN/q7tE4j+VcZ0yl14= github.com/go-sql-driver/mysql v1.7.1 h1:lUIinVbN1DY0xBg0eMOzmmtGoHwWBbvnWubQUrtU8EI= github.com/go-sql-driver/mysql v1.7.1/go.mod h1:OXbVy3sEdcQ2Doequ6Z5BW6fXNQTmx+9S1MCJN5yJMI= +github.com/go-task/slim-sprig v0.0.0-20210107165309-348f09dbbbc0/go.mod h1:fyg7847qk6SyHyPtNmDHnmrv/HOrqktSC+C9fM+CJOE= github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572 h1:tfuBGBXKqDEevZMzYi5KSi8KkcZtzBcTgAUUtapy0OI= github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572/go.mod h1:9Pwr4B2jHnOSGXyyzV8ROjYa2ojvAY6HCGYYfMoC3Ls= github.com/gobwas/httphead v0.0.0-20180130184737-2c6c146eadee/go.mod h1:L0fX3K22YWvt/FAX9NnzrNzcI4wNYi9Yku4O0LKYflo= @@ -690,6 +691,7 @@ golang.org/x/sys v0.0.0-20200519105757-fe76b779f299/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20201204225414-ed752295db88/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210112080510-489259a85091/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210403161142-5e06dd20ab57/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= @@ -748,6 +750,7 @@ golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtn golang.org/x/tools v0.0.0-20200130002326-2f3ba24bd6e7/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28= golang.org/x/tools v0.0.0-20200207183749-b753a1ba74fa/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28= golang.org/x/tools v0.0.0-20200619180055-7c47624df98f/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= +golang.org/x/tools v0.0.0-20201224043029-2b0845dc783e/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= golang.org/x/tools v0.1.1/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/tools v0.1.3/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= diff --git a/pkg/webhook/cnset_webhook.go b/pkg/webhook/cnset_webhook.go new file mode 100644 index 00000000..9c4eee4b --- /dev/null +++ b/pkg/webhook/cnset_webhook.go @@ -0,0 +1,158 @@ +// Copyright 2024 Matrix Origin +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package webhook + +import ( + "context" + "time" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/apimachinery/pkg/util/validation/field" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + "github.com/matrixorigin/matrixone-operator/api/core/v1alpha1" +) + +const ( + defaultStoreDrainTimeout = 5 * time.Minute + + defaultMaxSurge = 1 + defaultMaxUnavailable = 0 +) + +type cnSetWebhook struct{} + +func (cnSetWebhook) setupWebhookWithManager(mgr ctrl.Manager) error { + return ctrl.NewWebhookManagedBy(mgr). + For(&v1alpha1.CNSet{}). + WithDefaulter(&cnSetDefaulter{}). + WithValidator(&cnSetValidator{}). + Complete() +} + +// +kubebuilder:webhook:path=/mutate-core-matrixorigin-io-v1alpha1-cnset,mutating=true,failurePolicy=fail,sideEffects=None,groups=core.matrixorigin.io,resources=cnsets,verbs=create;update,versions=v1alpha1,name=mcnset.kb.io,admissionReviewVersions={v1,v1beta1} + +// cnSetDefaulter implements webhook.CustomDefaulter so a webhook will be registered for v1alpha1.CNSet +type cnSetDefaulter struct{} + +var _ webhook.CustomDefaulter = &cnSetDefaulter{} + +func (c *cnSetDefaulter) Default(ctx context.Context, obj runtime.Object) error { + cnSet, ok := obj.(*v1alpha1.CNSet) + if !ok { + return unexpectedKindError("CNSet", obj) + } + + c.DefaultSpec(&cnSet.Spec) + if cnSet.Spec.Role == "" { + cnSet.Spec.Role = v1alpha1.CNRoleTP + } + return nil +} + +func (c *cnSetDefaulter) DefaultSpec(spec *v1alpha1.CNSetSpec) { + if spec.ServiceType == "" { + spec.ServiceType = corev1.ServiceTypeClusterIP + } + if spec.Resources.Requests.Memory().Value() != 0 && spec.SharedStorageCache.MemoryCacheSize == nil { + // default memory cache size to 50% request memory + size := spec.Resources.Requests.Memory().DeepCopy() + size.Set(size.Value() / 2) + spec.SharedStorageCache.MemoryCacheSize = &size + } + if spec.CacheVolume != nil && spec.SharedStorageCache.DiskCacheSize == nil { + // default disk cache size based on the cache volume total size + spec.SharedStorageCache.DiskCacheSize = defaultDiskCacheSize(&spec.CacheVolume.Size) + } + if spec.ScalingConfig.StoreDrainEnabled != nil && *spec.ScalingConfig.StoreDrainEnabled { + if spec.ScalingConfig.StoreDrainTimeout == nil { + spec.ScalingConfig.StoreDrainTimeout = &metav1.Duration{Duration: defaultStoreDrainTimeout} + } + } + if spec.UpdateStrategy.MaxSurge == nil { + maxSurge := intstr.FromInt(defaultMaxSurge) + spec.UpdateStrategy.MaxSurge = &maxSurge + } + if spec.UpdateStrategy.MaxUnavailable == nil { + maxUnavailable := intstr.FromInt(defaultMaxUnavailable) + spec.UpdateStrategy.MaxUnavailable = &maxUnavailable + } + setDefaultServiceArgs(spec) + setPodSetDefaults(&spec.PodSet) +} + +// +kubebuilder:webhook:path=/validate-core-matrixorigin-io-v1alpha1-cnset,mutating=false,failurePolicy=fail,sideEffects=None,groups=core.matrixorigin.io,resources=cnsets,verbs=create;update,versions=v1alpha1,name=vcnset.kb.io,admissionReviewVersions={v1,v1beta1} + +// cnSetValidator implements webhook.CustomValidator so a webhook will be registered for v1alpha1.CNSet +type cnSetValidator struct{} + +var _ webhook.CustomValidator = &cnSetValidator{} + +func (c *cnSetValidator) ValidateCreate(_ context.Context, obj runtime.Object) (warnings admission.Warnings, err error) { + cnSet, ok := obj.(*v1alpha1.CNSet) + if !ok { + return nil, unexpectedKindError("CNSet", obj) + } + var errs field.ErrorList + errs = append(errs, validateLogSetRef(&cnSet.Deps.LogSetRef, field.NewPath("deps"))...) + errs = append(errs, c.ValidateSpecCreate(&cnSet.Spec)...) + errs = append(errs, validateMainContainer(&cnSet.Spec.MainContainer, field.NewPath("spec"))...) + return nil, invalidOrNil(errs, cnSet) +} + +func (c *cnSetValidator) ValidateUpdate(ctx context.Context, _, newObj runtime.Object) (warnings admission.Warnings, err error) { + warnings, err = c.ValidateCreate(ctx, newObj) + if err != nil { + return warnings, err + } + return warnings, nil +} + +func (c *cnSetValidator) ValidateDelete(_ context.Context, obj runtime.Object) (warnings admission.Warnings, err error) { + return nil, nil +} + +func (c *cnSetValidator) ValidateSpecCreate(spec *v1alpha1.CNSetSpec) field.ErrorList { + var errs field.ErrorList + if spec.CacheVolume != nil { + errs = append(errs, validateVolume(spec.CacheVolume, field.NewPath("spec").Child("cacheVolume"))...) + } + if spec.ServiceType == corev1.ServiceTypeExternalName { + errs = append(errs, field.Invalid(field.NewPath("spec").Child("serviceType"), spec.ServiceType, "must be one of [ClusterIP, NodePort, LoadBalancer]")) + } + if spec.NodePort != nil && spec.ServiceType == corev1.ServiceTypeClusterIP { + errs = append(errs, field.Invalid(field.NewPath("spec").Child("nodePort"), spec.NodePort, "cannot set node port when serviceType is ClusterIP")) + } + for i, l := range spec.Labels { + if l.Key == "" { + errs = append(errs, field.Invalid(field.NewPath("spec").Child("cnLabels").Index(i).Child("key"), spec.Labels[i], "label key cannot be empty")) + } + if len(l.Values) == 0 { + errs = append(errs, field.Invalid(field.NewPath("spec").Child("cnLabels").Index(i).Child("values"), spec.Labels[i], "label values cannot be empty")) + } + for j, v := range l.Values { + if v == "" { + errs = append(errs, field.Invalid(field.NewPath("spec").Child("cnLabels").Index(i).Child("values").Index(j), spec.Labels[i].Values, "label value cannot be empty string")) + } + } + } + errs = append(errs, validateGoMemLimitPercent(spec.MemoryLimitPercent, field.NewPath("spec").Child("memoryLimitPercent"))...) + return errs +} diff --git a/api/core/v1alpha1/cnset_webhook_test.go b/pkg/webhook/cnset_webhook_test.go similarity index 71% rename from api/core/v1alpha1/cnset_webhook_test.go rename to pkg/webhook/cnset_webhook_test.go index 3ca5d40d..53490d94 100644 --- a/api/core/v1alpha1/cnset_webhook_test.go +++ b/pkg/webhook/cnset_webhook_test.go @@ -12,14 +12,17 @@ // See the License for the specific language governing permissions and // limitations under the License. -package v1alpha1 +package webhook import ( "context" + . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/matrixorigin/matrixone-operator/api/core/v1alpha1" ) var _ = Describe("CNSet Webhook", func() { @@ -28,22 +31,22 @@ var _ = Describe("CNSet Webhook", func() { // DO NOT mutate the following spec. // This spec is valid in mo-operator v0.6.0 and should always be accepted by // the webhook for backward compatibility. - v06 := &CNSet{ + v06 := &v1alpha1.CNSet{ ObjectMeta: metav1.ObjectMeta{ Name: "cn-" + randomString(5), Namespace: "default", }, - Spec: CNSetSpec{ - PodSet: PodSet{ + Spec: v1alpha1.CNSetSpec{ + PodSet: v1alpha1.PodSet{ Replicas: 2, - MainContainer: MainContainer{ + MainContainer: v1alpha1.MainContainer{ Image: "test", }, }, }, - Deps: CNSetDeps{ - LogSetRef: LogSetRef{ - LogSet: &LogSet{ + Deps: v1alpha1.CNSetDeps{ + LogSetRef: v1alpha1.LogSetRef{ + LogSet: &v1alpha1.LogSet{ ObjectMeta: metav1.ObjectMeta{ Name: "test", Namespace: "default", @@ -56,27 +59,27 @@ var _ = Describe("CNSet Webhook", func() { }) It("should set default cache size", func() { - cn := &CNSet{ + cn := &v1alpha1.CNSet{ ObjectMeta: metav1.ObjectMeta{ Name: "cn-" + randomString(5), Namespace: "default", }, - Spec: CNSetSpec{ - PodSet: PodSet{ + Spec: v1alpha1.CNSetSpec{ + PodSet: v1alpha1.PodSet{ Replicas: 2, - MainContainer: MainContainer{ + MainContainer: v1alpha1.MainContainer{ Image: "test", }, }, - ConfigThatChangeCNSpec: ConfigThatChangeCNSpec{ - CacheVolume: &Volume{ + ConfigThatChangeCNSpec: v1alpha1.ConfigThatChangeCNSpec{ + CacheVolume: &v1alpha1.Volume{ Size: resource.MustParse("20Gi"), }, }, }, - Deps: CNSetDeps{ - LogSetRef: LogSetRef{ - ExternalLogSet: &ExternalLogSet{}, + Deps: v1alpha1.CNSetDeps{ + LogSetRef: v1alpha1.LogSetRef{ + ExternalLogSet: &v1alpha1.ExternalLogSet{}, }, }, } @@ -86,45 +89,45 @@ var _ = Describe("CNSet Webhook", func() { }) It("should reject empty CN label key or values", func() { - cnTpl := &CNSet{ + cnTpl := &v1alpha1.CNSet{ ObjectMeta: metav1.ObjectMeta{ Name: "cn-" + randomString(5), Namespace: "default", }, - Spec: CNSetSpec{ - PodSet: PodSet{ + Spec: v1alpha1.CNSetSpec{ + PodSet: v1alpha1.PodSet{ Replicas: 2, - MainContainer: MainContainer{ + MainContainer: v1alpha1.MainContainer{ Image: "test", }, }, }, - Deps: CNSetDeps{ - LogSetRef: LogSetRef{ - ExternalLogSet: &ExternalLogSet{}, + Deps: v1alpha1.CNSetDeps{ + LogSetRef: v1alpha1.LogSetRef{ + ExternalLogSet: &v1alpha1.ExternalLogSet{}, }, }, } emptyLabelKey := cnTpl.DeepCopy() - emptyLabelKey.Spec.Labels = []CNLabel{{ + emptyLabelKey.Spec.Labels = []v1alpha1.CNLabel{{ Key: "", Values: []string{"test"}, }} Expect(k8sClient.Create(context.TODO(), emptyLabelKey)).NotTo(Succeed()) emptyLabelValueList := cnTpl.DeepCopy() - emptyLabelValueList.Spec.Labels = []CNLabel{{ + emptyLabelValueList.Spec.Labels = []v1alpha1.CNLabel{{ Key: "test", Values: []string{}, }} Expect(k8sClient.Create(context.TODO(), emptyLabelValueList)).NotTo(Succeed()) emptyLabelValueItem := cnTpl.DeepCopy() - emptyLabelValueItem.Spec.Labels = []CNLabel{{ + emptyLabelValueItem.Spec.Labels = []v1alpha1.CNLabel{{ Key: "test", Values: []string{""}, }} Expect(k8sClient.Create(context.TODO(), emptyLabelValueItem)).NotTo(Succeed()) validLabel := cnTpl.DeepCopy() - emptyLabelValueItem.Spec.Labels = []CNLabel{{ + emptyLabelValueItem.Spec.Labels = []v1alpha1.CNLabel{{ Key: "test", Values: []string{"test"}, }} diff --git a/api/core/v1alpha1/webhook.go b/pkg/webhook/common.go similarity index 62% rename from api/core/v1alpha1/webhook.go rename to pkg/webhook/common.go index 50985d1c..0c3201ef 100644 --- a/api/core/v1alpha1/webhook.go +++ b/pkg/webhook/common.go @@ -12,41 +12,16 @@ // See the License for the specific language governing permissions and // limitations under the License. -package v1alpha1 +package webhook import ( corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/util/validation/field" - ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" - logf "sigs.k8s.io/controller-runtime/pkg/log" -) - -var webhookLog = logf.Log.WithName("mo-webhook") -func RegisterWebhooks(mgr ctrl.Manager) error { - if err := (&MatrixOneCluster{}).setupWebhookWithManager(mgr); err != nil { - return err - } - if err := (&LogSet{}).setupWebhookWithManager(mgr); err != nil { - return err - } - if err := (&DNSet{}).setupWebhookWithManager(mgr); err != nil { - return err - } - if err := (&CNSet{}).setupWebhookWithManager(mgr); err != nil { - return err - } - if err := (&WebUI{}).setupWebhookWithManager(mgr); err != nil { - return err - } - if err := (&ProxySet{}).setupWebhookWithManager(mgr); err != nil { - return err - } - return nil -} + "github.com/matrixorigin/matrixone-operator/api/core/v1alpha1" +) func invalidOrNil(allErrs field.ErrorList, r client.Object) error { if len(allErrs) == 0 { @@ -55,7 +30,7 @@ func invalidOrNil(allErrs field.ErrorList, r client.Object) error { return apierrors.NewInvalid(r.GetObjectKind().GroupVersionKind().GroupKind(), r.GetName(), allErrs) } -func validateLogSetRef(ref *LogSetRef, parent *field.Path) field.ErrorList { +func validateLogSetRef(ref *v1alpha1.LogSetRef, parent *field.Path) field.ErrorList { var errs field.ErrorList if ref.LogSet == nil && ref.ExternalLogSet == nil { errs = append(errs, field.Invalid(parent, nil, "one of deps.logSet or deps.externalLogSet must be set")) @@ -63,7 +38,7 @@ func validateLogSetRef(ref *LogSetRef, parent *field.Path) field.ErrorList { return errs } -func validateMainContainer(c *MainContainer, parent *field.Path) field.ErrorList { +func validateMainContainer(c *v1alpha1.MainContainer, parent *field.Path) field.ErrorList { var errs field.ErrorList if c.Image == "" { errs = append(errs, field.Invalid(parent.Child("image"), c.Image, "image must be set")) @@ -77,7 +52,7 @@ func validateContainerResource(r *corev1.ResourceRequirements, parent *field.Pat return nil } -func validateVolume(v *Volume, parent *field.Path) field.ErrorList { +func validateVolume(v *v1alpha1.Volume, parent *field.Path) field.ErrorList { var errs field.ErrorList if v.Size.IsZero() { errs = append(errs, field.Invalid(parent.Child("size"), v.Size, "size must not be zero")) @@ -95,9 +70,3 @@ func validateGoMemLimitPercent(memPercent *int, path *field.Path) field.ErrorLis } return errs } - -func defaultDiskCacheSize(total *resource.Quantity) *resource.Quantity { - // shrink the total size since a small amount of space will be used for filesystem and metadata - shrunk := total.Value() * 9 / 10 - return resource.NewQuantity(shrunk, total.Format) -} diff --git a/pkg/webhook/dnset_webhook.go b/pkg/webhook/dnset_webhook.go new file mode 100644 index 00000000..8aef1090 --- /dev/null +++ b/pkg/webhook/dnset_webhook.go @@ -0,0 +1,120 @@ +// Copyright 2024 Matrix Origin +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package webhook + +import ( + "context" + + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/validation/field" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + "github.com/matrixorigin/matrixone-operator/api/core/v1alpha1" +) + +type dnSetWebhook struct{} + +func (dnSetWebhook) setupWebhookWithManager(mgr ctrl.Manager) error { + return ctrl.NewWebhookManagedBy(mgr). + For(&v1alpha1.DNSet{}). + WithDefaulter(&dnSetDefaulter{}). + WithValidator(&dnSetValidator{}). + Complete() +} + +// +kubebuilder:webhook:path=/mutate-core-matrixorigin-io-v1alpha1-dnset,mutating=true,failurePolicy=fail,sideEffects=None,groups=core.matrixorigin.io,resources=dnsets,verbs=create;update,versions=v1alpha1,name=mdnset.kb.io,admissionReviewVersions={v1,v1beta1} + +// dnSetDefaulter implements webhook.CustomDefaulter so a webhook will be registered for the v1alpha1.DNSet +type dnSetDefaulter struct{} + +var _ webhook.CustomDefaulter = &dnSetDefaulter{} + +func (d *dnSetDefaulter) Default(_ context.Context, obj runtime.Object) error { + dnSet, ok := obj.(*v1alpha1.DNSet) + if !ok { + return unexpectedKindError("DNSet", obj) + } + d.DefaultSpec(&dnSet.Spec) + return nil +} + +func (d *dnSetDefaulter) DefaultSpec(spec *v1alpha1.DNSetSpec) { + if spec.Resources.Requests.Memory() != nil && spec.SharedStorageCache.MemoryCacheSize == nil { + // default memory cache size to 50% request memory + size := spec.Resources.Requests.Memory().DeepCopy() + size.Set(size.Value() / 2) + spec.SharedStorageCache.MemoryCacheSize = &size + } + if spec.CacheVolume != nil && spec.SharedStorageCache.DiskCacheSize == nil { + // default disk cache size based on the cache volume total size + spec.SharedStorageCache.DiskCacheSize = defaultDiskCacheSize(&spec.CacheVolume.Size) + } + setDefaultServiceArgs(spec) + setPodSetDefaults(&spec.PodSet) +} + +// +kubebuilder:webhook:path=/validate-core-matrixorigin-io-v1alpha1-dnset,mutating=false,failurePolicy=fail,sideEffects=None,groups=core.matrixorigin.io,resources=dnsets,verbs=create;update,versions=v1alpha1,name=vdnset.kb.io,admissionReviewVersions={v1,v1beta1} + +// dnSetValidator implements webhook.CustomValidator so a webhook will be registered for v1alpha1.DNSet +type dnSetValidator struct{} + +var _ webhook.CustomValidator = &dnSetValidator{} + +func (d *dnSetValidator) ValidateCreate(ctx context.Context, obj runtime.Object) (warnings admission.Warnings, err error) { + dnSet, ok := obj.(*v1alpha1.DNSet) + if !ok { + return nil, unexpectedKindError("DNSet", obj) + } + var errs field.ErrorList + errs = append(errs, validateLogSetRef(&dnSet.Deps.LogSetRef, field.NewPath("deps"))...) + errs = append(errs, d.ValidateSpecCreate(&dnSet.Spec)...) + errs = append(errs, validateMainContainer(&dnSet.Spec.MainContainer, field.NewPath("spec"))...) + errs = append(errs, d.validateConfig(dnSet.Spec.Config, field.NewPath("spec").Child("config"))...) + return nil, invalidOrNil(errs, dnSet) +} + +func (d *dnSetValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (warnings admission.Warnings, err error) { + warnings, err = d.ValidateCreate(ctx, newObj) + if err != nil { + return warnings, err + } + return warnings, nil +} + +func (d *dnSetValidator) ValidateDelete(ctx context.Context, obj runtime.Object) (warnings admission.Warnings, err error) { + return nil, nil +} + +func (d *dnSetValidator) ValidateSpecCreate(spec *v1alpha1.DNSetSpec) field.ErrorList { + var errs field.ErrorList + if spec.CacheVolume != nil { + errs = append(errs, validateVolume(spec.CacheVolume, field.NewPath("spec").Child("cacheVolume"))...) + } + errs = append(errs, validateGoMemLimitPercent(spec.MemoryLimitPercent, field.NewPath("spec").Child("memoryLimitPercent"))...) + return errs +} + +func (d *dnSetValidator) validateConfig(c *v1alpha1.TomlConfig, path *field.Path) field.ErrorList { + var errs field.ErrorList + if c == nil { + return errs + } + if c.Get("tn") != nil && c.Get("dn") != nil { + errs = append(errs, field.Invalid(path, c, "[tn] and [dn] cannot be set at the same time")) + } + return errs +} diff --git a/api/core/v1alpha1/dnset_webhook_test.go b/pkg/webhook/dnset_webhook_test.go similarity index 72% rename from api/core/v1alpha1/dnset_webhook_test.go rename to pkg/webhook/dnset_webhook_test.go index f423ecf2..5ca0d268 100644 --- a/api/core/v1alpha1/dnset_webhook_test.go +++ b/pkg/webhook/dnset_webhook_test.go @@ -12,14 +12,17 @@ // See the License for the specific language governing permissions and // limitations under the License. -package v1alpha1 +package webhook import ( "context" + . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/matrixorigin/matrixone-operator/api/core/v1alpha1" ) var _ = Describe("DNSet Webhook", func() { @@ -28,22 +31,22 @@ var _ = Describe("DNSet Webhook", func() { // DO NOT mutate the following spec. // This spec is valid in mo-operator v0.6.0 and should always be accepted by // the webhook for backward compatibility. - v06 := &DNSet{ + v06 := &v1alpha1.DNSet{ ObjectMeta: metav1.ObjectMeta{ Name: "dn-" + randomString(5), Namespace: "default", }, - Spec: DNSetSpec{ - PodSet: PodSet{ + Spec: v1alpha1.DNSetSpec{ + PodSet: v1alpha1.PodSet{ Replicas: 2, - MainContainer: MainContainer{ + MainContainer: v1alpha1.MainContainer{ Image: "test", }, }, }, - Deps: DNSetDeps{ - LogSetRef: LogSetRef{ - LogSet: &LogSet{ + Deps: v1alpha1.DNSetDeps{ + LogSetRef: v1alpha1.LogSetRef{ + LogSet: &v1alpha1.LogSet{ ObjectMeta: metav1.ObjectMeta{ Name: "test", Namespace: "default", @@ -56,25 +59,25 @@ var _ = Describe("DNSet Webhook", func() { }) It("should set default cache size", func() { - dn := &DNSet{ + dn := &v1alpha1.DNSet{ ObjectMeta: metav1.ObjectMeta{ Name: "dn-" + randomString(5), Namespace: "default", }, - Spec: DNSetSpec{ - PodSet: PodSet{ + Spec: v1alpha1.DNSetSpec{ + PodSet: v1alpha1.PodSet{ Replicas: 2, - MainContainer: MainContainer{ + MainContainer: v1alpha1.MainContainer{ Image: "test", }, }, - CacheVolume: &Volume{ + CacheVolume: &v1alpha1.Volume{ Size: resource.MustParse("20Gi"), }, }, - Deps: DNSetDeps{ - LogSetRef: LogSetRef{ - ExternalLogSet: &ExternalLogSet{}, + Deps: v1alpha1.DNSetDeps{ + LogSetRef: v1alpha1.LogSetRef{ + ExternalLogSet: &v1alpha1.ExternalLogSet{}, }, }, } @@ -84,18 +87,18 @@ var _ = Describe("DNSet Webhook", func() { }) It("should reject duplicate [tn] and [dn] config", func() { - dn := &DNSet{ + dn := &v1alpha1.DNSet{ ObjectMeta: metav1.ObjectMeta{ Name: "dn-" + randomString(5), Namespace: "default", }, - Spec: DNSetSpec{ - PodSet: PodSet{ + Spec: v1alpha1.DNSetSpec{ + PodSet: v1alpha1.PodSet{ Replicas: 2, - MainContainer: MainContainer{ + MainContainer: v1alpha1.MainContainer{ Image: "test", }, - Config: NewTomlConfig(map[string]interface{}{ + Config: v1alpha1.NewTomlConfig(map[string]interface{}{ "tn": map[string]interface{}{ "port-base": 1000, }, @@ -105,9 +108,9 @@ var _ = Describe("DNSet Webhook", func() { }), }, }, - Deps: DNSetDeps{ - LogSetRef: LogSetRef{ - ExternalLogSet: &ExternalLogSet{}, + Deps: v1alpha1.DNSetDeps{ + LogSetRef: v1alpha1.LogSetRef{ + ExternalLogSet: &v1alpha1.ExternalLogSet{}, }, }, } diff --git a/api/core/v1alpha1/logset_webhook.go b/pkg/webhook/logset_webhook.go similarity index 51% rename from api/core/v1alpha1/logset_webhook.go rename to pkg/webhook/logset_webhook.go index f255fa53..6edc7d0e 100644 --- a/api/core/v1alpha1/logset_webhook.go +++ b/pkg/webhook/logset_webhook.go @@ -12,13 +12,13 @@ // See the License for the specific language governing permissions and // limitations under the License. -package v1alpha1 +package webhook import ( + "context" "fmt" "time" - "github.com/matrixorigin/matrixone-operator/api/features" "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -28,6 +28,9 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/webhook" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + "github.com/matrixorigin/matrixone-operator/api/core/v1alpha1" + "github.com/matrixorigin/matrixone-operator/api/features" ) const ( @@ -39,27 +42,35 @@ const ( defaultStoreFailureTimeout = 10 * time.Minute ) -var ( - kClient client.Client -) +type logSetWebhook struct{} -func (r *LogSet) setupWebhookWithManager(mgr ctrl.Manager) error { - kClient = mgr.GetClient() +func (logSetWebhook) setupWebhookWithManager(mgr ctrl.Manager) error { return ctrl.NewWebhookManagedBy(mgr). - For(r). + For(&v1alpha1.LogSet{}). + WithDefaulter(&logSetDefaulter{}). + WithValidator(&logSetValidator{ + kClient: mgr.GetClient(), + }). Complete() } // +kubebuilder:webhook:path=/mutate-core-matrixorigin-io-v1alpha1-logset,mutating=true,failurePolicy=fail,sideEffects=None,groups=core.matrixorigin.io,resources=logsets,verbs=create;update,versions=v1alpha1,name=mlogset.kb.io,admissionReviewVersions={v1,v1beta1} -var _ webhook.Defaulter = &LogSet{} +// logSetDefaulter implements webhook.Defaulter so a webhook will be registered for v1alpha1.LogSet +type logSetDefaulter struct{} -// Default implements webhook.Defaulter so a webhook will be registered for the type -func (r *LogSet) Default() { - r.Spec.Default() +var _ webhook.CustomDefaulter = &logSetDefaulter{} + +func (l *logSetDefaulter) Default(ctx context.Context, obj runtime.Object) error { + logSet, ok := obj.(*v1alpha1.LogSet) + if !ok { + return unexpectedKindError("LogSet", obj) + } + l.DefaultSpec(&logSet.Spec) + return nil } -func (r *LogSetSpec) Default() { +func (l *logSetDefaulter) DefaultSpec(spec *v1alpha1.LogSetSpec) { //if r.InitialConfig.HAKeeperReplicas == nil { // if r.Replicas >= minHAReplicas { // r.InitialConfig.HAKeeperReplicas = pointer.Int(minHAReplicas) @@ -67,41 +78,41 @@ func (r *LogSetSpec) Default() { // r.InitialConfig.HAKeeperReplicas = pointer.Int(singleReplica) // } //} - if r.InitialConfig.LogShardReplicas == nil { - if r.Replicas >= minHAReplicas { - r.InitialConfig.LogShardReplicas = pointer.Int(minHAReplicas) + if spec.InitialConfig.LogShardReplicas == nil { + if spec.Replicas >= minHAReplicas { + spec.InitialConfig.LogShardReplicas = pointer.Int(minHAReplicas) } else { - r.InitialConfig.LogShardReplicas = pointer.Int(singleReplica) + spec.InitialConfig.LogShardReplicas = pointer.Int(singleReplica) } } - if r.InitialConfig.LogShards == nil { - r.InitialConfig.LogShards = pointer.Int(defaultShardNum) + if spec.InitialConfig.LogShards == nil { + spec.InitialConfig.LogShards = pointer.Int(defaultShardNum) } - if r.InitialConfig.DNShards == nil { - r.InitialConfig.DNShards = pointer.Int(defaultShardNum) + if spec.InitialConfig.DNShards == nil { + spec.InitialConfig.DNShards = pointer.Int(defaultShardNum) } - if r.StoreFailureTimeout == nil { - r.StoreFailureTimeout = &metav1.Duration{Duration: defaultStoreFailureTimeout} + if spec.StoreFailureTimeout == nil { + spec.StoreFailureTimeout = &metav1.Duration{Duration: defaultStoreFailureTimeout} } - r.setDefaultRetentionPolicy() - setDefaultServiceArgs(r) - setPodSetDefaults(&r.PodSet) + l.setDefaultRetentionPolicy(spec) + setDefaultServiceArgs(spec) + setPodSetDefaults(&spec.PodSet) } // setDefaultRetentionPolicy always set PVCRetentionPolicy, and always set S3RetentionPolicy only if S3 is not nil // setDefaultRetentionPolicy does not change origin policy and only set default value when policy is nil -func (r *LogSetSpec) setDefaultRetentionPolicy() { - defaultDeletePolicy := PVCRetentionPolicyDelete +func (l *logSetDefaulter) setDefaultRetentionPolicy(spec *v1alpha1.LogSetSpec) { + defaultDeletePolicy := v1alpha1.PVCRetentionPolicyDelete - if r.SharedStorage.S3 == nil { - if r.PVCRetentionPolicy == nil { - r.PVCRetentionPolicy = &defaultDeletePolicy + if spec.SharedStorage.S3 == nil { + if spec.PVCRetentionPolicy == nil { + spec.PVCRetentionPolicy = &defaultDeletePolicy } return } - pvcPolicy := r.PVCRetentionPolicy - s3Policy := r.SharedStorage.S3.S3RetentionPolicy + pvcPolicy := spec.PVCRetentionPolicy + s3Policy := spec.SharedStorage.S3.S3RetentionPolicy switch { // if both set, does not set any values @@ -109,80 +120,89 @@ func (r *LogSetSpec) setDefaultRetentionPolicy() { return // if both not set, set to delete case pvcPolicy == nil && s3Policy == nil: - r.PVCRetentionPolicy = &defaultDeletePolicy - r.SharedStorage.S3.S3RetentionPolicy = &defaultDeletePolicy + spec.PVCRetentionPolicy = &defaultDeletePolicy + spec.SharedStorage.S3.S3RetentionPolicy = &defaultDeletePolicy // if only set pvcPolicy, set it to s3Policy case pvcPolicy != nil && s3Policy == nil: - r.SharedStorage.S3.S3RetentionPolicy = pvcPolicy + spec.SharedStorage.S3.S3RetentionPolicy = pvcPolicy // if only set s3Policy, set it to pvcPolicy case pvcPolicy == nil && s3Policy != nil: - r.PVCRetentionPolicy = s3Policy + spec.PVCRetentionPolicy = s3Policy } } // +kubebuilder:webhook:path=/validate-core-matrixorigin-io-v1alpha1-logset,mutating=false,failurePolicy=fail,sideEffects=None,groups=core.matrixorigin.io,resources=logsets,verbs=create;update,versions=v1alpha1,name=vlogset.kb.io,admissionReviewVersions={v1,v1beta1} -var _ webhook.Validator = &LogSet{} - -// ValidateCreate implements webhook.Validator so a webhook will be registered for the type -func (r *LogSet) ValidateCreate() (admission.Warnings, error) { - errs := r.Spec.ValidateCreate(r.ObjectMeta) - errs = append(errs, validateMainContainer(&r.Spec.MainContainer, field.NewPath("spec"))...) - return nil, invalidOrNil(errs, r) +// logSetValidator implements webhook.Validator so a webhook will be registered for the v1alpha1.LogSet +type logSetValidator struct { + kClient client.Client } -func (r *LogSet) ValidateUpdate(o runtime.Object) (admission.Warnings, error) { - old := o.(*LogSet) - errs := r.Spec.ValidateUpdate(&old.Spec, r.ObjectMeta) - return nil, invalidOrNil(errs, r) +var _ webhook.CustomValidator = &logSetValidator{} + +func (l *logSetValidator) ValidateCreate(_ context.Context, obj runtime.Object) (warnings admission.Warnings, err error) { + logSet, ok := obj.(*v1alpha1.LogSet) + if !ok { + return nil, unexpectedKindError("LogSet", obj) + } + errs := l.ValidateSpecCreate(logSet.ObjectMeta, &logSet.Spec) + errs = append(errs, validateMainContainer(&logSet.Spec.MainContainer, field.NewPath("spec"))...) + return nil, invalidOrNil(errs, logSet) } -func (r *LogSet) ValidateDelete() (admission.Warnings, error) { - return nil, nil +func (l *logSetValidator) ValidateUpdate(_ context.Context, oldObj, newObj runtime.Object) (warnings admission.Warnings, err error) { + old := oldObj.(*v1alpha1.LogSet) + logSet := newObj.(*v1alpha1.LogSet) + errs := l.ValidateSpecUpdate(&old.Spec, &logSet.Spec, logSet.ObjectMeta) + return nil, invalidOrNil(errs, logSet) } -func (r *LogSetSpec) validateMutateCommon() field.ErrorList { - var errs field.ErrorList - errs = append(errs, validateVolume(&r.Volume, field.NewPath("spec").Child("volume"))...) - errs = append(errs, r.validateInitialConfig()...) - errs = append(errs, r.validateSharedStorage()...) - errs = append(errs, validateGoMemLimitPercent(r.MemoryLimitPercent, field.NewPath("spec").Child("memoryLimitPercent"))...) - return errs +func (l *logSetValidator) ValidateDelete(ctx context.Context, obj runtime.Object) (warnings admission.Warnings, err error) { + return nil, nil } -func (r *LogSetSpec) ValidateCreate(meta metav1.ObjectMeta) field.ErrorList { +func (l *logSetValidator) ValidateSpecCreate(meta metav1.ObjectMeta, spec *v1alpha1.LogSetSpec) field.ErrorList { var errs field.ErrorList - errs = append(errs, r.validateMutateCommon()...) - errs = append(errs, r.validateIfBucketInUse(meta)...) - errs = append(errs, r.validateIfBucketDeleting()...) + errs = append(errs, l.validateMutateCommon(spec)...) + errs = append(errs, l.validateIfBucketInUse(meta, spec)...) + errs = append(errs, l.validateIfBucketDeleting(spec)...) return errs } -func (r *LogSetSpec) ValidateUpdate(old *LogSetSpec, meta metav1.ObjectMeta) field.ErrorList { - if err := r.validateMutateCommon(); err != nil { +func (l *logSetValidator) ValidateSpecUpdate(oldSpec, spec *v1alpha1.LogSetSpec, meta metav1.ObjectMeta) field.ErrorList { + if err := l.validateMutateCommon(spec); err != nil { return err } var errs field.ErrorList - if !equality.Semantic.DeepEqual(old.InitialConfig, r.InitialConfig) { + if !equality.Semantic.DeepEqual(oldSpec.InitialConfig, spec.InitialConfig) { errs = append(errs, field.Invalid(field.NewPath("spec").Child("initialConfig"), nil, "initialConfig is immutable")) } - errs = append(errs, r.validateIfBucketInUse(meta)...) + errs = append(errs, l.validateIfBucketInUse(meta, spec)...) + return errs +} + +func (l *logSetValidator) validateMutateCommon(spec *v1alpha1.LogSetSpec) field.ErrorList { + var errs field.ErrorList + errs = append(errs, validateVolume(&spec.Volume, field.NewPath("spec").Child("volume"))...) + errs = append(errs, l.validateInitialConfig(spec)...) + errs = append(errs, l.validateSharedStorage(spec)...) + errs = append(errs, validateGoMemLimitPercent(spec.MemoryLimitPercent, field.NewPath("spec").Child("memoryLimitPercent"))...) return errs } -func (r *LogSetSpec) validateSharedStorage() field.ErrorList { +func (l *logSetValidator) validateSharedStorage(spec *v1alpha1.LogSetSpec) field.ErrorList { var errs field.ErrorList parent := field.NewPath("spec").Child("sharedStorage") count := 0 - if r.SharedStorage.S3 != nil { + if spec.SharedStorage.S3 != nil { count += 1 - if r.SharedStorage.S3.Path == "" { + if spec.SharedStorage.S3.Path == "" { errs = append(errs, field.Invalid(parent, nil, "path must be set for S3 storage")) } } - if r.SharedStorage.FileSystem != nil { + if spec.SharedStorage.FileSystem != nil { count += 1 - if r.SharedStorage.FileSystem.Path == "" { + if spec.SharedStorage.FileSystem.Path == "" { errs = append(errs, field.Invalid(parent, nil, "path must be set for file-system storage")) } } @@ -195,7 +215,7 @@ func (r *LogSetSpec) validateSharedStorage() field.ErrorList { return errs } -func (r *LogSetSpec) validateInitialConfig() field.ErrorList { +func (l *logSetValidator) validateInitialConfig(spec *v1alpha1.LogSetSpec) field.ErrorList { var errs field.ErrorList parent := field.NewPath("spec").Child("initialConfig") @@ -205,32 +225,32 @@ func (r *LogSetSpec) validateInitialConfig() field.ErrorList { // errs = append(errs, field.Invalid(parent.Child("haKeeperReplicas"), hrs, "haKeeperReplicas must not larger then logservice replicas")) //} - if lrs := r.InitialConfig.LogShardReplicas; lrs == nil { + if lrs := spec.InitialConfig.LogShardReplicas; lrs == nil { errs = append(errs, field.Invalid(parent.Child("logShardReplicas"), lrs, "logShardReplicas must be set")) - } else if *lrs > int(r.Replicas) { + } else if *lrs > int(spec.Replicas) { errs = append(errs, field.Invalid(parent.Child("logShardReplicas"), lrs, "logShardReplicas must not larger then logservice replicas")) } - if lss := r.InitialConfig.LogShards; lss == nil { + if lss := spec.InitialConfig.LogShards; lss == nil { errs = append(errs, field.Invalid(parent.Child("logShards"), lss, "logShards must be set")) } - if dss := r.InitialConfig.DNShards; dss == nil { + if dss := spec.InitialConfig.DNShards; dss == nil { errs = append(errs, field.Invalid(parent.Child("dnShards"), dss, "dnShards must be set")) } return errs } -func (r *LogSetSpec) validateIfBucketDeleting() field.ErrorList { +func (l *logSetValidator) validateIfBucketDeleting(spec *v1alpha1.LogSetSpec) field.ErrorList { if !features.DefaultFeatureGate.Enabled(features.S3Reclaim) { return nil } - if r.SharedStorage.S3 == nil { + if spec.SharedStorage.S3 == nil { return nil } var errs field.ErrorList path := field.NewPath("spec").Child("sharedStorage").Child("s3") - bucket, err := ClaimedBucket(kClient, r.SharedStorage.S3) + bucket, err := v1alpha1.ClaimedBucket(l.kClient, spec.SharedStorage.S3) if err != nil { errs = append(errs, field.Invalid(path, nil, err.Error())) return errs @@ -246,16 +266,16 @@ func (r *LogSetSpec) validateIfBucketDeleting() field.ErrorList { return nil } -func (r *LogSetSpec) validateIfBucketInUse(meta metav1.ObjectMeta) field.ErrorList { +func (l *logSetValidator) validateIfBucketInUse(meta metav1.ObjectMeta, spec *v1alpha1.LogSetSpec) field.ErrorList { if !features.DefaultFeatureGate.Enabled(features.S3Reclaim) { return nil } - if r.SharedStorage.S3 == nil { + if spec.SharedStorage.S3 == nil { return nil } var errs field.ErrorList path := field.NewPath("spec").Child("sharedStorage").Child("s3") - bucket, err := ClaimedBucket(kClient, r.SharedStorage.S3) + bucket, err := v1alpha1.ClaimedBucket(l.kClient, spec.SharedStorage.S3) if err != nil { errs = append(errs, field.Invalid(path, nil, err.Error())) return errs @@ -263,8 +283,8 @@ func (r *LogSetSpec) validateIfBucketInUse(meta metav1.ObjectMeta) field.ErrorLi if bucket == nil { return nil } - if bucket.Status.State == StatusInUse && - bucket.Status.BindTo != BucketBindToMark(meta) { + if bucket.Status.State == v1alpha1.StatusInUse && + bucket.Status.BindTo != v1alpha1.BucketBindToMark(meta) { msg := fmt.Sprintf("claimed bucket %v already bind to %v", client.ObjectKeyFromObject(bucket), bucket.Status.BindTo) errs = append(errs, field.Invalid(path, nil, msg)) return errs diff --git a/api/core/v1alpha1/logset_webhook_test.go b/pkg/webhook/logset_webhook_test.go similarity index 74% rename from api/core/v1alpha1/logset_webhook_test.go rename to pkg/webhook/logset_webhook_test.go index 576c0615..916d49bd 100644 --- a/api/core/v1alpha1/logset_webhook_test.go +++ b/pkg/webhook/logset_webhook_test.go @@ -12,14 +12,17 @@ // See the License for the specific language governing permissions and // limitations under the License. -package v1alpha1 +package webhook import ( "context" + . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/matrixorigin/matrixone-operator/api/core/v1alpha1" ) var _ = Describe("LogSet Webhook", func() { @@ -28,23 +31,23 @@ var _ = Describe("LogSet Webhook", func() { // DO NOT mutate the following spec. // This spec is valid in mo-operator v0.6.0 and should always be accepted by // the webhook for backward compatibility. - v06 := &LogSet{ + v06 := &v1alpha1.LogSet{ ObjectMeta: metav1.ObjectMeta{ Name: "ls-" + randomString(5), Namespace: "default", }, - Spec: LogSetSpec{ - PodSet: PodSet{ + Spec: v1alpha1.LogSetSpec{ + PodSet: v1alpha1.PodSet{ Replicas: 3, - MainContainer: MainContainer{ + MainContainer: v1alpha1.MainContainer{ Image: "test", }, }, - Volume: Volume{ + Volume: v1alpha1.Volume{ Size: resource.MustParse("10Gi"), }, - SharedStorage: SharedStorageProvider{ - S3: &S3Provider{Path: "test/data"}, + SharedStorage: v1alpha1.SharedStorageProvider{ + S3: &v1alpha1.S3Provider{Path: "test/data"}, }, }, } @@ -52,29 +55,29 @@ var _ = Describe("LogSet Webhook", func() { }) It("should set defaults", func() { - tpl := &LogSet{ + tpl := &v1alpha1.LogSet{ ObjectMeta: metav1.ObjectMeta{ Name: "ls-" + randomString(5), Namespace: "default", }, - Spec: LogSetSpec{ - PodSet: PodSet{ + Spec: v1alpha1.LogSetSpec{ + PodSet: v1alpha1.PodSet{ Replicas: 3, - MainContainer: MainContainer{ + MainContainer: v1alpha1.MainContainer{ Image: "test", }, }, - Volume: Volume{ + Volume: v1alpha1.Volume{ Size: resource.MustParse("10Gi"), }, - SharedStorage: SharedStorageProvider{ - S3: &S3Provider{Path: "test/data"}, + SharedStorage: v1alpha1.SharedStorageProvider{ + S3: &v1alpha1.S3Provider{Path: "test/data"}, }, }, } testDefaultPVCRetainPolicy := tpl.DeepCopy() Expect(k8sClient.Create(context.TODO(), testDefaultPVCRetainPolicy)).To(Succeed()) Expect(testDefaultPVCRetainPolicy.Spec.PVCRetentionPolicy).NotTo(BeNil()) - Expect(*testDefaultPVCRetainPolicy.Spec.PVCRetentionPolicy).To(Equal(PVCRetentionPolicyDelete)) + Expect(*testDefaultPVCRetainPolicy.Spec.PVCRetentionPolicy).To(Equal(v1alpha1.PVCRetentionPolicyDelete)) }) }) diff --git a/pkg/webhook/matrixonecluster_webhook.go b/pkg/webhook/matrixonecluster_webhook.go new file mode 100644 index 00000000..b916743d --- /dev/null +++ b/pkg/webhook/matrixonecluster_webhook.go @@ -0,0 +1,174 @@ +// Copyright 2024 Matrix Origin +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package webhook + +import ( + "context" + "fmt" + + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/validation" + "k8s.io/apimachinery/pkg/util/validation/field" + 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" + + "github.com/matrixorigin/matrixone-operator/api/core/v1alpha1" +) + +const ( + // MatrixOneClusterNameMaxLength is the maximum length of a MatrixOneCluster name + MatrixOneClusterNameMaxLength = 46 +) + +// log is for logging in this package. +var moLog = logf.Log.WithName("mo-cluster") + +type matrixOneClusterWebhook struct{} + +func (matrixOneClusterWebhook) setupWebhookWithManager(mgr ctrl.Manager) error { + return ctrl.NewWebhookManagedBy(mgr). + For(&v1alpha1.MatrixOneCluster{}). + WithDefaulter(&matrixOneClusterDefaulter{ + cn: &cnSetDefaulter{}, + dn: &dnSetDefaulter{}, + logService: &logSetDefaulter{}, + }). + WithValidator(&matrixOneClusterValidator{}). + Complete() +} + +// +kubebuilder:webhook:path=/mutate-core-matrixorigin-io-v1alpha1-matrixonecluster,mutating=true,failurePolicy=fail,sideEffects=None,groups=core.matrixorigin.io,resources=matrixoneclusters,verbs=create;update,versions=v1alpha1,name=mmatrixonecluster.kb.io,admissionReviewVersions=v1;v1beta1 + +// matrixOneClusterDefaulter implements webhook.Defaulter so a webhook will be registered for v1alpha1.MatrixOneCluster +type matrixOneClusterDefaulter struct { + cn *cnSetDefaulter + dn *dnSetDefaulter + logService *logSetDefaulter +} + +var _ webhook.CustomDefaulter = &matrixOneClusterDefaulter{} + +func (m *matrixOneClusterDefaulter) Default(ctx context.Context, obj runtime.Object) error { + moc, ok := obj.(*v1alpha1.MatrixOneCluster) + if !ok { + return unexpectedKindError("MatrixOneCluster", obj) + } + m.logService.DefaultSpec(&moc.Spec.LogService) + if moc.Spec.DN != nil { + m.dn.DefaultSpec(moc.Spec.DN) + } + if moc.Spec.TN != nil { + m.dn.DefaultSpec(moc.Spec.TN) + } + if moc.Spec.TP != nil { + m.cn.DefaultSpec(moc.Spec.TP) + } + if moc.Spec.AP != nil { + m.cn.DefaultSpec(moc.Spec.AP) + } + for i := range moc.Spec.CNGroups { + m.cn.DefaultSpec(&moc.Spec.CNGroups[i].CNSetSpec) + } + return nil +} + +// +kubebuilder:webhook:path=/validate-core-matrixorigin-io-v1alpha1-matrixonecluster,mutating=false,failurePolicy=fail,sideEffects=None,groups=core.matrixorigin.io,resources=matrixoneclusters,verbs=create;update,versions=v1alpha1,name=vmatrixonecluster.kb.io,admissionReviewVersions=v1;v1beta1 + +// matrixOneClusterValidator implements webhook.Validator so a webhook will be registered for v1alpha1.MatrixOneCluster +type matrixOneClusterValidator struct { + cn *cnSetValidator + dn *dnSetValidator + logService *logSetValidator +} + +var _ webhook.CustomValidator = &matrixOneClusterValidator{} + +func (m *matrixOneClusterValidator) ValidateCreate(ctx context.Context, obj runtime.Object) (warnings admission.Warnings, err error) { + moc, ok := obj.(*v1alpha1.MatrixOneCluster) + if !ok { + return nil, unexpectedKindError("MatrixOneCluster", obj) + } + var errs field.ErrorList + if len(moc.Name) > MatrixOneClusterNameMaxLength { + errs = append(errs, field.Invalid(field.NewPath("metadata").Child("name"), moc.Name, fmt.Sprintf("must be no more than %d characters", MatrixOneClusterNameMaxLength))) + } + dns1035ErrorList := validation.IsDNS1035Label(moc.Name) + for _, errMsg := range dns1035ErrorList { + errs = append(errs, field.Invalid(field.NewPath("metadata").Child("name"), moc.Name, errMsg)) + } + if moc.Spec.DN == nil && moc.Spec.TN == nil { + errs = append(errs, field.Invalid(field.NewPath("spec").Child("tn"), "", ".spec.tn must be set")) + } + if moc.Spec.DN != nil && moc.Spec.TN != nil { + errs = append(errs, field.Invalid(field.NewPath("spec").Child("dn"), "", "legacy component .spec.dn cannot be set when .spec.tn is set")) + } + errs = append(errs, m.validateMutateCommon(moc)...) + //errs = append(errs, r.Spec.LogService.ValidateCreate(LogSetKey(r))...) + errs = append(errs, m.logService.ValidateSpecCreate(v1alpha1.LogSetKey(moc), &moc.Spec.LogService)...) + return nil, invalidOrNil(errs, moc) +} + +func (m *matrixOneClusterValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (warnings admission.Warnings, err error) { + var errs field.ErrorList + moc := newObj.(*v1alpha1.MatrixOneCluster) + errs = append(errs, m.validateMutateCommon(moc)...) + + old := oldObj.(*v1alpha1.MatrixOneCluster) + errs = append(errs, m.logService.ValidateSpecUpdate(&old.Spec.LogService, &moc.Spec.LogService, v1alpha1.LogSetKey(moc))...) + return nil, invalidOrNil(errs, moc) +} + +func (m *matrixOneClusterValidator) ValidateDelete(ctx context.Context, obj runtime.Object) (warnings admission.Warnings, err error) { + return nil, nil +} + +func (m *matrixOneClusterValidator) validateMutateCommon(moc *v1alpha1.MatrixOneCluster) field.ErrorList { + var errs field.ErrorList + errs = append(errs, m.dn.ValidateSpecCreate(moc.GetTN())...) + groups := map[string]bool{} + if moc.Spec.TP != nil { + errs = append(errs, m.cn.ValidateSpecCreate(moc.Spec.TP)...) + groups["tp"] = true + } + if moc.Spec.AP != nil { + errs = append(errs, m.cn.ValidateSpecCreate(moc.Spec.AP)...) + groups["ap"] = true + } + + for i, cn := range moc.Spec.CNGroups { + errs = append(errs, m.validateCNGroup(cn, field.NewPath("spec").Child("cnGroups").Index(i))...) + if groups[cn.Name] { + errs = append(errs, field.Invalid(field.NewPath("spec").Child("cnGroups").Index(i).Child("name"), cn.Name, "name must be unique")) + } + groups[cn.Name] = true + } + if moc.Spec.Version == "" { + errs = append(errs, field.Invalid(field.NewPath("spec").Child("version"), "", "version must be set")) + } + return errs +} + +func (m *matrixOneClusterValidator) validateCNGroup(g v1alpha1.CNGroup, parent *field.Path) field.ErrorList { + var errs field.ErrorList + if es := validation.IsDNS1123Subdomain(g.Name); es != nil { + for _, err := range es { + errs = append(errs, field.Invalid(parent.Child("name"), g.Name, err)) + } + } + errs = append(errs, m.cn.ValidateSpecCreate(&g.CNSetSpec)...) + return errs +} diff --git a/api/core/v1alpha1/matrixonecluster_webhook_test.go b/pkg/webhook/matrixonecluster_webhook_test.go similarity index 69% rename from api/core/v1alpha1/matrixonecluster_webhook_test.go rename to pkg/webhook/matrixonecluster_webhook_test.go index 21f0afba..4b57ebd2 100644 --- a/api/core/v1alpha1/matrixonecluster_webhook_test.go +++ b/pkg/webhook/matrixonecluster_webhook_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package v1alpha1 +package webhook import ( "context" @@ -23,6 +23,8 @@ import ( "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/pointer" + + "github.com/matrixorigin/matrixone-operator/api/core/v1alpha1" ) var _ = Describe("MatrixOneCluster Webhook", func() { @@ -32,30 +34,30 @@ var _ = Describe("MatrixOneCluster Webhook", func() { // DO NOT mutate the following spec. // This spec is valid in mo-operator v0.6.0 and should always be accepted by // the webhook for backward compatibility. - v06 := &MatrixOneCluster{ + v06 := &v1alpha1.MatrixOneCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "mo-" + randomString(5), Namespace: "default", }, - Spec: MatrixOneClusterSpec{ - LogService: LogSetSpec{ - PodSet: PodSet{ + Spec: v1alpha1.MatrixOneClusterSpec{ + LogService: v1alpha1.LogSetSpec{ + PodSet: v1alpha1.PodSet{ Replicas: 3, }, - Volume: Volume{ + Volume: v1alpha1.Volume{ Size: resource.MustParse("10Gi"), }, - SharedStorage: SharedStorageProvider{ - S3: &S3Provider{Path: "test/data"}, + SharedStorage: v1alpha1.SharedStorageProvider{ + S3: &v1alpha1.S3Provider{Path: "test/data"}, }, }, - TN: &DNSetSpec{ - PodSet: PodSet{ + TN: &v1alpha1.DNSetSpec{ + PodSet: v1alpha1.PodSet{ Replicas: 2, }, }, - TP: &CNSetSpec{ - PodSet: PodSet{ + TP: &v1alpha1.CNSetSpec{ + PodSet: v1alpha1.PodSet{ Replicas: 2, }, }, @@ -63,7 +65,7 @@ var _ = Describe("MatrixOneCluster Webhook", func() { }, } Expect(k8sClient.Create(context.TODO(), v06.DeepCopy())).To(Succeed()) - Expect(k8sClient.Create(context.TODO(), func() *MatrixOneCluster { + Expect(k8sClient.Create(context.TODO(), func() *v1alpha1.MatrixOneCluster { singleReplica := v06.DeepCopy() singleReplica.Spec.LogService.Replicas = 1 singleReplica.Spec.TN.Replicas = 1 @@ -74,30 +76,30 @@ var _ = Describe("MatrixOneCluster Webhook", func() { }) It("should reject invalid MatrixOneCluster", func() { - tpl := &MatrixOneCluster{ + tpl := &v1alpha1.MatrixOneCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "mo-" + randomString(5), Namespace: "default", }, - Spec: MatrixOneClusterSpec{ - LogService: LogSetSpec{ - PodSet: PodSet{ + Spec: v1alpha1.MatrixOneClusterSpec{ + LogService: v1alpha1.LogSetSpec{ + PodSet: v1alpha1.PodSet{ Replicas: 3, }, - Volume: Volume{ + Volume: v1alpha1.Volume{ Size: resource.MustParse("10Gi"), }, - SharedStorage: SharedStorageProvider{ - S3: &S3Provider{Path: "test/data"}, + SharedStorage: v1alpha1.SharedStorageProvider{ + S3: &v1alpha1.S3Provider{Path: "test/data"}, }, }, - TN: &DNSetSpec{ - PodSet: PodSet{ + TN: &v1alpha1.DNSetSpec{ + PodSet: v1alpha1.PodSet{ Replicas: 2, }, }, - TP: &CNSetSpec{ - PodSet: PodSet{ + TP: &v1alpha1.CNSetSpec{ + PodSet: v1alpha1.PodSet{ Replicas: 2, }, }, @@ -123,32 +125,33 @@ var _ = Describe("MatrixOneCluster Webhook", func() { }) It("should validate and mutate MatrixOneCluster", func() { - cluster := &MatrixOneCluster{ + cluster := &v1alpha1.MatrixOneCluster{ ObjectMeta: metav1.ObjectMeta{ - Name: "mo-" + randomString(5), + Name: "mo-test-mutate-cc", + //Name: "mo-" + randomString(5), Namespace: "default", }, - Spec: MatrixOneClusterSpec{ - LogService: LogSetSpec{ - PodSet: PodSet{ + Spec: v1alpha1.MatrixOneClusterSpec{ + LogService: v1alpha1.LogSetSpec{ + PodSet: v1alpha1.PodSet{ Replicas: 3, }, - Volume: Volume{ + Volume: v1alpha1.Volume{ Size: resource.MustParse("10Gi"), }, - SharedStorage: SharedStorageProvider{ - S3: &S3Provider{ + SharedStorage: v1alpha1.SharedStorageProvider{ + S3: &v1alpha1.S3Provider{ Path: "test/data", }, }, }, - TN: &DNSetSpec{ - PodSet: PodSet{ + TN: &v1alpha1.DNSetSpec{ + PodSet: v1alpha1.PodSet{ Replicas: 2, }, }, - TP: &CNSetSpec{ - PodSet: PodSet{ + TP: &v1alpha1.CNSetSpec{ + PodSet: v1alpha1.PodSet{ Replicas: 2, }, }, @@ -164,8 +167,8 @@ var _ = Describe("MatrixOneCluster Webhook", func() { By("accept valid update") cluster.Spec.LogService.Replicas = 5 - cluster.Spec.AP = &CNSetSpec{ - PodSet: PodSet{ + cluster.Spec.AP = &v1alpha1.CNSetSpec{ + PodSet: v1alpha1.PodSet{ Replicas: 2, }, } @@ -174,7 +177,7 @@ var _ = Describe("MatrixOneCluster Webhook", func() { By("reject invalid update") invalidReplica := cluster.DeepCopy() invalidReplica.Spec.LogService.Replicas = 2 - Expect(k8sClient.Update(context.TODO(), invalidReplica)).ToNot(Succeed(), "logservice replicas cannot be lower than HAKeeperReplicas") + Expect(k8sClient.Update(context.TODO(), invalidReplica)).NotTo(Succeed(), "logservice replicas cannot be lower than HAKeeperReplicas") mutateInitialConfig := cluster.DeepCopy() mutateInitialConfig.Spec.LogService.InitialConfig.LogShardReplicas = pointer.Int(*mutateInitialConfig.Spec.LogService.InitialConfig.LogShardReplicas - 1) @@ -182,44 +185,44 @@ var _ = Describe("MatrixOneCluster Webhook", func() { }) It("should validate and set defaults for CNGroups", func() { - cluster := &MatrixOneCluster{ + cluster := &v1alpha1.MatrixOneCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "mo-" + randomString(5), Namespace: "default", }, - Spec: MatrixOneClusterSpec{ - LogService: LogSetSpec{ - PodSet: PodSet{ + Spec: v1alpha1.MatrixOneClusterSpec{ + LogService: v1alpha1.LogSetSpec{ + PodSet: v1alpha1.PodSet{ Replicas: 3, }, - Volume: Volume{ + Volume: v1alpha1.Volume{ Size: resource.MustParse("10Gi"), }, - SharedStorage: SharedStorageProvider{ - S3: &S3Provider{ + SharedStorage: v1alpha1.SharedStorageProvider{ + S3: &v1alpha1.S3Provider{ Path: "test/data", }, }, }, - TN: &DNSetSpec{ - PodSet: PodSet{ + TN: &v1alpha1.DNSetSpec{ + PodSet: v1alpha1.PodSet{ Replicas: 2, }, }, Version: "test", - CNGroups: []CNGroup{{ + CNGroups: []v1alpha1.CNGroup{{ Name: "test", - CNSetSpec: CNSetSpec{ - PodSet: PodSet{ + CNSetSpec: v1alpha1.CNSetSpec{ + PodSet: v1alpha1.PodSet{ Replicas: 3, }, }, }, { Name: "cache", - CNSetSpec: CNSetSpec{ - PodSet: PodSet{ + CNSetSpec: v1alpha1.CNSetSpec{ + PodSet: v1alpha1.PodSet{ Replicas: 3, - MainContainer: MainContainer{ + MainContainer: v1alpha1.MainContainer{ Resources: corev1.ResourceRequirements{ Requests: map[corev1.ResourceName]resource.Quantity{ corev1.ResourceMemory: resource.MustParse("10Gi"), @@ -227,8 +230,8 @@ var _ = Describe("MatrixOneCluster Webhook", func() { }, }, }, - ConfigThatChangeCNSpec: ConfigThatChangeCNSpec{ - CacheVolume: &Volume{ + ConfigThatChangeCNSpec: v1alpha1.ConfigThatChangeCNSpec{ + CacheVolume: &v1alpha1.Volume{ Size: resource.MustParse("10Gi"), }, }, @@ -252,43 +255,43 @@ var _ = Describe("MatrixOneCluster Webhook", func() { }) It("should reject duplicate CNGroups", func() { - cluster := &MatrixOneCluster{ + cluster := &v1alpha1.MatrixOneCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "mo-" + randomString(5), Namespace: "default", }, - Spec: MatrixOneClusterSpec{ - LogService: LogSetSpec{ - PodSet: PodSet{ + Spec: v1alpha1.MatrixOneClusterSpec{ + LogService: v1alpha1.LogSetSpec{ + PodSet: v1alpha1.PodSet{ Replicas: 3, }, - Volume: Volume{ + Volume: v1alpha1.Volume{ Size: resource.MustParse("10Gi"), }, - SharedStorage: SharedStorageProvider{ - S3: &S3Provider{ + SharedStorage: v1alpha1.SharedStorageProvider{ + S3: &v1alpha1.S3Provider{ Path: "test/data", }, }, }, - TN: &DNSetSpec{ - PodSet: PodSet{ + TN: &v1alpha1.DNSetSpec{ + PodSet: v1alpha1.PodSet{ Replicas: 1, }, }, Version: "test", - TP: &CNSetSpec{ - PodSet: PodSet{ + TP: &v1alpha1.CNSetSpec{ + PodSet: v1alpha1.PodSet{ Replicas: 3, }, }, }, } dupTP := cluster.DeepCopy() - dupTP.Spec.CNGroups = []CNGroup{{ + dupTP.Spec.CNGroups = []v1alpha1.CNGroup{{ Name: "tp", - CNSetSpec: CNSetSpec{ - PodSet: PodSet{ + CNSetSpec: v1alpha1.CNSetSpec{ + PodSet: v1alpha1.PodSet{ Replicas: 3, }, }, @@ -296,17 +299,17 @@ var _ = Describe("MatrixOneCluster Webhook", func() { Expect(k8sClient.Create(context.TODO(), dupTP)).NotTo(Succeed()) dupCNGroup := cluster.DeepCopy() - dupCNGroup.Spec.CNGroups = []CNGroup{{ + dupCNGroup.Spec.CNGroups = []v1alpha1.CNGroup{{ Name: "a", - CNSetSpec: CNSetSpec{ - PodSet: PodSet{ + CNSetSpec: v1alpha1.CNSetSpec{ + PodSet: v1alpha1.PodSet{ Replicas: 3, }, }, }, { Name: "a", - CNSetSpec: CNSetSpec{ - PodSet: PodSet{ + CNSetSpec: v1alpha1.CNSetSpec{ + PodSet: v1alpha1.PodSet{ Replicas: 3, }, }, @@ -315,33 +318,33 @@ var _ = Describe("MatrixOneCluster Webhook", func() { }) It("should reject MatrixOneCluster with invalid name", func() { - cluster := &MatrixOneCluster{ + cluster := &v1alpha1.MatrixOneCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "mo-" + randomString(5), Namespace: "default", }, - Spec: MatrixOneClusterSpec{ - LogService: LogSetSpec{ - PodSet: PodSet{ + Spec: v1alpha1.MatrixOneClusterSpec{ + LogService: v1alpha1.LogSetSpec{ + PodSet: v1alpha1.PodSet{ Replicas: 3, }, - Volume: Volume{ + Volume: v1alpha1.Volume{ Size: resource.MustParse("10Gi"), }, - SharedStorage: SharedStorageProvider{ - S3: &S3Provider{ + SharedStorage: v1alpha1.SharedStorageProvider{ + S3: &v1alpha1.S3Provider{ Path: "test/data", }, }, }, - TN: &DNSetSpec{ - PodSet: PodSet{ + TN: &v1alpha1.DNSetSpec{ + PodSet: v1alpha1.PodSet{ Replicas: 1, }, }, Version: "test", - TP: &CNSetSpec{ - PodSet: PodSet{ + TP: &v1alpha1.CNSetSpec{ + PodSet: v1alpha1.PodSet{ Replicas: 3, }, }, diff --git a/api/core/v1alpha1/proxy_webhook.go b/pkg/webhook/proxy_webhook.go similarity index 50% rename from api/core/v1alpha1/proxy_webhook.go rename to pkg/webhook/proxy_webhook.go index de148b9f..404f061c 100644 --- a/api/core/v1alpha1/proxy_webhook.go +++ b/pkg/webhook/proxy_webhook.go @@ -12,47 +12,64 @@ // See the License for the specific language governing permissions and // limitations under the License. -package v1alpha1 +package webhook import ( + "context" + "k8s.io/apimachinery/pkg/runtime" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/webhook" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + "github.com/matrixorigin/matrixone-operator/api/core/v1alpha1" ) -func (r *ProxySet) setupWebhookWithManager(mgr ctrl.Manager) error { +type proxySetWebhook struct{} + +func (proxySetWebhook) setupWebhookWithManager(mgr ctrl.Manager) error { return ctrl.NewWebhookManagedBy(mgr). - For(r). + For(&v1alpha1.ProxySet{}). + WithDefaulter(&proxySetDefaulter{}). + WithValidator(&proxySetValidator{}). Complete() } // +kubebuilder:webhook:path=/mutate-core-matrixorigin-io-v1alpha1-proxyset,mutating=true,failurePolicy=fail,sideEffects=None,groups=core.matrixorigin.io,resources=proxysets,verbs=create;update,versions=v1alpha1,name=mproxyset.kb.io,admissionReviewVersions={v1,v1beta1} -var _ webhook.Defaulter = &ProxySet{} +// proxySetDefaulter implements webhook.CustomDefaulter so a webhook will be registered for v1alpha1.ProxySet +type proxySetDefaulter struct{} -// Default implements webhook.Defaulter so a webhook will be registered for the type -func (r *ProxySet) Default() { - r.Spec.Default() +var _ webhook.CustomDefaulter = &proxySetDefaulter{} + +func (p *proxySetDefaulter) Default(ctx context.Context, obj runtime.Object) error { + proxySet, ok := obj.(*v1alpha1.ProxySet) + if !ok { + return unexpectedKindError("ProxySet", obj) + } + p.DefaultSpec(&proxySet.Spec) + return nil } -func (r *ProxySetSpec) Default() { - setDefaultServiceArgs(r) +func (p *proxySetDefaulter) DefaultSpec(spec *v1alpha1.ProxySetSpec) { + setDefaultServiceArgs(spec) } // +kubebuilder:webhook:path=/validate-core-matrixorigin-io-v1alpha1-proxyset,mutating=false,failurePolicy=fail,sideEffects=None,groups=core.matrixorigin.io,resources=proxysets,verbs=create;update,versions=v1alpha1,name=vproxyset.kb.io,admissionReviewVersions={v1,v1beta1} -var _ webhook.Validator = &ProxySet{} +// proxySetValidator implements webhook.Validator so a webhook will be registered for v1alpha1.ProxySet +type proxySetValidator struct{} + +var _ webhook.CustomValidator = &proxySetValidator{} -// ValidateCreate implements webhook.Validator so a webhook will be registered for the type -func (r *ProxySet) ValidateCreate() (admission.Warnings, error) { +func (p proxySetValidator) ValidateCreate(ctx context.Context, obj runtime.Object) (warnings admission.Warnings, err error) { return nil, nil } -func (r *ProxySet) ValidateUpdate(old runtime.Object) (admission.Warnings, error) { +func (p proxySetValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (warnings admission.Warnings, err error) { return nil, nil } -func (r *ProxySet) ValidateDelete() (admission.Warnings, error) { +func (p proxySetValidator) ValidateDelete(ctx context.Context, obj runtime.Object) (warnings admission.Warnings, err error) { return nil, nil } diff --git a/api/core/v1alpha1/proxy_webhook_test.go b/pkg/webhook/proxy_webhook_test.go similarity index 76% rename from api/core/v1alpha1/proxy_webhook_test.go rename to pkg/webhook/proxy_webhook_test.go index 78571573..fd398092 100644 --- a/api/core/v1alpha1/proxy_webhook_test.go +++ b/pkg/webhook/proxy_webhook_test.go @@ -12,35 +12,37 @@ // See the License for the specific language governing permissions and // limitations under the License. -package v1alpha1 +package webhook import ( "context" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/matrixorigin/matrixone-operator/api/core/v1alpha1" ) var _ = Describe("ProxySet Webhook", func() { It("should hook proxyset", func() { - ps := &ProxySet{ + ps := &v1alpha1.ProxySet{ ObjectMeta: metav1.ObjectMeta{ Name: "proxy-" + randomString(5), Namespace: "default", }, - Spec: ProxySetSpec{ - PodSet: PodSet{ + Spec: v1alpha1.ProxySetSpec{ + PodSet: v1alpha1.PodSet{ Replicas: 2, - MainContainer: MainContainer{ + MainContainer: v1alpha1.MainContainer{ Image: "test", }, }, }, - Deps: ProxySetDeps{ - LogSetRef: LogSetRef{ - ExternalLogSet: &ExternalLogSet{}, + Deps: v1alpha1.ProxySetDeps{ + LogSetRef: v1alpha1.LogSetRef{ + ExternalLogSet: &v1alpha1.ExternalLogSet{}, }, }, } diff --git a/pkg/webhook/utils.go b/pkg/webhook/utils.go new file mode 100644 index 00000000..dd57df89 --- /dev/null +++ b/pkg/webhook/utils.go @@ -0,0 +1,97 @@ +// Copyright 2024 Matrix Origin +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package webhook + +import ( + "fmt" + + "github.com/go-errors/errors" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + "k8s.io/apimachinery/pkg/runtime" + + "github.com/matrixorigin/matrixone-operator/api/core/v1alpha1" +) + +var ( + // ServiceDefaultArgs is a cache variable for default args, should be read only in this package + ServiceDefaultArgs *DefaultArgs +) + +// DefaultArgs contain default service args for logservice/dn/tp, these default args set in matrixone-operator-cm configmap +type DefaultArgs struct { + LogService []string `json:"logService,omitempty"` + DN []string `json:"dn,omitempty"` + CN []string `json:"cn,omitempty"` + Proxy []string `json:"proxy,omitempty"` +} + +// setDefaultServiceArgs set default args for service, we only set default args when there is service args config in service spec +func setDefaultServiceArgs(object interface{}) { + if ServiceDefaultArgs == nil { + return + } + switch obj := object.(type) { + case *v1alpha1.LogSetSpec: + // set default arguments only when user does not set any arguments + if len(obj.ServiceArgs) == 0 { + obj.ServiceArgs = ServiceDefaultArgs.LogService + } + case *v1alpha1.DNSetSpec: + if len(obj.ServiceArgs) == 0 { + obj.ServiceArgs = ServiceDefaultArgs.DN + } + case *v1alpha1.CNSetSpec: + if len(obj.ServiceArgs) == 0 { + obj.ServiceArgs = ServiceDefaultArgs.CN + } + case *v1alpha1.ProxySetSpec: + if len(obj.ServiceArgs) == 0 { + obj.ServiceArgs = ServiceDefaultArgs.Proxy + } + default: + moLog.Error(fmt.Errorf("unknown type:%T", object), "expected types: *LogSetSpec, *DNSetSpec, *CNSetSpec") + return + } +} + +// setPodSetDefaults set default values in pod set +func setPodSetDefaults(s *v1alpha1.PodSet) { + if s.Overlay == nil { + s.Overlay = &v1alpha1.Overlay{} + } + s.Overlay.Env = appendIfNotExist(s.Overlay.Env, corev1.EnvVar{Name: v1alpha1.EnvGoDebug, Value: v1alpha1.DefaultGODebug}, func(v corev1.EnvVar) string { + return v.Name + }) +} + +func appendIfNotExist[K comparable, V any](list []V, elem V, keyFunc func(V) K) []V { + for _, o := range list { + if keyFunc(o) == keyFunc(elem) { + return list + } + } + return append(list, elem) +} + +func defaultDiskCacheSize(total *resource.Quantity) *resource.Quantity { + // shrink the total size since a small amount of space will be used for filesystem and metadata + shrunk := total.Value() * 9 / 10 + return resource.NewQuantity(shrunk, total.Format) +} + +func unexpectedKindError(expected string, obj runtime.Object) error { + return errors.Errorf("expected %s but received %T", expected, obj) +} diff --git a/pkg/webhook/webhook.go b/pkg/webhook/webhook.go new file mode 100644 index 00000000..2e0d3731 --- /dev/null +++ b/pkg/webhook/webhook.go @@ -0,0 +1,44 @@ +// Copyright 2024 Matrix Origin +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package webhook + +import ( + ctrl "sigs.k8s.io/controller-runtime" + logf "sigs.k8s.io/controller-runtime/pkg/log" +) + +var webhookLog = logf.Log.WithName("mo-webhook") + +func RegisterWebhooks(mgr ctrl.Manager) error { + if err := (cnSetWebhook{}).setupWebhookWithManager(mgr); err != nil { + return err + } + if err := (dnSetWebhook{}).setupWebhookWithManager(mgr); err != nil { + return err + } + if err := (logSetWebhook{}).setupWebhookWithManager(mgr); err != nil { + return err + } + if err := (proxySetWebhook{}).setupWebhookWithManager(mgr); err != nil { + return err + } + if err := (webUIWebhook{}).setupWebhookWithManager(mgr); err != nil { + return err + } + if err := (matrixOneClusterWebhook{}).setupWebhookWithManager(mgr); err != nil { + return err + } + return nil +} diff --git a/api/core/v1alpha1/webhook_suite_test.go b/pkg/webhook/webhook_suite_test.go similarity index 93% rename from api/core/v1alpha1/webhook_suite_test.go rename to pkg/webhook/webhook_suite_test.go index dbbc37d9..5728bfe8 100644 --- a/api/core/v1alpha1/webhook_suite_test.go +++ b/pkg/webhook/webhook_suite_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package v1alpha1 +package webhook import ( "context" @@ -26,9 +26,11 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + admissionv1beta1 "k8s.io/api/admission/v1beta1" corev1 "k8s.io/api/core/v1" - admissionv1beta1 "k8s.io/api/admission/v1beta1" + "github.com/matrixorigin/matrixone-operator/api/core/v1alpha1" + //+kubebuilder:scaffold:imports "k8s.io/apimachinery/pkg/runtime" ctrl "sigs.k8s.io/controller-runtime" @@ -59,10 +61,10 @@ var _ = BeforeSuite(func() { By("bootstrapping test environment") testEnv = &envtest.Environment{ - CRDDirectoryPaths: []string{filepath.Join("..", "..", "..", "deploy", "crds")}, + CRDDirectoryPaths: []string{filepath.Join("..", "..", "deploy", "crds")}, ErrorIfCRDPathMissing: false, WebhookInstallOptions: envtest.WebhookInstallOptions{ - Paths: []string{filepath.Join("..", "..", "..", "deploy", "webhook")}, + Paths: []string{filepath.Join("..", "..", "deploy", "webhook")}, }, } @@ -73,7 +75,7 @@ var _ = BeforeSuite(func() { scheme := runtime.NewScheme() By("should register schemes successfully") - err = AddToScheme(scheme) + err = v1alpha1.AddToScheme(scheme) Expect(err).NotTo(HaveOccurred()) err = admissionv1beta1.AddToScheme(scheme) Expect(err).NotTo(HaveOccurred()) diff --git a/api/core/v1alpha1/webui_webhook.go b/pkg/webhook/webui_webhook.go similarity index 51% rename from api/core/v1alpha1/webui_webhook.go rename to pkg/webhook/webui_webhook.go index 673d7044..2be572f5 100644 --- a/api/core/v1alpha1/webui_webhook.go +++ b/pkg/webhook/webui_webhook.go @@ -12,49 +12,66 @@ // See the License for the specific language governing permissions and // limitations under the License. -package v1alpha1 +package webhook import ( + "context" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/validation/field" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/webhook" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + "github.com/matrixorigin/matrixone-operator/api/core/v1alpha1" ) -func (r *WebUI) setupWebhookWithManager(mgr ctrl.Manager) error { +type webUIWebhook struct{} + +func (webUIWebhook) setupWebhookWithManager(mgr ctrl.Manager) error { return ctrl.NewWebhookManagedBy(mgr). - For(r). + For(&v1alpha1.WebUI{}). + WithDefaulter(&webUIDefaulter{}). + WithValidator(&webUIValidator{}). Complete() } // +kubebuilder:webhook:path=/mutate-core-matrixorigin-io-v1alpha1-webui,mutating=true,failurePolicy=fail,sideEffects=None,groups=core.matrixorigin.io,resources=webuis,verbs=create;update,versions=v1alpha1,name=mwebui.kb.io,admissionReviewVersions={v1,v1beta1} -var _ webhook.Defaulter = &WebUI{} +// webUIDefaulter implements webhook.Defaulter so a webhook will be registered for the v1alpha1.WebUI +type webUIDefaulter struct{} + +var _ webhook.CustomDefaulter = &webUIDefaulter{} -// Default implements webhook.Defaulter so a webhook will be registered for the type -func (r *WebUI) Default() { +func (w *webUIDefaulter) Default(_ context.Context, obj runtime.Object) error { + return nil } // +kubebuilder:webhook:path=/validate-core-matrixorigin-io-v1alpha1-webui,mutating=false,failurePolicy=fail,sideEffects=None,groups=core.matrixorigin.io,resources=webuis,verbs=create;update,versions=v1alpha1,name=vwebui.kb.io,admissionReviewVersions={v1,v1beta1} -var _ webhook.Validator = &WebUI{} +// webUIValidator implements webhook.Validator so a webhook will be registered for the v1alpha1.WebUI +type webUIValidator struct{} -// ValidateCreate implements webhook.Validator so a webhook will be registered for the type -func (r *WebUI) ValidateCreate() (admission.Warnings, error) { +var _ webhook.CustomValidator = &webUIValidator{} + +func (w *webUIValidator) ValidateCreate(_ context.Context, obj runtime.Object) (warnings admission.Warnings, err error) { + webui, ok := obj.(*v1alpha1.WebUI) + if !ok { + return nil, unexpectedKindError("WebUI", obj) + } var errs field.ErrorList - errs = append(errs, validateMainContainer(&r.Spec.MainContainer, field.NewPath("spec"))...) - return nil, invalidOrNil(errs, r) + errs = append(errs, validateMainContainer(&webui.Spec.MainContainer, field.NewPath("spec"))...) + return nil, invalidOrNil(errs, webui) } -func (r *WebUI) ValidateUpdate(old runtime.Object) (admission.Warnings, error) { - warnings, err := r.ValidateCreate() +func (w *webUIValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (warnings admission.Warnings, err error) { + warnings, err = w.ValidateCreate(ctx, newObj) if err != nil { return warnings, err } - return nil, nil + return warnings, nil } -func (r *WebUI) ValidateDelete() (admission.Warnings, error) { +func (w *webUIValidator) ValidateDelete(ctx context.Context, obj runtime.Object) (warnings admission.Warnings, err error) { return nil, nil } diff --git a/test/e2e/matrixonecluster_test.go b/test/e2e/matrixonecluster_test.go index 5030db09..20242165 100644 --- a/test/e2e/matrixonecluster_test.go +++ b/test/e2e/matrixonecluster_test.go @@ -20,6 +20,7 @@ import ( "github.com/matrixorigin/controller-runtime/pkg/util" "github.com/matrixorigin/matrixone-operator/api/core/v1alpha1" "github.com/matrixorigin/matrixone-operator/pkg/controllers/common" + mowebhook "github.com/matrixorigin/matrixone-operator/pkg/webhook" "github.com/matrixorigin/matrixone-operator/test/e2e/sql" e2eutil "github.com/matrixorigin/matrixone-operator/test/e2e/util" . "github.com/onsi/ginkgo/v2" @@ -291,7 +292,7 @@ var _ = Describe("MatrixOneCluster test", func() { minioSecret := e2eutil.MinioSecret(env.Namespace) minioProvider := e2eutil.MinioShareStorage(minioSecret.Name) Expect(kubeCli.Create(context.TODO(), minioSecret)).To(Succeed()) - maxLengthName := strings.Repeat("a", v1alpha1.MatrixOneClusterNameMaxLength) + maxLengthName := strings.Repeat("a", mowebhook.MatrixOneClusterNameMaxLength) mo := &v1alpha1.MatrixOneCluster{ ObjectMeta: metav1.ObjectMeta{ Namespace: env.Namespace, From 51a43a0a484672e3ec6437959db81f9da6de8670 Mon Sep 17 00:00:00 2001 From: cyberchen98 Date: Thu, 20 Jun 2024 14:32:35 +0800 Subject: [PATCH 2/3] fix: script & tests --- Makefile | 4 +- api/core/v1alpha1/common_helpers.go | 54 -------------------- cmd/operator/main.go | 2 +- pkg/webhook/matrixonecluster_webhook.go | 8 ++- pkg/webhook/matrixonecluster_webhook_test.go | 3 +- pkg/webhook/utils.go | 14 +---- pkg/webhook/webhook.go | 5 ++ 7 files changed, 18 insertions(+), 72 deletions(-) diff --git a/Makefile b/Makefile index 7342f9fc..dc7bc376 100644 --- a/Makefile +++ b/Makefile @@ -87,7 +87,7 @@ go-lint: golangci-lint check-license: license-eye $(LICENSE_EYE) -v info -c .licenserc.yml header check -LOCALBIN ?= $(shell pwd)/bin +LOCALBIN ?= $(shell pwd)/api/bin $(LOCALBIN): mkdir -p $(LOCALBIN) @@ -102,7 +102,7 @@ $(ENVTEST): $(LOCALBIN) test: api-test unit # Run unit tests -unit: generate fmt vet manifests +unit: generate fmt vet manifests envtest KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" CGO_ENABLED=0 go test ./pkg/... -coverprofile cover.out api-test: diff --git a/api/core/v1alpha1/common_helpers.go b/api/core/v1alpha1/common_helpers.go index fbf4856b..2e9a83e7 100644 --- a/api/core/v1alpha1/common_helpers.go +++ b/api/core/v1alpha1/common_helpers.go @@ -24,7 +24,6 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - logf "sigs.k8s.io/controller-runtime/pkg/log" "strings" "time" ) @@ -33,11 +32,6 @@ const ( reasonEmpty = "empty" ) -var ( - // ServiceDefaultArgs is a cache variable for default args, should be read only in this package - ServiceDefaultArgs *DefaultArgs -) - func (c *ConditionalStatus) SetCondition(condition metav1.Condition) { if c.Conditions == nil { c.Conditions = []metav1.Condition{} @@ -216,54 +210,6 @@ type DefaultArgs struct { Proxy []string `json:"proxy,omitempty"` } -// setDefaultServiceArgs set default args for service, we only set default args when there is service args config in service spec -func setDefaultServiceArgs(object interface{}) { - if ServiceDefaultArgs == nil { - return - } - switch obj := object.(type) { - case *LogSetSpec: - // set default arguments only when user does not set any arguments - if len(obj.ServiceArgs) == 0 { - obj.ServiceArgs = ServiceDefaultArgs.LogService - } - case *DNSetSpec: - if len(obj.ServiceArgs) == 0 { - obj.ServiceArgs = ServiceDefaultArgs.DN - } - case *CNSetSpec: - if len(obj.ServiceArgs) == 0 { - obj.ServiceArgs = ServiceDefaultArgs.CN - } - case *ProxySetSpec: - if len(obj.ServiceArgs) == 0 { - obj.ServiceArgs = ServiceDefaultArgs.Proxy - } - default: - logf.Log.WithName("mo-cluster").Error(fmt.Errorf("unknown type:%T", object), "expected types: *LogSetSpec, *DNSetSpec, *CNSetSpec") - return - } -} - -// setPodSetDefaults set default values in pod set -func setPodSetDefaults(s *PodSet) { - if s.Overlay == nil { - s.Overlay = &Overlay{} - } - s.Overlay.Env = appendIfNotExist(s.Overlay.Env, corev1.EnvVar{Name: EnvGoDebug, Value: DefaultGODebug}, func(v corev1.EnvVar) string { - return v.Name - }) -} - -func appendIfNotExist[K comparable, V any](list []V, elem V, keyFunc func(V) K) []V { - for _, o := range list { - if keyFunc(o) == keyFunc(elem) { - return list - } - } - return append(list, elem) -} - func GetCNPodUUID(pod *corev1.Pod) string { addr := fmt.Sprintf("%s.%s.%s.svc\n", pod.Name, pod.Spec.Subdomain, pod.Namespace) sum := sha256.Sum256([]byte(addr)) diff --git a/cmd/operator/main.go b/cmd/operator/main.go index 4c32cfe5..d055314a 100644 --- a/cmd/operator/main.go +++ b/cmd/operator/main.go @@ -147,7 +147,7 @@ func main() { controllermetrics.Registry.MustRegister(collector) if os.Getenv("ENABLE_WEBHOOKS") != "false" { - v1alpha1.ServiceDefaultArgs = operatorCfg.DefaultArgs + mowebhook.ServiceDefaultArgs = operatorCfg.DefaultArgs err := mowebhook.RegisterWebhooks(mgr) exitIf(err, "unable to set up webhook") diff --git a/pkg/webhook/matrixonecluster_webhook.go b/pkg/webhook/matrixonecluster_webhook.go index b916743d..5547d6b2 100644 --- a/pkg/webhook/matrixonecluster_webhook.go +++ b/pkg/webhook/matrixonecluster_webhook.go @@ -47,7 +47,13 @@ func (matrixOneClusterWebhook) setupWebhookWithManager(mgr ctrl.Manager) error { dn: &dnSetDefaulter{}, logService: &logSetDefaulter{}, }). - WithValidator(&matrixOneClusterValidator{}). + WithValidator(&matrixOneClusterValidator{ + cn: &cnSetValidator{}, + dn: &dnSetValidator{}, + logService: &logSetValidator{ + kClient: mgr.GetClient(), + }, + }). Complete() } diff --git a/pkg/webhook/matrixonecluster_webhook_test.go b/pkg/webhook/matrixonecluster_webhook_test.go index 4b57ebd2..6d8c44f6 100644 --- a/pkg/webhook/matrixonecluster_webhook_test.go +++ b/pkg/webhook/matrixonecluster_webhook_test.go @@ -127,8 +127,7 @@ var _ = Describe("MatrixOneCluster Webhook", func() { It("should validate and mutate MatrixOneCluster", func() { cluster := &v1alpha1.MatrixOneCluster{ ObjectMeta: metav1.ObjectMeta{ - Name: "mo-test-mutate-cc", - //Name: "mo-" + randomString(5), + Name: "mo-" + randomString(5), Namespace: "default", }, Spec: v1alpha1.MatrixOneClusterSpec{ diff --git a/pkg/webhook/utils.go b/pkg/webhook/utils.go index dd57df89..275abb7c 100644 --- a/pkg/webhook/utils.go +++ b/pkg/webhook/utils.go @@ -25,18 +25,8 @@ import ( "github.com/matrixorigin/matrixone-operator/api/core/v1alpha1" ) -var ( - // ServiceDefaultArgs is a cache variable for default args, should be read only in this package - ServiceDefaultArgs *DefaultArgs -) - -// DefaultArgs contain default service args for logservice/dn/tp, these default args set in matrixone-operator-cm configmap -type DefaultArgs struct { - LogService []string `json:"logService,omitempty"` - DN []string `json:"dn,omitempty"` - CN []string `json:"cn,omitempty"` - Proxy []string `json:"proxy,omitempty"` -} +// DefaultArgs alias to v1alpha1.DefaultArgs +type DefaultArgs = v1alpha1.DefaultArgs // setDefaultServiceArgs set default args for service, we only set default args when there is service args config in service spec func setDefaultServiceArgs(object interface{}) { diff --git a/pkg/webhook/webhook.go b/pkg/webhook/webhook.go index 2e0d3731..8ef7ffd6 100644 --- a/pkg/webhook/webhook.go +++ b/pkg/webhook/webhook.go @@ -19,6 +19,11 @@ import ( logf "sigs.k8s.io/controller-runtime/pkg/log" ) +var ( + // ServiceDefaultArgs is a cache variable for default args, should be read only in this package + ServiceDefaultArgs *DefaultArgs +) + var webhookLog = logf.Log.WithName("mo-webhook") func RegisterWebhooks(mgr ctrl.Manager) error { From 1692b1f15f74d9815e4eed705ab60a672ebc671e Mon Sep 17 00:00:00 2001 From: cyberchen98 Date: Thu, 20 Jun 2024 15:02:46 +0800 Subject: [PATCH 3/3] chore: fix go-lint --- pkg/webhook/cnset_webhook.go | 4 ++-- pkg/webhook/common.go | 2 +- pkg/webhook/dnset_webhook.go | 6 +++--- pkg/webhook/logset_webhook.go | 4 ++-- pkg/webhook/matrixonecluster_webhook.go | 8 ++++---- pkg/webhook/proxy_webhook.go | 8 ++++---- pkg/webhook/webui_webhook.go | 6 +++--- 7 files changed, 19 insertions(+), 19 deletions(-) diff --git a/pkg/webhook/cnset_webhook.go b/pkg/webhook/cnset_webhook.go index 9c4eee4b..476707aa 100644 --- a/pkg/webhook/cnset_webhook.go +++ b/pkg/webhook/cnset_webhook.go @@ -54,7 +54,7 @@ type cnSetDefaulter struct{} var _ webhook.CustomDefaulter = &cnSetDefaulter{} -func (c *cnSetDefaulter) Default(ctx context.Context, obj runtime.Object) error { +func (c *cnSetDefaulter) Default(_ context.Context, obj runtime.Object) error { cnSet, ok := obj.(*v1alpha1.CNSet) if !ok { return unexpectedKindError("CNSet", obj) @@ -125,7 +125,7 @@ func (c *cnSetValidator) ValidateUpdate(ctx context.Context, _, newObj runtime.O return warnings, nil } -func (c *cnSetValidator) ValidateDelete(_ context.Context, obj runtime.Object) (warnings admission.Warnings, err error) { +func (c *cnSetValidator) ValidateDelete(_ context.Context, _ runtime.Object) (warnings admission.Warnings, err error) { return nil, nil } diff --git a/pkg/webhook/common.go b/pkg/webhook/common.go index 0c3201ef..e0adfb3b 100644 --- a/pkg/webhook/common.go +++ b/pkg/webhook/common.go @@ -46,7 +46,7 @@ func validateMainContainer(c *v1alpha1.MainContainer, parent *field.Path) field. return errs } -func validateContainerResource(r *corev1.ResourceRequirements, parent *field.Path) field.ErrorList { +func validateContainerResource(_ *corev1.ResourceRequirements, _ *field.Path) field.ErrorList { // TODO: use kubernetes/api/validation.ValidatePodSpec to perform through Validation after we migrate // webhooks out of api package return nil diff --git a/pkg/webhook/dnset_webhook.go b/pkg/webhook/dnset_webhook.go index 8aef1090..6ba1f0ba 100644 --- a/pkg/webhook/dnset_webhook.go +++ b/pkg/webhook/dnset_webhook.go @@ -74,7 +74,7 @@ type dnSetValidator struct{} var _ webhook.CustomValidator = &dnSetValidator{} -func (d *dnSetValidator) ValidateCreate(ctx context.Context, obj runtime.Object) (warnings admission.Warnings, err error) { +func (d *dnSetValidator) ValidateCreate(_ context.Context, obj runtime.Object) (warnings admission.Warnings, err error) { dnSet, ok := obj.(*v1alpha1.DNSet) if !ok { return nil, unexpectedKindError("DNSet", obj) @@ -87,7 +87,7 @@ func (d *dnSetValidator) ValidateCreate(ctx context.Context, obj runtime.Object) return nil, invalidOrNil(errs, dnSet) } -func (d *dnSetValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (warnings admission.Warnings, err error) { +func (d *dnSetValidator) ValidateUpdate(ctx context.Context, _, newObj runtime.Object) (warnings admission.Warnings, err error) { warnings, err = d.ValidateCreate(ctx, newObj) if err != nil { return warnings, err @@ -95,7 +95,7 @@ func (d *dnSetValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runt return warnings, nil } -func (d *dnSetValidator) ValidateDelete(ctx context.Context, obj runtime.Object) (warnings admission.Warnings, err error) { +func (d *dnSetValidator) ValidateDelete(_ context.Context, _ runtime.Object) (warnings admission.Warnings, err error) { return nil, nil } diff --git a/pkg/webhook/logset_webhook.go b/pkg/webhook/logset_webhook.go index 6edc7d0e..acce6cec 100644 --- a/pkg/webhook/logset_webhook.go +++ b/pkg/webhook/logset_webhook.go @@ -61,7 +61,7 @@ type logSetDefaulter struct{} var _ webhook.CustomDefaulter = &logSetDefaulter{} -func (l *logSetDefaulter) Default(ctx context.Context, obj runtime.Object) error { +func (l *logSetDefaulter) Default(_ context.Context, obj runtime.Object) error { logSet, ok := obj.(*v1alpha1.LogSet) if !ok { return unexpectedKindError("LogSet", obj) @@ -157,7 +157,7 @@ func (l *logSetValidator) ValidateUpdate(_ context.Context, oldObj, newObj runti return nil, invalidOrNil(errs, logSet) } -func (l *logSetValidator) ValidateDelete(ctx context.Context, obj runtime.Object) (warnings admission.Warnings, err error) { +func (l *logSetValidator) ValidateDelete(_ context.Context, _ runtime.Object) (warnings admission.Warnings, err error) { return nil, nil } diff --git a/pkg/webhook/matrixonecluster_webhook.go b/pkg/webhook/matrixonecluster_webhook.go index 5547d6b2..5668f026 100644 --- a/pkg/webhook/matrixonecluster_webhook.go +++ b/pkg/webhook/matrixonecluster_webhook.go @@ -68,7 +68,7 @@ type matrixOneClusterDefaulter struct { var _ webhook.CustomDefaulter = &matrixOneClusterDefaulter{} -func (m *matrixOneClusterDefaulter) Default(ctx context.Context, obj runtime.Object) error { +func (m *matrixOneClusterDefaulter) Default(_ context.Context, obj runtime.Object) error { moc, ok := obj.(*v1alpha1.MatrixOneCluster) if !ok { return unexpectedKindError("MatrixOneCluster", obj) @@ -103,7 +103,7 @@ type matrixOneClusterValidator struct { var _ webhook.CustomValidator = &matrixOneClusterValidator{} -func (m *matrixOneClusterValidator) ValidateCreate(ctx context.Context, obj runtime.Object) (warnings admission.Warnings, err error) { +func (m *matrixOneClusterValidator) ValidateCreate(_ context.Context, obj runtime.Object) (warnings admission.Warnings, err error) { moc, ok := obj.(*v1alpha1.MatrixOneCluster) if !ok { return nil, unexpectedKindError("MatrixOneCluster", obj) @@ -128,7 +128,7 @@ func (m *matrixOneClusterValidator) ValidateCreate(ctx context.Context, obj runt return nil, invalidOrNil(errs, moc) } -func (m *matrixOneClusterValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (warnings admission.Warnings, err error) { +func (m *matrixOneClusterValidator) ValidateUpdate(_ context.Context, oldObj, newObj runtime.Object) (warnings admission.Warnings, err error) { var errs field.ErrorList moc := newObj.(*v1alpha1.MatrixOneCluster) errs = append(errs, m.validateMutateCommon(moc)...) @@ -138,7 +138,7 @@ func (m *matrixOneClusterValidator) ValidateUpdate(ctx context.Context, oldObj, return nil, invalidOrNil(errs, moc) } -func (m *matrixOneClusterValidator) ValidateDelete(ctx context.Context, obj runtime.Object) (warnings admission.Warnings, err error) { +func (m *matrixOneClusterValidator) ValidateDelete(_ context.Context, _ runtime.Object) (warnings admission.Warnings, err error) { return nil, nil } diff --git a/pkg/webhook/proxy_webhook.go b/pkg/webhook/proxy_webhook.go index 404f061c..9fb94adf 100644 --- a/pkg/webhook/proxy_webhook.go +++ b/pkg/webhook/proxy_webhook.go @@ -42,7 +42,7 @@ type proxySetDefaulter struct{} var _ webhook.CustomDefaulter = &proxySetDefaulter{} -func (p *proxySetDefaulter) Default(ctx context.Context, obj runtime.Object) error { +func (p *proxySetDefaulter) Default(_ context.Context, obj runtime.Object) error { proxySet, ok := obj.(*v1alpha1.ProxySet) if !ok { return unexpectedKindError("ProxySet", obj) @@ -62,14 +62,14 @@ type proxySetValidator struct{} var _ webhook.CustomValidator = &proxySetValidator{} -func (p proxySetValidator) ValidateCreate(ctx context.Context, obj runtime.Object) (warnings admission.Warnings, err error) { +func (p proxySetValidator) ValidateCreate(_ context.Context, _ runtime.Object) (warnings admission.Warnings, err error) { return nil, nil } -func (p proxySetValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (warnings admission.Warnings, err error) { +func (p proxySetValidator) ValidateUpdate(_ context.Context, _, _ runtime.Object) (warnings admission.Warnings, err error) { return nil, nil } -func (p proxySetValidator) ValidateDelete(ctx context.Context, obj runtime.Object) (warnings admission.Warnings, err error) { +func (p proxySetValidator) ValidateDelete(_ context.Context, _ runtime.Object) (warnings admission.Warnings, err error) { return nil, nil } diff --git a/pkg/webhook/webui_webhook.go b/pkg/webhook/webui_webhook.go index 2be572f5..9546bbab 100644 --- a/pkg/webhook/webui_webhook.go +++ b/pkg/webhook/webui_webhook.go @@ -43,7 +43,7 @@ type webUIDefaulter struct{} var _ webhook.CustomDefaulter = &webUIDefaulter{} -func (w *webUIDefaulter) Default(_ context.Context, obj runtime.Object) error { +func (w *webUIDefaulter) Default(_ context.Context, _ runtime.Object) error { return nil } @@ -64,7 +64,7 @@ func (w *webUIValidator) ValidateCreate(_ context.Context, obj runtime.Object) ( return nil, invalidOrNil(errs, webui) } -func (w *webUIValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (warnings admission.Warnings, err error) { +func (w *webUIValidator) ValidateUpdate(ctx context.Context, _, newObj runtime.Object) (warnings admission.Warnings, err error) { warnings, err = w.ValidateCreate(ctx, newObj) if err != nil { return warnings, err @@ -72,6 +72,6 @@ func (w *webUIValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runt return warnings, nil } -func (w *webUIValidator) ValidateDelete(ctx context.Context, obj runtime.Object) (warnings admission.Warnings, err error) { +func (w *webUIValidator) ValidateDelete(_ context.Context, _ runtime.Object) (warnings admission.Warnings, err error) { return nil, nil }