New field ExternalPort for GRPC Service to override --grpc-public-port arg (#278)

kobzonega · web-flow · commit 25b0d7ae4eb4 · 2025-01-28T21:59:38.000+08:00
diff --git a/.changes/unreleased/Added-20250104-143452.yaml b/.changes/unreleased/Added-20250104-143452.yaml
@@ -0,0 +1,3 @@
+kind: Added
+body: field externalPort for grpc service to override --grpc-public-port arg
+time: 2025-01-04T14:34:52.706824+08:00
diff --git a/.changes/unreleased/Fixed-20250104-143747.yaml b/.changes/unreleased/Fixed-20250104-143747.yaml
@@ -0,0 +1,3 @@
+kind: Fixed
+body: update the Makefile with the changes in GitHub CI
+time: 2025-01-04T14:37:47.689565+08:00
diff --git a/.gitignore b/.gitignore
@@ -14,6 +14,8 @@ com_crashlytics_export_strings.xml
 crashlytics.properties
 crashlytics-build.properties
 fabric.properties
+log.json
+log.txt
 
 bin/
 config/
diff --git a/Makefile b/Makefile
@@ -9,6 +9,8 @@ VERSION ?= 0.1.0
 IMG ?= cr.yandex/yc/ydb-operator:latest
 # ENVTEST_K8S_VERSION refers to the version of kubebuilder assets to be downloaded by envtest binary.
 ENVTEST_K8S_VERSION = 1.26
+# Image URL which uses in tests
+YDB_IMAGE ?= $(shell grep "anchor_for_fetching_image_from_workflow" ./tests/**/*.go | grep -o -E '"cr\.yandex.*"' | tr -d '"')
 
 # Get the currently used golang install path (in GOPATH/bin, unless GOBIN is set)
 ifeq (,$(shell go env GOBIN))
@@ -65,25 +67,29 @@ vet: ## Run go vet against code.
 
 kind-init:
 	if kind get clusters | grep "kind-ydb-operator"; then exit 0; fi; \
-	kind create cluster --config e2e/kind-cluster-config.yaml --name kind-ydb-operator; \
-	docker pull k8s.gcr.io/ingress-nginx/kube-webhook-certgen:v1.0; \
-	kind load docker-image k8s.gcr.io/ingress-nginx/kube-webhook-certgen:v1.0 --name kind-ydb-operator; \
-	docker pull cr.yandex/crptqonuodf51kdj7a7d/ydb:24.2.7; \
-	kind load docker-image cr.yandex/crptqonuodf51kdj7a7d/ydb:24.2.7 --name kind-ydb-operator
+	kind create cluster \
+	--config tests/cfg/kind-cluster-config.yaml \
+	--image=kindest/node:v1.31.2@sha256:18fbefc20a7113353c7b75b5c869d7145a6abd6269154825872dc59c1329912e \
+	--name kind-ydb-operator
+	docker pull k8s.gcr.io/ingress-nginx/kube-webhook-certgen:v1.0
+	kind load docker-image k8s.gcr.io/ingress-nginx/kube-webhook-certgen:v1.0 --name kind-ydb-operator
+	docker pull ${YDB_IMAGE}
+	kind load docker-image ${YDB_IMAGE} --name kind-ydb-operator
 
 kind-load:
-	docker tag cr.yandex/yc/ydb-operator:latest kind/ydb-operator:current
+	docker tag ${IMG} kind/ydb-operator:current
 	kind load docker-image kind/ydb-operator:current --name kind-ydb-operator
 
 opts ?= ''
 
 .PHONY: unit-test
 unit-test: manifests generate fmt vet envtest ## Run unit tests
-	KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use --arch=amd64 $(ENVTEST_K8S_VERSION) -p path)" go test -v -timeout 900s -p 1 ./internal/... -ginkgo.v -coverprofile cover.out $(opts)
+	KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use --arch=amd64 $(ENVTEST_K8S_VERSION) -p path)" \
+	go test -v -timeout 900s -p 1 ./internal/... -ginkgo.vv -coverprofile cover.out $(opts)
 
 .PHONY: e2e-test
 e2e-test: manifests generate fmt vet docker-build kind-init kind-load ## Run e2e tests
-	go test -v -timeout 3600s -p 1 ./e2e/... -ginkgo.v $(opts)
+	go test -v -timeout 3600s -p 1 ./tests/e2e/... -ginkgo.vv $(opts)
 
 .PHONY: test
 test: unit-test e2e-test ## Run all tests
diff --git a/api/v1alpha1/const.go b/api/v1alpha1/const.go
@@ -64,6 +64,7 @@ const (
 	AnnotationDisableLivenessProbe   = "ydb.tech/disable-liveness-probe"
 	AnnotationDataCenter             = "ydb.tech/data-center"
 	AnnotationGRPCPublicHost         = "ydb.tech/grpc-public-host"
+	AnnotationGRPCPublicPort         = "ydb.tech/grpc-public-port"
 	AnnotationNodeHost               = "ydb.tech/node-host"
 	AnnotationNodeDomain             = "ydb.tech/node-domain"
 	AnnotationAuthTokenSecretName    = "ydb.tech/auth-token-secret-name"
diff --git a/api/v1alpha1/service_types.go b/api/v1alpha1/service_types.go
@@ -21,7 +21,8 @@ type GRPCService struct {
 	Service `json:""`
 
 	TLSConfiguration *TLSConfiguration `json:"tls,omitempty"`
-	ExternalHost     string            `json:"externalHost,omitempty"` // TODO implementation
+	ExternalHost     string            `json:"externalHost,omitempty"`
+	ExternalPort     int32             `json:"externalPort,omitempty"`
 	IPDiscovery      *IPDiscovery      `json:"ipDiscovery,omitempty"`
 }
 
diff --git a/deploy/ydb-operator/crds/database.yaml b/deploy/ydb-operator/crds/database.yaml
@@ -4075,6 +4075,9 @@ spec:
                         type: object
                       externalHost:
                         type: string
+                      externalPort:
+                        format: int32
+                        type: integer
                       ipDiscovery:
                         properties:
                           enabled:
diff --git a/deploy/ydb-operator/crds/databasenodeset.yaml b/deploy/ydb-operator/crds/databasenodeset.yaml
@@ -2687,6 +2687,9 @@ spec:
                         type: object
                       externalHost:
                         type: string
+                      externalPort:
+                        format: int32
+                        type: integer
                       ipDiscovery:
                         properties:
                           enabled:
diff --git a/deploy/ydb-operator/crds/remotedatabasenodeset.yaml b/deploy/ydb-operator/crds/remotedatabasenodeset.yaml
@@ -2688,6 +2688,9 @@ spec:
                         type: object
                       externalHost:
                         type: string
+                      externalPort:
+                        format: int32
+                        type: integer
                       ipDiscovery:
                         properties:
                           enabled:
diff --git a/deploy/ydb-operator/crds/remotestoragenodeset.yaml b/deploy/ydb-operator/crds/remotestoragenodeset.yaml
@@ -2715,6 +2715,9 @@ spec:
                         type: object
                       externalHost:
                         type: string
+                      externalPort:
+                        format: int32
+                        type: integer
                       ipDiscovery:
                         properties:
                           enabled:
diff --git a/deploy/ydb-operator/crds/storage.yaml b/deploy/ydb-operator/crds/storage.yaml
@@ -5216,6 +5216,9 @@ spec:
                         type: object
                       externalHost:
                         type: string
+                      externalPort:
+                        format: int32
+                        type: integer
                       ipDiscovery:
                         properties:
                           enabled:
diff --git a/deploy/ydb-operator/crds/storagenodeset.yaml b/deploy/ydb-operator/crds/storagenodeset.yaml
@@ -2714,6 +2714,9 @@ spec:
                         type: object
                       externalHost:
                         type: string
+                      externalPort:
+                        format: int32
+                        type: integer
                       ipDiscovery:
                         properties:
                           enabled:
diff --git a/internal/controllers/database/controller_test.go b/internal/controllers/database/controller_test.go
@@ -24,6 +24,7 @@ import (
 	. "github.com/ydb-platform/ydb-kubernetes-operator/internal/controllers/constants"
 	"github.com/ydb-platform/ydb-kubernetes-operator/internal/controllers/database"
 	"github.com/ydb-platform/ydb-kubernetes-operator/internal/controllers/storage"
+	"github.com/ydb-platform/ydb-kubernetes-operator/internal/resources"
 	"github.com/ydb-platform/ydb-kubernetes-operator/internal/test"
 	testobjects "github.com/ydb-platform/ydb-kubernetes-operator/tests/test-k8s-objects"
 )
@@ -306,4 +307,85 @@ var _ = Describe("Database controller medium tests", func() {
 
 		Expect(args).To(ContainElements([]string{"--grpc-public-address-v4", "--grpc-public-target-name-override"}))
 	})
+
+	It("Check externalPort GRPC Service field propagation", func() {
+		By("Create test database")
+		databaseSample = *testobjects.DefaultDatabase()
+		Expect(k8sClient.Create(ctx, &databaseSample)).Should(Succeed())
+
+		checkPublicPortArg := func(expectedGRPCPort string) error {
+			foundStatefulSet := appsv1.StatefulSet{}
+			Eventually(func() error {
+				return k8sClient.Get(ctx,
+					types.NamespacedName{
+						Name:      testobjects.DatabaseName,
+						Namespace: testobjects.YdbNamespace,
+					},
+					&foundStatefulSet,
+				)
+			}, test.Timeout, test.Interval).Should(Succeed())
+			podContainerArgs := foundStatefulSet.Spec.Template.Spec.Containers[0].Args
+			for idx, argKey := range podContainerArgs {
+				if argKey == "--grpc-public-port" {
+					if podContainerArgs[idx+1] != expectedGRPCPort {
+						return fmt.Errorf(
+							"Found arg `--grpc-public-port` value %s does not match with expected: %s",
+							podContainerArgs[idx+1],
+							expectedGRPCPort,
+						)
+					}
+				}
+			}
+			return nil
+		}
+
+		By("Check that args `--grpc-public-host` and `--grpc-public-port` propagated to StatefulSet pods...")
+		Eventually(
+			checkPublicPortArg(fmt.Sprintf("%d", v1alpha1.GRPCPort)),
+			test.Timeout,
+			test.Interval).ShouldNot(HaveOccurred())
+
+		externalPort := int32(30001)
+		By("Update externalHost and externalPort for Database GRPC Service...", func() {
+			database := v1alpha1.Database{}
+			Expect(k8sClient.Get(ctx, types.NamespacedName{
+				Name:      testobjects.DatabaseName,
+				Namespace: testobjects.YdbNamespace,
+			}, &database))
+			database.Spec.Service.GRPC.ExternalPort = externalPort
+			Expect(k8sClient.Update(ctx, &database)).Should(Succeed())
+		})
+
+		By("Check that type was updated for Database GRPC Service to NodePort...")
+		Eventually(func() error {
+			databaseGRPCService := corev1.Service{}
+			Expect(k8sClient.Get(ctx, types.NamespacedName{
+				Name:      fmt.Sprintf(resources.GRPCServiceNameFormat, databaseSample.Name),
+				Namespace: testobjects.YdbNamespace,
+			}, &databaseGRPCService))
+			if databaseGRPCService.Spec.Type != corev1.ServiceTypeNodePort {
+				return fmt.Errorf(
+					"Found GRPC Service .spec.type %s does not match with expected: %s",
+					databaseGRPCService.Spec.Type,
+					corev1.ServiceTypeNodePort,
+				)
+			}
+			for _, port := range databaseGRPCService.Spec.Ports {
+				if port.NodePort != externalPort {
+					return fmt.Errorf(
+						"Found GRPC Service NodePort value %d does not match with expected: %s",
+						port.NodePort,
+						fmt.Sprintf("%d", externalPort),
+					)
+				}
+			}
+			return nil
+		}, test.Timeout, test.Interval).ShouldNot(HaveOccurred())
+
+		By("Check that args `--grpc-public-port` was updated in StatefulSet...")
+		Eventually(
+			checkPublicPortArg(fmt.Sprintf("%d", externalPort)),
+			test.Timeout,
+			test.Interval).ShouldNot(HaveOccurred())
+	})
 })
diff --git a/internal/resources/database.go b/internal/resources/database.go
@@ -175,8 +175,9 @@ func (b *DatabaseBuilder) GetResourceBuilders(restConfig *rest.Config) []Resourc
 			SelectorLabels: databaseSelectorLabels,
 			Annotations:    b.Spec.Service.GRPC.AdditionalAnnotations,
 			Ports: []corev1.ServicePort{{
-				Name: api.GRPCServicePortName,
-				Port: api.GRPCPort,
+				Name:     api.GRPCServicePortName,
+				Port:     api.GRPCPort,
+				NodePort: b.Spec.Service.GRPC.ExternalPort,
 			}},
 			IPFamilies:     b.Spec.Service.GRPC.IPFamilies,
 			IPFamilyPolicy: b.Spec.Service.GRPC.IPFamilyPolicy,
diff --git a/internal/resources/database_statefulset.go b/internal/resources/database_statefulset.go
@@ -4,7 +4,6 @@ import (
 	"errors"
 	"fmt"
 	"regexp"
-	"strconv"
 
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -74,6 +73,15 @@ func (b *DatabaseStatefulSetBuilder) buildEnv() []corev1.EnvVar {
 	var envVars []corev1.EnvVar
 
 	envVars = append(envVars,
+		corev1.EnvVar{
+			Name: "POD_NAME", // for `--grpc-public-host` flag
+			ValueFrom: &corev1.EnvVarSource{
+				FieldRef: &corev1.ObjectFieldSelector{
+					APIVersion: "v1",
+					FieldPath:  "metadata.name",
+				},
+			},
+		},
 		corev1.EnvVar{
 			Name: "NODE_NAME", // for `--grpc-public-host` flag
 			ValueFrom: &corev1.EnvVarSource{
@@ -640,10 +648,11 @@ func (b *DatabaseStatefulSetBuilder) buildContainerArgs() ([]string, []string) {
 	}
 
 	publicHostOption := "--grpc-public-host"
-	publicHost := fmt.Sprintf(api.InterconnectServiceFQDNFormat, b.Database.Name, b.GetNamespace(), domain) // FIXME .svc.cluster.local
+	publicHostDomain := fmt.Sprintf(api.InterconnectServiceFQDNFormat, b.Database.Name, b.GetNamespace(), domain)
+	publicHost := fmt.Sprintf("%s.%s", "$(POD_NAME)", publicHostDomain)
 
 	if b.Spec.Service.GRPC.ExternalHost != "" {
-		publicHost = b.Spec.Service.GRPC.ExternalHost
+		publicHost = fmt.Sprintf("%s.%s", "$(POD_NAME)", b.Spec.Service.GRPC.ExternalHost)
 	}
 	if value, ok := b.ObjectMeta.Annotations[api.AnnotationGRPCPublicHost]; ok {
 		publicHost = value
@@ -667,22 +676,28 @@ func (b *DatabaseStatefulSetBuilder) buildContainerArgs() ([]string, []string) {
 			args = append(
 				args,
 				"--grpc-public-target-name-override",
-				fmt.Sprintf("%s.%s", "$(NODE_NAME)", targetNameOverride),
+				fmt.Sprintf("%s.%s", "$(POD_NAME)", targetNameOverride),
 			)
 		}
 	}
 
 	publicPortOption := "--grpc-public-port"
-	publicPort := api.GRPCPort
+	publicPort := fmt.Sprintf("%d", api.GRPCPort)
+	if b.Spec.Service.GRPC.ExternalPort > 0 {
+		publicPort = fmt.Sprintf("%d", b.Spec.Service.GRPC.ExternalPort)
+	}
+	if value, ok := b.ObjectMeta.Annotations[api.AnnotationGRPCPublicPort]; ok {
+		publicPort = value
+	}
 
 	args = append(
 		args,
 
 		publicHostOption,
-		fmt.Sprintf("%s.%s", "$(NODE_NAME)", publicHost), // fixme $(NODE_NAME)
+		publicHost,
 
 		publicPortOption,
-		strconv.Itoa(publicPort),
+		publicPort,
 	)
 
 	if value, ok := b.ObjectMeta.Annotations[api.AnnotationDataCenter]; ok {
diff --git a/internal/resources/service.go b/internal/resources/service.go
@@ -62,6 +62,12 @@ func (b *ServiceBuilder) Build(obj client.Object) error {
 		service.Spec.ClusterIP = "None"
 	}
 
+	for _, port := range service.Spec.Ports {
+		if port.NodePort > 0 {
+			service.Spec.Type = corev1.ServiceTypeNodePort
+		}
+	}
+
 	return nil
 }
 
diff --git a/tests/cfg/kind-cluster-config.yaml b/tests/cfg/kind-cluster-config.yaml
@@ -4,6 +4,11 @@ nodes:
 - role: control-plane
 
 - role: worker
+  extraPortMappings:
+  - containerPort: 30001
+    hostPort: 30001
+    listenAddress: "127.0.0.1"
+    protocol: tcp
   labels:
     topology.kubernetes.io/zone: az-1
     worker: true
diff --git a/tests/cfg/operator-local-values.yaml b/tests/cfg/operator-local-values.yaml
@@ -1,3 +1,8 @@
+image:
+  pullPolicy: IfNotPresent
+  repository: kind/ydb-operator
+  tag: current
+
 webhook:
   enabled: true
 
diff --git a/tests/e2e/smoke_test.go b/tests/e2e/smoke_test.go
@@ -733,6 +733,41 @@ var _ = Describe("Operator smoke test", func() {
 		ExecuteSimpleTableE2ETest(podName, testobjects.YdbNamespace, storageEndpoint, databasePath)
 	})
 
+	It("Check externalPort for Database", func() {
+		By("create storage...")
+		Expect(k8sClient.Create(ctx, storageSample)).Should(Succeed())
+		defer DeleteStorageSafely(ctx, k8sClient, storageSample)
+		By("create database...")
+		databaseSample.Spec.Nodes = 1
+		databaseSample.Spec.NodeSelector = map[string]string{
+			"topology.kubernetes.io/zone": "az-1",
+		}
+		databaseSample.Annotations = map[string]string{
+			v1alpha1.AnnotationGRPCPublicHost: "localhost",
+		}
+		databaseSample.Spec.Service.GRPC.ExternalPort = 30001
+		Expect(k8sClient.Create(ctx, databaseSample)).Should(Succeed())
+		defer func() {
+			Expect(k8sClient.Delete(ctx, databaseSample)).Should(Succeed())
+		}()
+
+		By("waiting until Storage is ready...")
+		WaitUntilStorageReady(ctx, k8sClient, storageSample.Name, testobjects.YdbNamespace)
+
+		By("checking that all the storage pods are running and ready...")
+		CheckPodsRunningAndReady(ctx, k8sClient, "ydb-cluster", "kind-storage", storageSample.Spec.Nodes)
+
+		By("waiting until database is ready...")
+		WaitUntilDatabaseReady(ctx, k8sClient, databaseSample.Name, testobjects.YdbNamespace)
+
+		By("checking that all the database pods are running and ready...")
+		CheckPodsRunningAndReady(ctx, k8sClient, "ydb-cluster", "kind-database", databaseSample.Spec.Nodes)
+
+		By("execute simple query with ydb-go-sdk...")
+		databasePath := DatabasePathWithDefaultDomain(databaseSample)
+		ExecuteSimpleTableE2ETestWithSDK(databaseSample.Name, testobjects.YdbNamespace, databasePath)
+	})
+
 	AfterEach(func() {
 		UninstallOperatorWithHelm(testobjects.YdbNamespace)
 		Expect(k8sClient.Delete(ctx, &namespace)).Should(Succeed())
diff --git a/tests/test-utils/test-utils.go b/tests/test-utils/test-utils.go

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+kind: Added`
	`2`	`+body: field externalPort for grpc service to override --grpc-public-port arg`
	`3`	`+time: 2025-01-04T14:34:52.706824+08:00`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+kind: Fixed`
	`2`	`+body: update the Makefile with the changes in GitHub CI`
	`3`	`+time: 2025-01-04T14:37:47.689565+08:00`
Original file line number	Diff line number	Diff line change
`@@ -21,7 +21,8 @@ type GRPCService struct {`
`21`	`21`	Service `json:""`
`22`	`22`
`23`	`23`	TLSConfiguration *TLSConfiguration `json:"tls,omitempty"`
`24`		- ExternalHost string `json:"externalHost,omitempty"` // TODO implementation
	`24`	+ ExternalHost string `json:"externalHost,omitempty"`
	`25`	+ ExternalPort int32 `json:"externalPort,omitempty"`
`25`	`26`	IPDiscovery *IPDiscovery `json:"ipDiscovery,omitempty"`
`26`	`27`	`}`
`27`	`28`