feat: 添加istio策略下发服务 by dove0012 · Pull Request #3827 · TencentBlueKing/bk-bcs

dove0012 · 2026-01-27T03:32:14Z

添加istio策略下发服务

Copilot

Pull request overview

This PR introduces a new istio-policy-controller component to automatically manage Istio DestinationRule and VirtualService resources based on Kubernetes Service events and a YAML configuration, and wires it into the build and runtime packaging.

Changes:

Adds a controller-runtime based istio-policy-controller binary that watches Service resources and creates/updates/deletes corresponding Istio policies using configurable merge and cleanup behavior.
Introduces configuration, metrics scaffolding, and Go module dependencies required to drive traffic policy generation and integrate with Istio client-go.
Provides container packaging (Dockerfile, start script) and a Makefile target to build and package the new controller.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 15 comments.

Show a summary per file

File	Description
`install/conf/bcs-runtime/bcs-k8s/bcs-component/istio-policy-controller/container-start.sh`	Startup script for the istio-policy-controller container, including optional config rendering and a (currently outdated) flag usage comment block.
`install/conf/bcs-runtime/bcs-k8s/bcs-component/istio-policy-controller/Dockerfile`	Defines the istio-policy-controller container image, installing `envsubst` and wiring in the binary and start script.
`bcs-runtime/bcs-k8s/bcs-component/istio-policy-controller/main.go`	Entrypoint that parses flags, initializes logging/config, sets up the controller-runtime manager with Istio and cloud schemes, and registers the `ServiceReconciler`.
`bcs-runtime/bcs-k8s/bcs-component/istio-policy-controller/internal/option/option.go`	Declares the YAML configuration schema and controller options, and implements loading/unmarshalling of the config file into structured types.
`bcs-runtime/bcs-k8s/bcs-component/istio-policy-controller/internal/metric/metric.go`	Adds (currently unused) Prometheus client metrics definitions and helpers for reporting client request counts and latencies.
`bcs-runtime/bcs-k8s/bcs-component/istio-policy-controller/go.mod`	Creates a dedicated Go module for istio-policy-controller with Kubernetes, Istio, controller-runtime, and BCS dependencies and replaces.
`bcs-runtime/bcs-k8s/bcs-component/istio-policy-controller/controllers/util.go`	Provides helper constants, a `Service` event predicate, and reflection-based merging/empty-check utilities for manipulating Istio `TrafficPolicy` fields.
`bcs-runtime/bcs-k8s/bcs-component/istio-policy-controller/controllers/service_controller.go`	Implements the `ServiceReconciler` that reconciles `Service` resources to Istio `DestinationRule` and `VirtualService` objects, including create/update/cleanup logic driven by the loaded configuration.
`Makefile`	Adds an `istio-policy` target to build the istio-policy-controller binary and copy its configuration into the package layout.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-28T12:06:33Z

+ * either express or implied. See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+// package xxx


The // package xxx placeholder comment here is outdated and does not describe the option package; consider removing it or replacing it with a brief package‑level description of what these options/config types are used for.

Suggested change

// package xxx

// Package option defines configuration structures and controller options used by the istio-policy-controller.

Copilot · 2026-01-28T12:06:34Z

