diff --git a/.github/workflows/ci-tests.yml b/.github/workflows/ci-tests.yml new file mode 100644 index 000000000..e756d0ae0 --- /dev/null +++ b/.github/workflows/ci-tests.yml @@ -0,0 +1,41 @@ +name: run tests + +on: + pull_request: + branches: + - master + - "*_release" + paths: + - '**/*.go' + push: + branches: + - master + - "*_release" + paths: + - '**/*.go' + +env: + GO_VERSION: "1.22" + +jobs: + run-tests: + runs-on: ubuntu-latest + + steps: + - name: Checkout repository + uses: actions/checkout@v3 + + - name: Set up Go 1.x + uses: actions/setup-go@v3 + with: + go-version: ${{ env.GO_VERSION }} + + - name: go mod vendor + run: go mod vendor + + - name: install tools + run: make tools + + - name: test webhooks + run: make test-webhooks + \ No newline at end of file diff --git a/api/v1alpha1/obcluster_types.go b/api/v1alpha1/obcluster_types.go index 7c844c56e..2f29d3a73 100644 --- a/api/v1alpha1/obcluster_types.go +++ b/api/v1alpha1/obcluster_types.go @@ -88,5 +88,7 @@ func init() { } func (c *OBCluster) SupportStaticIP() bool { - return c.Annotations[oceanbaseconst.AnnotationsSupportStaticIP] == "true" + return c.Annotations[oceanbaseconst.AnnotationsSupportStaticIP] == "true" || + c.Annotations[oceanbaseconst.AnnotationsMode] == oceanbaseconst.ModeService || + c.Annotations[oceanbaseconst.AnnotationsMode] == oceanbaseconst.ModeStandalone } diff --git a/api/v1alpha1/obcluster_webhook_test.go b/api/v1alpha1/obcluster_webhook_test.go index 97b80b436..58cef98f5 100644 --- a/api/v1alpha1/obcluster_webhook_test.go +++ b/api/v1alpha1/obcluster_webhook_test.go @@ -148,7 +148,7 @@ var _ = Describe("Test OBCluster Webhook", Label("webhook"), func() { It("Validate existence of secrets", func() { By("Create normal cluster") - cluster := newOBCluster("test", 1, 1) + cluster := newOBCluster("test3", 1, 1) cluster.Spec.UserSecrets.Monitor = "" cluster.Spec.UserSecrets.ProxyRO = "" cluster.Spec.UserSecrets.Operator = "" @@ -158,17 +158,19 @@ var _ = Describe("Test OBCluster Webhook", Label("webhook"), func() { cluster2.Spec.UserSecrets.Monitor = "secret-that-does-not-exist" cluster2.Spec.UserSecrets.ProxyRO = "" cluster2.Spec.UserSecrets.Operator = "" - Expect(k8sClient.Create(ctx, cluster)).ShouldNot(Succeed()) + Expect(k8sClient.Create(ctx, cluster2)).Should(Succeed()) + cluster3 := newOBCluster("test3", 1, 1) cluster2.Spec.UserSecrets.Monitor = wrongKeySecret - Expect(k8sClient.Create(ctx, cluster)).ShouldNot(Succeed()) + Expect(k8sClient.Create(ctx, cluster3)).ShouldNot(Succeed()) Expect(k8sClient.Delete(ctx, cluster)).Should(Succeed()) + Expect(k8sClient.Delete(ctx, cluster2)).Should(Succeed()) }) It("Validate secrets creation and fetch them", func() { By("Create normal cluster") - cluster := newOBCluster("test", 1, 1) + cluster := newOBCluster("test-create-secrets", 1, 1) cluster.Spec.UserSecrets.Monitor = "" cluster.Spec.UserSecrets.ProxyRO = "" cluster.Spec.UserSecrets.Operator = "" @@ -178,6 +180,7 @@ var _ = Describe("Test OBCluster Webhook", Label("webhook"), func() { Expect(cluster.Spec.UserSecrets.Monitor).ShouldNot(BeEmpty()) Expect(cluster.Spec.UserSecrets.ProxyRO).ShouldNot(BeEmpty()) Expect(cluster.Spec.UserSecrets.Operator).ShouldNot(BeEmpty()) + Expect(k8sClient.Delete(ctx, cluster)).Should(Succeed()) }) It("Validate single pvc with multiple storage classes", func() { diff --git a/api/v1alpha1/obtenantbackuppolicy_webhook.go b/api/v1alpha1/obtenantbackuppolicy_webhook.go index 66ac4f818..4ae6a9c27 100644 --- a/api/v1alpha1/obtenantbackuppolicy_webhook.go +++ b/api/v1alpha1/obtenantbackuppolicy_webhook.go @@ -304,44 +304,46 @@ func (r *OBTenantBackupPolicy) validateDestination(cluster *OBCluster, dest *api if !pattern.MatchString(dest.Path) { return field.Invalid(field.NewPath("spec").Child(fieldName).Child("destination"), dest.Path, "invalid backup destination path, the path format should be "+pattern.String()) } - if dest.Type != constants.BackupDestTypeNFS && dest.OSSAccessSecret == "" { - return field.Invalid(field.NewPath("spec").Child(fieldName).Child("destination"), dest.OSSAccessSecret, "OSSAccessSecret is required when backing up data to OSS, COS or S3") - } - secret := &v1.Secret{} - err := bakClt.Get(context.Background(), types.NamespacedName{ - Namespace: r.GetNamespace(), - Name: dest.OSSAccessSecret, - }, secret) - fieldPath := field.NewPath("spec").Child(fieldName).Child("destination").Child("ossAccessSecret") - if err != nil { - if apierrors.IsNotFound(err) { - return field.Invalid(fieldPath, dest.OSSAccessSecret, "Given OSSAccessSecret not found") - } - return field.InternalError(fieldPath, err) - } - // All the following types need accessId and accessKey - switch dest.Type { - case - constants.BackupDestTypeCOS, - constants.BackupDestTypeOSS, - constants.BackupDestTypeS3, - constants.BackupDestTypeS3Compatible: - if _, ok := secret.Data["accessId"]; !ok { - return field.Invalid(fieldPath, dest.OSSAccessSecret, "accessId field not found in given OSSAccessSecret") + if dest.Type != constants.BackupDestTypeNFS { + if dest.OSSAccessSecret == "" { + return field.Invalid(field.NewPath("spec").Child(fieldName).Child("destination"), dest.OSSAccessSecret, "OSSAccessSecret is required when backing up data to OSS, COS or S3") } - if _, ok := secret.Data["accessKey"]; !ok { - return field.Invalid(fieldPath, dest.OSSAccessSecret, "accessKey field not found in given OSSAccessSecret") + secret := &v1.Secret{} + err := bakClt.Get(context.Background(), types.NamespacedName{ + Namespace: r.GetNamespace(), + Name: dest.OSSAccessSecret, + }, secret) + fieldPath := field.NewPath("spec").Child(fieldName).Child("destination").Child("ossAccessSecret") + if err != nil { + if apierrors.IsNotFound(err) { + return field.Invalid(fieldPath, dest.OSSAccessSecret, "Given OSSAccessSecret not found") + } + return field.InternalError(fieldPath, err) } - } - // The following types need additional fields - switch dest.Type { - case constants.BackupDestTypeCOS: - if _, ok := secret.Data["appId"]; !ok { - return field.Invalid(fieldPath, dest.OSSAccessSecret, "appId field not found in given OSSAccessSecret") + // All the following types need accessId and accessKey + switch dest.Type { + case + constants.BackupDestTypeCOS, + constants.BackupDestTypeOSS, + constants.BackupDestTypeS3, + constants.BackupDestTypeS3Compatible: + if _, ok := secret.Data["accessId"]; !ok { + return field.Invalid(fieldPath, dest.OSSAccessSecret, "accessId field not found in given OSSAccessSecret") + } + if _, ok := secret.Data["accessKey"]; !ok { + return field.Invalid(fieldPath, dest.OSSAccessSecret, "accessKey field not found in given OSSAccessSecret") + } } - case constants.BackupDestTypeS3: - if _, ok := secret.Data["s3Region"]; !ok { - return field.Invalid(fieldPath, dest.OSSAccessSecret, "s3Region field not found in given OSSAccessSecret") + // The following types need additional fields + switch dest.Type { + case constants.BackupDestTypeCOS: + if _, ok := secret.Data["appId"]; !ok { + return field.Invalid(fieldPath, dest.OSSAccessSecret, "appId field not found in given OSSAccessSecret") + } + case constants.BackupDestTypeS3: + if _, ok := secret.Data["s3Region"]; !ok { + return field.Invalid(fieldPath, dest.OSSAccessSecret, "s3Region field not found in given OSSAccessSecret") + } } } return nil diff --git a/api/v1alpha1/obtenantoperation_webhook.go b/api/v1alpha1/obtenantoperation_webhook.go index 994551f12..a7e90643c 100644 --- a/api/v1alpha1/obtenantoperation_webhook.go +++ b/api/v1alpha1/obtenantoperation_webhook.go @@ -226,15 +226,23 @@ func (r *OBTenantOperation) validateMutation() error { allErrs = append(allErrs, field.Invalid(field.NewPath("spec").Child("targetTenant"), r.Spec.TargetTenant, "The target tenant is not a standby")) } } + case constants.TenantOpSetUnitNumber, + constants.TenantOpSetConnectWhiteList, + constants.TenantOpAddResourcePools, + constants.TenantOpModifyResourcePools, + constants.TenantOpDeleteResourcePools: + return r.validateNewOperations() default: - if r.Spec.TargetTenant == nil { - allErrs = append(allErrs, field.Required(field.NewPath("spec").Child("targetTenant"), "name of targetTenant is required")) - } - } - if len(allErrs) != 0 { - return allErrs.ToAggregate() + allErrs = append(allErrs, field.Required(field.NewPath("spec").Child("type"), string(r.Spec.Type)+" type of operation is not supported")) } + return allErrs.ToAggregate() +} +func (r *OBTenantOperation) validateNewOperations() error { + if r.Spec.TargetTenant == nil { + return field.Required(field.NewPath("spec").Child("targetTenant"), "name of targetTenant is required") + } + allErrs := field.ErrorList{} obtenant := &OBTenant{} err := clt.Get(context.Background(), types.NamespacedName{Name: *r.Spec.TargetTenant, Namespace: r.Namespace}, obtenant) if err != nil { @@ -274,9 +282,11 @@ func (r *OBTenantOperation) validateMutation() error { } else { allErrs = append(allErrs, field.Invalid(field.NewPath("spec").Child("targetTenant"), r.Spec.TargetTenant, "The target tenant's cluster "+obtenant.Spec.ClusterName+" does not exist")) } + break } if obcluster.Spec.Topology == nil { allErrs = append(allErrs, field.Invalid(field.NewPath("spec").Child("targetTenant"), r.Spec.TargetTenant, "The target tenant's cluster "+obtenant.Spec.ClusterName+" does not have a topology")) + break } pools := make(map[string]any) for _, pool := range obtenant.Spec.Pools { @@ -287,6 +297,9 @@ func (r *OBTenantOperation) validateMutation() error { allErrs = append(allErrs, field.Invalid(field.NewPath("spec").Child("addResourcePools"), r.Spec.AddResourcePools, "The resource pool already exists")) } } + if len(allErrs) != 0 { + return allErrs.ToAggregate() + } zonesInOBCluster := make(map[string]any, len(obcluster.Spec.Topology)) for _, zone := range obcluster.Spec.Topology { zonesInOBCluster[zone.Zone] = struct{}{} @@ -313,6 +326,7 @@ func (r *OBTenantOperation) validateMutation() error { case constants.TenantOpDeleteResourcePools: if len(r.Spec.DeleteResourcePools) == 0 { allErrs = append(allErrs, field.Required(field.NewPath("spec").Child("deleteResourcePools"), "deleteResourcePools is required")) + break } pools := make(map[string]any) for _, pool := range obtenant.Spec.Pools { @@ -324,7 +338,6 @@ func (r *OBTenantOperation) validateMutation() error { } } default: - allErrs = append(allErrs, field.Required(field.NewPath("spec").Child("type"), string(r.Spec.Type)+" type of operation is not supported")) } return allErrs.ToAggregate() } diff --git a/api/v1alpha1/obtenantoperation_webhook_test.go b/api/v1alpha1/obtenantoperation_webhook_test.go index dc31104ac..58d7f2336 100644 --- a/api/v1alpha1/obtenantoperation_webhook_test.go +++ b/api/v1alpha1/obtenantoperation_webhook_test.go @@ -13,10 +13,11 @@ See the Mulan PSL v2 for more details. package v1alpha1 import ( - apiconsts "github.com/oceanbase/ob-operator/api/constants" - . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/api/resource" + + apiconsts "github.com/oceanbase/ob-operator/api/constants" ) var _ = Describe("Test OBTenantOperation Webhook", Label("webhook"), Serial, func() { @@ -25,7 +26,7 @@ var _ = Describe("Test OBTenantOperation Webhook", Label("webhook"), Serial, fun tenantStandby := "test-tenant-for-operation2" It("Create cluster and tenants", func() { - c := newOBCluster(clusterName, 1, 1) + c := newOBCluster(clusterName, 3, 1) t := newOBTenant(tenantPrimary, clusterName) t2 := newOBTenant(tenantStandby, clusterName) t2.Spec.TenantRole = apiconsts.TenantRoleStandby @@ -35,6 +36,10 @@ var _ = Describe("Test OBTenantOperation Webhook", Label("webhook"), Serial, fun Expect(k8sClient.Create(ctx, c)).Should(Succeed()) Expect(k8sClient.Create(ctx, t)).Should(Succeed()) Expect(k8sClient.Create(ctx, t2)).Should(Succeed()) + + t.Status.TenantRole = apiconsts.TenantRolePrimary + t.Status.Pools = []ResourcePoolStatus{} + Expect(k8sClient.Status().Update(ctx, t)).Should(Succeed()) }) It("Check operation types", func() { @@ -119,6 +124,9 @@ var _ = Describe("Test OBTenantOperation Webhook", Label("webhook"), Serial, fun notexist := "tenant-not-exist" op.Spec.TargetTenant = ¬exist Expect(k8sClient.Create(ctx, op)).ShouldNot(Succeed()) + + op.Spec.TargetTenant = &tenantPrimary + Expect(k8sClient.Create(ctx, op)).Should(Succeed()) }) It("Check operation replay log", func() { @@ -144,4 +152,90 @@ var _ = Describe("Test OBTenantOperation Webhook", Label("webhook"), Serial, fun } Expect(k8sClient.Create(ctx, op)).ShouldNot(Succeed()) }) + + It("Check adding resource pools", func() { + op := newTenantOperation(tenantPrimary) + op.Spec.Type = apiconsts.TenantOpAddResourcePools + op.Spec.AddResourcePools = []ResourcePoolSpec{{ + Zone: "zone1", + Type: &LocalityType{ + Name: "Full", + Replica: 1, + IsActive: true, + }, + UnitConfig: &UnitConfig{ + MaxCPU: resource.MustParse("1"), + MemorySize: resource.MustParse("5Gi"), + MinCPU: resource.MustParse("1"), + MaxIops: 1024, + MinIops: 1024, + IopsWeight: 2, + LogDiskSize: resource.MustParse("12Gi"), + }, + }} + + notexist := "tenant-not-exist" + op.Spec.TargetTenant = ¬exist + + Expect(k8sClient.Create(ctx, op)).ShouldNot(Succeed()) + + op.Spec.TargetTenant = &tenantPrimary + Expect(k8sClient.Create(ctx, op)).ShouldNot(Succeed()) + + op.Spec.Force = true + Expect(k8sClient.Create(ctx, op)).Should(Succeed()) + + // Delete resource pool + opDel := newTenantOperation(tenantPrimary) + opDel.Spec.Type = apiconsts.TenantOpDeleteResourcePools + opDel.Spec.DeleteResourcePools = []string{"zone0"} + opDel.Spec.TargetTenant = &tenantPrimary + Expect(k8sClient.Create(ctx, opDel)).ShouldNot(Succeed()) + opDel.Spec.Force = true + Expect(k8sClient.Create(ctx, opDel)).Should(Succeed()) + }) + + It("Check modifying resource pools", func() { + op := newTenantOperation(tenantPrimary) + op.Spec.Type = apiconsts.TenantOpModifyResourcePools + op.Spec.ModifyResourcePools = []ResourcePoolSpec{{ + Zone: "zone0", + Type: &LocalityType{ + Name: "Full", + Replica: 1, + IsActive: true, + }, + UnitConfig: &UnitConfig{ + MaxCPU: resource.MustParse("6"), + MemorySize: resource.MustParse("6Gi"), + MinCPU: resource.MustParse("2"), + MaxIops: 1024, + MinIops: 1024, + IopsWeight: 2, + LogDiskSize: resource.MustParse("12Gi"), + }, + }} + + op.Spec.TargetTenant = &tenantPrimary + Expect(k8sClient.Create(ctx, op)).ShouldNot(Succeed()) + + op.Spec.Force = true + Expect(k8sClient.Create(ctx, op)).Should(Succeed()) + }) + + It("Check setting connection white list", func() { + op := newTenantOperation(tenantPrimary) + op.Spec.Type = apiconsts.TenantOpSetConnectWhiteList + op.Spec.ConnectWhiteList = "%,127.0.0.1" + op.Spec.Force = true + Expect(k8sClient.Create(ctx, op)).Should(Succeed()) + }) + + It("Check setting unit number", func() { + op := newTenantOperation(tenantPrimary) + op.Spec.Type = apiconsts.TenantOpSetUnitNumber + op.Spec.UnitNumber = 2 + op.Spec.Force = true + Expect(k8sClient.Create(ctx, op)).Should(Succeed()) + }) }) diff --git a/api/v1alpha1/webhook_suite_test.go b/api/v1alpha1/webhook_suite_test.go index 9926ecb2a..5f103f02f 100644 --- a/api/v1alpha1/webhook_suite_test.go +++ b/api/v1alpha1/webhook_suite_test.go @@ -210,6 +210,8 @@ var _ = AfterSuite(func() { By("Clean auxiliary resources") cancel() By("tearing down the test environment") - err := testEnv.Stop() - Expect(err).NotTo(HaveOccurred()) + if testEnv != nil { + err := testEnv.Stop() + Expect(err).NotTo(HaveOccurred()) + } }) diff --git a/make/deps.mk b/make/deps.mk index ee9c53846..a6cedb1e4 100644 --- a/make/deps.mk +++ b/make/deps.mk @@ -39,7 +39,7 @@ install-delve: ## Install delve, a debugger for the Go programming language. Mor go install github.com/go-delve/delve/cmd/dlv@master .PHONY: tools -tools: kustomize controller-gen envtest install-delve ## Download all tools +tools: kustomize controller-gen envtest ## Download all tools .PHONY: init-generator init-generator: ## Install generator tools diff --git a/make/development.mk b/make/development.mk index 7f6b00e3b..e66f58d29 100644 --- a/make/development.mk +++ b/make/development.mk @@ -35,6 +35,11 @@ test-all: manifests generate fmt vet envtest ## Run all tests including long-run go run github.com/onsi/ginkgo/v2/ginkgo -r --covermode=atomic --coverprofile=cover.profile --cpuprofile=cpu.profile --memprofile=mem.profile --cover \ --output-dir=testreports --keep-going --json-report=report.json --label-filter='$(CASE_LABEL_FILTERS)' --skip-package './distribution' +.PHONY: test-webhooks +test-webhooks: ## Test the webhooks + KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" \ + go run github.com/onsi/ginkgo/v2/ginkgo ./api/... + REPORT_PORT ?= 8480 .PHONY: unit-coverage