refactor: dedup k8s config onto app.K8sConfig and remove redundant manager config section by pingsutw · Pull Request #7526 · flyteorg/flyte

pingsutw · 2026-06-16T05:21:09Z

Tracking issue

N/A — internal config cleanup.

Why are the changes needed?

Two related sources of config duplication in the v2 services:

manager, actions, and secret each defined their own near-duplicate
KubernetesConfig struct for their kubernetes config section, even though
flytestdlib/app.K8sConfig (the input to InitKubernetesClient) already
defines that exact shape.
The manager package then carried a whole manager config section
(server, kubernetes, executor.healthProbePort) that just duplicated
config already owned by the runs and actions services. The unified binary
listens on the run service's port and shares the actions service's k8s client,
so a separate manager section was redundant.

What changes were proposed in this pull request?

Promote app.K8sConfig to the shared, config-loadable type:

flytestdlib/app/k8s.go — add json/pflag tags to K8sConfig. It also
carries ClusterName (used by the secret service for status reporting), so the
secret section keeps its previous secret.kubernetes.clusterName shape.
manager / actions / secret — use app.K8sConfig directly as the
kubernetes field and delete their local KubernetesConfig; main.go passes
cfg.Kubernetes straight to InitKubernetesClient (no convert step).

Remove the manager config section entirely:

Delete manager/config/ (config.go + generated config_flags).
manager/cmd/main.go now reuses runs.Server for the HTTP listen address and
actions.Kubernetes for the shared k8s client, instead of a manager-specific
section.
The dead Executor.HealthProbePort field (defined but never read) is dropped.
The manager's previous k8s client tuning (qps=1000, burst=2000,
timeout=30s) is preserved by setting it on the actions kubernetes
defaults, so the shared client behaves identically.
manager/config.yaml updated to match: server under runs.server,
kubernetes under actions.kubernetes.

Schema change

The manager config section is removed; the unified binary reads runs.server
and actions.kubernetes instead. The secret section is unchanged
(secret.kubernetes.* including clusterName).

How was this patch tested?

go build ./..., go vet, gofmt clean.
Regenerated config_flags for the affected sections; all flags present
(kubernetes.{namespace,kubeconfig,qps,burst,timeout,clusterName});
go test ./secret/... ./actions/config/... ./manager/... passes.
helm template renders the secret section with kubernetes.* (including
clusterName) for the client config.

Labels

changed

Check all the applicable boxes

I updated the documentation accordingly.
All new and existing tests passed.
All commits are signed-off.

Copilot

Pull request overview

This PR refactors service configs to remove duplicate per-service KubernetesConfig structs by promoting flytestdlib/app.K8sConfig into the shared, config-loadable type used across manager, actions, and secret.

Changes:

Added json/pflag tags to flytestdlib/app.K8sConfig so it can be loaded from config and used for flag generation.
Updated manager and actions to use app.K8sConfig directly and pass it straight into InitKubernetesClient.
Updated secret to embed app.K8sConfig (json:",squash") alongside ClusterName, and added a decode test verifying squash behavior.

Reviewed changes

Copilot reviewed 8 out of 14 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
`flytestdlib/app/k8s.go`	Adds `json`/`pflag` tags to `K8sConfig` so it can serve as the shared config section type.
`manager/config/config.go`	Replaces local `KubernetesConfig` with `app.K8sConfig` in manager config.
`manager/config/config_flags.go`	Aligns generated flags/help text with `app.K8sConfig` tags.
`manager/config/config_flags_test.go`	Updates flag tests for the manager config changes.
`manager/cmd/main.go`	Simplifies k8s client init by passing `cfg.Kubernetes` directly.
`actions/config/config.go`	Replaces local `KubernetesConfig` with `app.K8sConfig` in actions config.
`actions/config/config_flags.go`	Exposes additional k8s flags now available via `app.K8sConfig`.
`actions/config/config_flags_test.go`	Updates/adds flag tests for the actions config changes.
`actions/cmd/main.go`	Simplifies k8s client init by passing `cfg.Kubernetes` directly.
`secret/config/config.go`	Embeds `app.K8sConfig` with `json:",squash"` and keeps `ClusterName` as secret-specific.
`secret/config/config_flags.go`	Regenerates flags for secret config (notably emits `kubernetes.K8sConfig.*` paths).
`secret/config/config_flags_test.go`	Updates secret flag tests to match the regenerated flag names.
`secret/config/squash_decode_test.go`	Adds a test proving `json:",squash"` decoding works for the embedded `app.K8sConfig`.
`secret/cmd/main.go`	Simplifies k8s client init to pass the embedded config directly.

Files not reviewed (6)

actions/config/config_flags.go: Generated file
actions/config/config_flags_test.go: Generated file
manager/config/config_flags.go: Generated file
manager/config/config_flags_test.go: Generated file
secret/config/config_flags.go: Generated file
secret/config/config_flags_test.go: Generated file

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

manager, actions, and secret each defined their own KubernetesConfig for the `kubernetes` config section. Promote flytestdlib/app.K8sConfig (the InitKubernetesClient input, already imported by every service) to the canonical config-loadable type: - app.K8sConfig: add json/pflag tags so it can back a config section. - manager, actions, secret: use app.K8sConfig directly as the `kubernetes` field; main.go passes cfg.Kubernetes straight to InitKubernetesClient (drops the convert step). actions gains qps/burst/timeout (InitKubernetesClient guards zero values, so no behavior change). - secret: its cluster-identity ClusterName is a service setting, not a k8s-client one, so it moves from secret.kubernetes.clusterName to a top-level secret.clusterName. All three then reuse app.K8sConfig with full pflag coverage. Schema change: secret.kubernetes.clusterName -> secret.clusterName (chart values.yaml updated to match). Signed-off-by: Kevin Su <pingsutw@gmail.com> Signed-off-by: Kevin Su <pingsutw@apache.org>

…etes config The unified binary's manager package duplicated config (server listen address, shared k8s client settings) that already lives in the runs and actions services. Remove the manager config section entirely and rewire manager/cmd to reuse runs.Server for the HTTP listen address and actions.Kubernetes for the shared k8s client. The dead Executor.HealthProbePort field is dropped (never read). The manager's previous k8s client tuning (QPS=1000, Burst=2000, Timeout=30s) is preserved on the actions Kubernetes config so the shared client behaves identically. Signed-off-by: Kevin Su <pingsutw@apache.org>

Copilot

Pull request overview

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

Files not reviewed (4)

actions/config/config_flags.go: Generated file
actions/config/config_flags_test.go: Generated file
secret/config/config_flags.go: Generated file
secret/config/config_flags_test.go: Generated file

Move server config under runs.server and the shared k8s client config under actions.kubernetes, matching the removal of the manager config section. Drop the dead executor.healthProbePort and fix the webhook namespace comment to reference actions.kubernetes.namespace. Signed-off-by: Kevin Su <pingsutw@apache.org>

Signed-off-by: Kevin Su <pingsutw@apache.org>

Copilot

Pull request overview

Copilot reviewed 13 out of 17 changed files in this pull request and generated 1 comment.

Files not reviewed (4)

actions/config/config_flags.go: Generated file
actions/config/config_flags_test.go: Generated file
secret/config/config_flags.go: Generated file
secret/config/config_flags_test.go: Generated file

+	Kubernetes: app.K8sConfig{
 		Namespace: "flyte",
+		QPS:       1000,
+		Burst:     2000,
+		Timeout:   "30s",
 	},


Signed-off-by: Kevin Su <pingsutw@apache.org>

Restore the previous schema where the secret service's cluster name lives under the kubernetes section (secret.kubernetes.clusterName) instead of a top-level secret.clusterName. Move ClusterName onto app.K8sConfig and read it via cfg.Kubernetes.ClusterName. Regenerate config_flags and update the chart values default accordingly. Signed-off-by: Kevin Su <pingsutw@apache.org>

Signed-off-by: Kevin Su <pingsutw@apache.org>

Copilot

Pull request overview

Copilot reviewed 12 out of 16 changed files in this pull request and generated 2 comments.

Files not reviewed (4)

actions/config/config_flags.go: Generated file
actions/config/config_flags_test.go: Generated file
secret/config/config_flags.go: Generated file
secret/config/config_flags_test.go: Generated file

    secret:
+      clusterName: flyte-devbox
      kubernetes:
        burst: 200
-        clusterName: flyte-devbox
        kubeconfig: ""


Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Kevin Su <pingsutw@gmail.com>

Copilot

Pull request overview

Copilot reviewed 12 out of 16 changed files in this pull request and generated 3 comments.

Files not reviewed (4)

actions/config/config_flags.go: Generated file
actions/config/config_flags_test.go: Generated file
secret/config/config_flags.go: Generated file
secret/config/config_flags_test.go: Generated file

Signed-off-by: Kevin Su <pingsutw@apache.org>

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Kevin Su <pingsutw@gmail.com>

Copilot

Pull request overview

Copilot reviewed 12 out of 16 changed files in this pull request and generated 2 comments.

Files not reviewed (4)

actions/config/config_flags.go: Generated file
actions/config/config_flags_test.go: Generated file
secret/config/config_flags.go: Generated file
secret/config/config_flags_test.go: Generated file

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Kevin Su <pingsutw@gmail.com>

Copilot

Pull request overview

Copilot reviewed 12 out of 16 changed files in this pull request and generated 3 comments.

Files not reviewed (4)

actions/config/config_flags.go: Generated file
actions/config/config_flags_test.go: Generated file
secret/config/config_flags.go: Generated file
secret/config/config_flags_test.go: Generated file

Copilot AI review requested due to automatic review settings June 16, 2026 05:21

github-actions Bot added the flyte2 label Jun 16, 2026

Copilot started reviewing on behalf of pingsutw June 16, 2026 05:21 View session

Copilot AI reviewed Jun 16, 2026

View reviewed changes

Comment thread secret/config/config.go Outdated

Comment thread secret/config/squash_decode_test.go Outdated

pingsutw force-pushed the manager-dedup-kubernetes-config branch from d343072 to c279fe3 Compare June 16, 2026 05:27

Copilot AI review requested due to automatic review settings June 16, 2026 17:29

Copilot started reviewing on behalf of pingsutw June 16, 2026 17:30 View session

pingsutw changed the title ~~refactor: dedup per-service KubernetesConfig onto shared app.K8sConfig~~ refactor: dedup k8s config onto app.K8sConfig and remove redundant manager config section Jun 16, 2026

Copilot AI reviewed Jun 16, 2026

View reviewed changes

Comment thread manager/cmd/main.go

Comment thread actions/config/config.go

pingsutw added 2 commits June 16, 2026 11:40

remove manager config

2a1e877

Signed-off-by: Kevin Su <pingsutw@apache.org>

Copilot AI review requested due to automatic review settings June 16, 2026 18:51

Copilot started reviewing on behalf of pingsutw June 16, 2026 18:51 View session

Copilot AI reviewed Jun 16, 2026

View reviewed changes

Comment thread actions/config/config.go

Comment on lines +17 to 22

Kubernetes: app.K8sConfig{

Namespace: "flyte",

QPS: 1000,

Burst: 2000,

Timeout: "30s",

},

pingsutw self-assigned this Jun 17, 2026

pingsutw added this to the V2 GA milestone Jun 17, 2026

pingsutw added 2 commits June 16, 2026 17:35

nit

47c34fb

Signed-off-by: Kevin Su <pingsutw@apache.org>

Copilot AI review requested due to automatic review settings June 17, 2026 00:38

Copilot started reviewing on behalf of pingsutw June 17, 2026 00:39 View session

nit

1d6186c

Signed-off-by: Kevin Su <pingsutw@apache.org>

Copilot AI reviewed Jun 17, 2026

View reviewed changes

Comment thread docker/devbox-bundled/manifests/complete.yaml

Comment on lines 7725 to 7729

secret:

clusterName: flyte-devbox

kubernetes:

burst: 200

clusterName: flyte-devbox

kubeconfig: ""

Comment thread flytestdlib/app/k8s.go Outdated

Potential fix for pull request finding

ac2529a

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Kevin Su <pingsutw@gmail.com>

Copilot AI review requested due to automatic review settings June 17, 2026 01:02

Copilot started reviewing on behalf of pingsutw June 17, 2026 01:03 View session

Copilot AI reviewed Jun 17, 2026

View reviewed changes

Comment thread docker/devbox-bundled/manifests/complete.yaml

Comment thread actions/config/config_flags.go Outdated

Comment thread secret/config/config_flags.go Outdated

pingsutw and others added 2 commits June 16, 2026 18:08

nit

2327e4c

Signed-off-by: Kevin Su <pingsutw@apache.org>

Potential fix for pull request finding

b99a95f

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Kevin Su <pingsutw@gmail.com>

Copilot AI review requested due to automatic review settings June 17, 2026 01:08

Copilot started reviewing on behalf of pingsutw June 17, 2026 01:08 View session

Copilot AI reviewed Jun 17, 2026

View reviewed changes

Comment thread actions/config/config_flags.go Outdated

Comment thread actions/config/config.go

Potential fix for pull request finding

80d5caf

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Kevin Su <pingsutw@gmail.com>

Copilot AI review requested due to automatic review settings June 17, 2026 01:12

Copilot started reviewing on behalf of pingsutw June 17, 2026 01:12 View session

Copilot AI reviewed Jun 17, 2026

View reviewed changes

Comment thread manager/config.yaml

Comment thread docker/devbox-bundled/manifests/complete.yaml

Comment thread charts/flyte-binary/values.yaml

machichima approved these changes Jun 17, 2026

View reviewed changes

pingsutw merged commit e38343e into main Jun 17, 2026
22 checks passed

pingsutw deleted the manager-dedup-kubernetes-config branch June 17, 2026 04:54

Conversation

pingsutw commented Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Tracking issue

Why are the changes needed?

What changes were proposed in this pull request?

Schema change

How was this patch tested?

Labels

Check all the applicable boxes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pingsutw commented Jun 16, 2026 •

edited

Loading