+	cliReqCounter = prometheus.NewCounterVec(
+		prometheus.CounterOpts{
+			Namespace: "bcs_network",
+			Subsystem: "cloudnetcontroller",
+			Name:      "cli_request_total",
+			Help:      "total request counter as client",
+		},


The Prometheus metrics use Subsystem: "cloudnetcontroller", which doesn’t match this component’s name and will make the exported metrics harder to discover and distinguish; consider updating the subsystem (and, if needed, metric names) to reflect istio-policy-controller.

Copilot · 2026-01-28T12:06:34Z

+		if svc.Name == name && svc.Namespace == namespace {
+			if svc.Setting.AutoGenerateVS {
+				_, err := sr.IstioClient.NetworkingV1().VirtualServices(namespace).
+					Create(ctx, vs, metav1.CreateOptions{})
+				if err != nil {


Here you create a VirtualService when a matching per‑service config has AutoGenerateVS enabled, but createVs will also unconditionally try to create the same VirtualService again if Global.Setting.AutoGenerateVS is true, which will cause an AlreadyExists error on the second call. It would be safer to ensure only one create path runs (e.g. by giving per‑service settings precedence over global, or by checking whether a VirtualService was already created).

Copilot · 2026-01-28T12:06:35Z

+exec /data/bcs/${module}/${module} $@
+
+# Usage of ./istio-policy-controller:
+#   -address string
+#         address for controller (default "127.0.0.1")
+#   -alsologtostderr
+#         log to standard error as well as files
+#   -cloud string
+#         cloud mode for bcs network controller (default "tencentcloud")
+#   -kubeconfig string
+#         Paths to a kubeconfig. Only required if out-of-cluster.
+#   -log_backtrace_at string
+#         when logging hits line file:N, emit a stack trace
+#   -log_dir string
+#         If non-empty, write log files in this directory (default "./logs")
+#   -log_max_num int
+#         Max num of log file. (default 10)
+#   -log_max_size uint
+#         Max size (MB) per log file. (default 500)
+#   -logtostderr
+#         log to standard error instead of files
+#   -master --kubeconfig
+#         (Deprecated: switch to --kubeconfig) The address of the Kubernetes API server. Overrides any value in kubeconfig. Only required if out-of-cluster.
+#   -metric_port int
+#         metric port for controller (default 8081)
+#   -port int
+#         por for controller (default 8080)
+#   -stderrthreshold string
+#         logs at or above this threshold go to stderr (default "2")
+#   -v int
+#         log level for V logs
+#   -vmodule string
+#         comma-separated list of pattern=N settings for file-filtered logging


The flag usage block here appears to be copied from another component: several flags listed (e.g. -cloud, -kubeconfig, -port) are not actually defined in main.go, and one description even has a typo (por for controller). This can confuse operators trying to start or debug the binary; consider updating the usage comments to reflect the real flag set or removing this block if it’s not maintained.

Suggested change

exec /data/bcs/${module}/${module} $@

# Usage of ./istio-policy-controller:

# -address string

# address for controller (default "127.0.0.1")

# -alsologtostderr

# log to standard error as well as files

# -cloud string

# cloud mode for bcs network controller (default "tencentcloud")

# -kubeconfig string

# Paths to a kubeconfig. Only required if out-of-cluster.

# -log_backtrace_at string

# when logging hits line file:N, emit a stack trace

# -log_dir string

# If non-empty, write log files in this directory (default "./logs")

# -log_max_num int

# Max num of log file. (default 10)

# -log_max_size uint

# Max size (MB) per log file. (default 500)

# -logtostderr

# log to standard error instead of files

# -master --kubeconfig

# (Deprecated: switch to --kubeconfig) The address of the Kubernetes API server. Overrides any value in kubeconfig. Only required if out-of-cluster.

# -metric_port int

# metric port for controller (default 8081)

# -port int

# por for controller (default 8080)

# -stderrthreshold string

# logs at or above this threshold go to stderr (default "2")

# -v int

# log level for V logs

# -vmodule string

# comma-separated list of pattern=N settings for file-filtered logging

exec /data/bcs/${module}/${module} $@

Copilot · 2026-01-28T12:06:35Z

+ * either express or implied. See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+// package xxx


The // package xxx placeholder comment no longer matches the actual package name and can be misleading; it’s better to remove or replace it with a meaningful description of this binary.

Suggested change

// package xxx

// istio-policy-controller is the entry point for the Istio policy controller binary.

Copilot · 2026-01-28T12:06:37Z

+		return err
+	}
+
+	ctrl.Log.WithName("config").Info(fmt.Sprintf("config content: %s", string(content)))


InitCfg logs the entire raw configuration file content, which can bloat logs and may inadvertently expose sensitive data if secrets are ever added to this config; it would be safer to either omit logging the full file or log only high‑level, non‑sensitive fields.

Suggested change

ctrl.Log.WithName("config").Info(fmt.Sprintf("config content: %s", string(content)))

ctrl.Log.WithName("config").Info(fmt.Sprintf("loaded config from %s", o.ConfigPath))

Copilot · 2026-01-28T12:06:37Z

+	dr.Spec.TrafficPolicy = tp
+
+	if isEmptyStruct(dr.Spec.TrafficPolicy.LoadBalancer) {
+		dr.Spec.TrafficPolicy.LoadBalancer = nil
+	}


dr.Spec.TrafficPolicy is set directly to tp, which can be nil when neither a service‑level nor a global TrafficPolicy is configured; the subsequent field accesses (e.g. dr.Spec.TrafficPolicy.LoadBalancer) will then panic with a nil pointer dereference. Consider keeping a non‑nil empty TrafficPolicy when no policy is provided, or guarding these field clean‑up calls with a dr.Spec.TrafficPolicy != nil check.

Copilot · 2026-01-28T12:06:37Z

+func init() {
+	metrics.Registry.MustRegister(cliReqCounter)
+	metrics.Registry.MustRegister(cliRespSummary)
+}
+
+// StatClientRequest report client request metrics
+func StatClientRequest(module, rpc string, respCode int, result string, inTime, outTime time.Time) {


StatClientRequest and the associated Prometheus vectors are defined but not used anywhere in this module, which may indicate dead code or an incomplete metrics integration; consider either using these metrics in the controller or removing them until they are needed.

Copilot · 2026-01-28T12:06:38Z

+		if client.IgnoreNotFound(err) != nil {
+			sr.Log.Error(err, "failed to get DestinationRule")
+		}
+	}
+
+	vs, err := sr.IstioClient.NetworkingV1().VirtualServices(namespace).Get(ctx, name, metav1.GetOptions{})
+	if err != nil {
+		if client.IgnoreNotFound(err) != nil {
+			sr.Log.Error(err, "failed to get VirtualService")


In this branch, non‑NotFound errors returned when fetching the DestinationRule are only logged and then ignored, so the reconciler never retries deletion and may leave policies orphaned if the API call failed transiently. It would be more robust to return the error (or a combined error) so that the controller can requeue and attempt cleanup again.

Suggested change

if client.IgnoreNotFound(err) != nil {

sr.Log.Error(err, "failed to get DestinationRule")

}

}

vs, err := sr.IstioClient.NetworkingV1().VirtualServices(namespace).Get(ctx, name, metav1.GetOptions{})

if err != nil {

if client.IgnoreNotFound(err) != nil {

sr.Log.Error(err, "failed to get VirtualService")

if ignErr := client.IgnoreNotFound(err); ignErr != nil {

sr.Log.Error(ignErr, "failed to get DestinationRule")

return ignErr

}

}

vs, err := sr.IstioClient.NetworkingV1().VirtualServices(namespace).Get(ctx, name, metav1.GetOptions{})

if err != nil {

if ignErr := client.IgnoreNotFound(err); ignErr != nil {

sr.Log.Error(ignErr, "failed to get VirtualService")

return ignErr

Copilot · 2026-01-28T12:06:38Z

+	vs, err := sr.IstioClient.NetworkingV1().VirtualServices(namespace).Get(ctx, name, metav1.GetOptions{})
+	if err != nil {
+		if client.IgnoreNotFound(err) != nil {
+			sr.Log.Error(err, "failed to get VirtualService")
+		}


Similarly to the DestinationRule fetch above, non‑NotFound errors when getting the VirtualService are logged but not returned, so transient API failures can prevent policy cleanup without a retry. Consider propagating this error as well (for example via errors.Join) so deletion can be retried by the reconciliation loop.

silenceper · 2026-01-29T02:32:52Z

+			},
+		},
+		Spec: v1alpha3.DestinationRule{
+			Host:          fmt.Sprintf("%s.%s.svc.cluster.local", name, namespace),


这个svc.cluster.local 多次用到拼接，可以弄一个封装的方法

添加prometheu指标

dove0012 added 6 commits January 27, 2026 11:31

添加istio策略下发服务

b7e0a80

修复dr策略更新、更新和删除的逻辑

abc68bd

优化代码

942a2a2

修复更新dr流程

658aff0

优化更新流程

813a73a

删除冗余代码

22f4ae5

silenceper requested a review from Copilot January 28, 2026 11:55

Copilot started reviewing on behalf of silenceper January 28, 2026 11:56 View session

Copilot AI reviewed Jan 28, 2026

View reviewed changes

完善日志信息

c259375

silenceper requested changes Jan 29, 2026

View reviewed changes

dove0012 added 10 commits January 29, 2026 16:51

修复bug

09c1fee

添加prometheu指标

删除多余代码

9a5e522

修复删除策略逻辑

c504acc

修复创建vs流程的bug

3f589ad

修复bug

c0ecb8c

添加istio策略下发helm

dccd00d

修复更新label map引用的问题

b8550ad

删除helm多余文件

2ca77a1

把configmap的值设置为变量

30c9050

删除测试代码

2ef031a

dove0012 requested a review from silenceper February 3, 2026 06:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: 添加istio策略下发服务#3827

feat: 添加istio策略下发服务#3827
dove0012 wants to merge 17 commits into
TencentBlueKing:masterfrom
dove0012:runtime-istio-policy

dove0012 commented Jan 27, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 28, 2026

Uh oh!

Copilot AI Jan 28, 2026

Uh oh!

Copilot AI Jan 28, 2026

Uh oh!

Copilot AI Jan 28, 2026

Uh oh!

Copilot AI Jan 28, 2026

Uh oh!

Copilot AI Jan 28, 2026

Uh oh!

Copilot AI Jan 28, 2026

Uh oh!

Copilot AI Jan 28, 2026

Uh oh!

Copilot AI Jan 28, 2026

Uh oh!

Copilot AI Jan 28, 2026

Uh oh!

silenceper Jan 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	// package xxx
	// Package option defines configuration structures and controller options used by the istio-policy-controller.

	// package xxx
	// istio-policy-controller is the entry point for the Istio policy controller binary.

	ctrl.Log.WithName("config").Info(fmt.Sprintf("config content: %s", string(content)))
	ctrl.Log.WithName("config").Info(fmt.Sprintf("loaded config from %s", o.ConfigPath))

Uh oh!

Conversation

dove0012 commented Jan 27, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

silenceper Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants