feat(syncer): add resource syncer by liubog2008 · Pull Request #6595 · pingcap/tidb-operator

liubog2008 · 2025-12-09T09:30:19Z

Resource syncer is designed to sync secrets or configmaps into the pod.

Usage Example

apiVersion: apps/v1
kind: Deployment
metadata:
  labels:
    app: test
  name: test
  namespace: default
spec:
  replicas: 1
  selector:
    matchLabels:
      app: test
  template:
    metadata:
      labels:
        app: test
    spec:
      containers:
      - command:
        - /bin/sh
        - -c
        - ls -la /data/secrets/; sleep 1000
        image: busybox
        imagePullPolicy: Always
        name: busybox
        volumeMounts:
        - mountPath: /data
          name: secret
          readOnly: true
          recursiveReadOnly: IfPossible
      initContainers:
      - command:
        - /resource-syncer
        - -d=/data
        - -n=default
        - -l=pingcap.com/managed-by=tidb-operator,test=test
        image: pingcap/tidb-operator-resource-syncer:latest
        imagePullPolicy: IfNotPresent
        name: syncer
        resources: {}
        restartPolicy: Always
        startupProbe:
          failureThreshold: 10
          httpGet:
            path: /healthz
            port: 8081
            scheme: HTTP
          periodSeconds: 10
          successThreshold: 1
          timeoutSeconds: 1
        volumeMounts:
        - mountPath: /data
          name: secret
      restartPolicy: Always
      volumes:
      - emptyDir: {}
        name: secret

Signed-off-by: liubo02 <liubo02@pingcap.com>

fgksgf · 2025-12-09T09:38:55Z

cmd/resource-syncer/main.go

+	"sigs.k8s.io/controller-runtime/pkg/cache"
+	"sigs.k8s.io/controller-runtime/pkg/healthz"
+	"sigs.k8s.io/controller-runtime/pkg/log/zap"
+


Suggested change

fgksgf · 2025-12-09T09:40:23Z

cmd/resource-syncer/main.go

+	if err := run(&cfg); err != nil {
+		setupLog.Error(err, "unable to new manager")
+		os.Exit(1)
+


Suggested change

Copilot

Pull request overview

This PR introduces a new resource syncer component designed to sync Kubernetes secrets into pod filesystems. The syncer runs as an init container that watches secrets with specific labels and synchronizes their data to a mounted volume.

Key Changes:

Adds a new resource-syncer binary with a controller that watches and syncs secrets to the filesystem
Implements health and readiness checks that report ready once initial sync completes
Updates build system (Makefile, Docker, scripts) to include the new resource-syncer component

Reviewed changes

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

Show a summary per file

File	Description
pkg/controllers/resourcesyncer/secret/controller.go	Core reconciler that watches secrets and syncs their data to filesystem directories
pkg/controllers/resourcesyncer/secret/controller_test.go	Unit tests for the secret syncer controller
cmd/resource-syncer/main.go	Main entry point with CLI flags and manager setup
cmd/resource-syncer/README.md	Documentation for the resource syncer component
image/Dockerfile	Adds Docker image build stage for resource-syncer
hack/lib/build.sh	Adds resource-syncer to build targets
hack/lib/image.sh	Configures image prefix for resource-syncer
Makefile	Adds resource-syncer to the list of built commands
go.mod	Adds afero filesystem abstraction dependency
go.sum	Checksums for afero dependency

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

Copilot · 2025-12-09T09:56:14Z

pkg/controllers/resourcesyncer/secret/controller_test.go

+		if fileData.isDir {
+			err := f.MkdirAll(path, 0o755)
+			assert.NoError(t, err)
+		}
+
+		dir := filepath.Dir(path)
+		err := f.MkdirAll(dir, 0o755)
+		assert.NoError(t, err)
+		err = afero.WriteFile(f, path, fileData.data, 0o644)
+		assert.NoError(t, err)


The setupFs function has a logic error. For directory entries (when fileData.isDir is true), it creates the directory but then continues to write it as a file on line 324. This will fail because you cannot write file data to a directory path. Either skip the file write for directories with a continue statement after line 318, or restructure the logic to handle directories separately.

Copilot · 2025-12-09T09:56:15Z

pkg/controllers/resourcesyncer/secret/controller_test.go

+		c := &cases[i]
+		t.Run(c.desc, func(tt *testing.T) {
+			fc := client.NewFakeClient(c.objs...)
+			afero.NewMemMapFs()


Unused variable: afero.NewMemMapFs() is called but the result is not used. This line should be removed as setupFs creates and returns the filesystem.

Suggested change

afero.NewMemMapFs()

Copilot · 2025-12-09T09:56:15Z

pkg/controllers/resourcesyncer/secret/controller_test.go

+			r := Reconciler{
+				Client:    fc,
+				Labels:    c.labels,
+				BaseDirFS: dirFs,


The Reconciler is missing initialization of the cache field. In the test setup, cache is not initialized, which will cause a nil map panic when the reconciler tries to access r.cache[s.Name] on line 180 of the controller. Add cache: map[string]string{} to the Reconciler initialization.

Suggested change

BaseDirFS: dirFs,

BaseDirFS: dirFs,

cache: map[string]string{},

Copilot · 2025-12-09T09:56:15Z

cmd/resource-syncer/README.md

@@ -0,0 +1,3 @@
+# Resource Syncer
+
+This binary is designed for syncing resources(secrets, configmap, etc...) of kubernetes into pods


The README description mentions "configmap" but the implementation only handles secrets. Either update the description to be accurate ("This binary is designed for syncing secrets into pods") or add a note that configmap support is planned for the future.

Suggested change

This binary is designed for syncing resources(secrets, configmap, etc...) of kubernetes into pods

This binary is designed for syncing secrets of Kubernetes into pods.

> Note: Support for syncing configmaps is planned for a future release.

Copilot · 2025-12-09T09:56:16Z

pkg/controllers/resourcesyncer/secret/controller.go

+					client.InNamespace(r.Namespace),
+					client.MatchingLabels(r.Labels),
+				); err != nil {
+					logger.Error(err, "cannot list secrets", "namepsace", r.Namespace, "labels", r.Labels)


Typo in log message: "namepsace" should be "namespace".

Suggested change

logger.Error(err, "cannot list secrets", "namepsace", r.Namespace, "labels", r.Labels)

logger.Error(err, "cannot list secrets", "namespace", r.Namespace, "labels", r.Labels)

Copilot · 2025-12-09T09:56:17Z

pkg/controllers/resourcesyncer/secret/controller.go

+			failed := true
+			for range maxRetryTimes {
+				// has synced, no need to list secrets again
+				if r.hasSynced.Load() {
+					return
+				}
+
+				nctx, cancel := context.WithTimeout(ctx, defaultTimeout)
+				defer cancel()
+
+				if err := r.Client.List(
+					nctx,
+					&sl,
+					client.InNamespace(r.Namespace),
+					client.MatchingLabels(r.Labels),
+				); err != nil {
+					logger.Error(err, "cannot list secrets", "namepsace", r.Namespace, "labels", r.Labels)
+				} else {
+					failed = false
+					break
+				}
+			}
+
+			if !failed && len(sl.Items) == 0 {
+				if r.hasSynced.CompareAndSwap(false, true) {
+					logger.Info("no secrets, hasSynced set to true")
+				}
+
+				return
+			}
+		}


The variable failed is set but the retry logic doesn't use it to handle the failure case. If all retries fail, the goroutine continues to check len(sl.Items) == 0 with potentially stale data from the last failed attempt. Consider logging an error or handling the case when all retries are exhausted.

Copilot · 2025-12-09T09:56:17Z

pkg/controllers/resourcesyncer/secret/controller.go

+			errs = append(errs, err)
+		}
+		files[name] = struct{}{}
+	}


The resourceVersionFile constant should be added to the keep map to prevent it from being cleaned up as an unknown file. Currently, when cleaning up the secret directory, the resourceVersion file is not in the files map (only actual secret data keys are), so it could be incorrectly removed.

Suggested change

}

}

// Ensure resourceVersionFile is not deleted during cleanup

files[resourceVersionFile] = struct{}{}

Copilot · 2025-12-09T09:56:17Z

cmd/resource-syncer/main.go

+	setupLog.Info("current config", "config", &cfg)
+
+	if err := run(&cfg); err != nil {
+		setupLog.Error(err, "unable to new manager")


The error message is misleading. It says "unable to new manager" but the run function can fail for multiple reasons (manager creation, directory setup, controller setup, health checks, or starting the manager). Use a more generic message like "unable to run resource syncer" or pass through the actual error from run().

Suggested change

setupLog.Error(err, "unable to new manager")

setupLog.Error(err, "unable to run resource syncer")

Copilot · 2025-12-09T09:56:18Z

pkg/controllers/resourcesyncer/secret/controller.go

+	Labels    map[string]string
+	BaseDirFS afero.Fs
+
+	// secret name to it's resourceVersion


Typo in comment: "it's" should be "its" (possessive form doesn't use an apostrophe).

Suggested change

// secret name to it's resourceVersion

// secret name to its resourceVersion

Copilot · 2025-12-09T09:56:18Z

pkg/controllers/resourcesyncer/secret/controller_test.go

+					fake.ResourceVersion[corev1.Secret]("test"),
+					fake.Label[corev1.Secret]("test", "test"),
+					// change data to test whether update is actually skipped
+					// not really happended in real world


Typo in comment: "happended" should be "happened".

Suggested change

// not really happended in real world

// not really happened in real world

Signed-off-by: liubo02 <liubo02@pingcap.com>

codecov-commenter · 2025-12-09T11:12:39Z

Codecov Report

❌ Patch coverage is 52.53165% with 75 lines in your changes missing coverage. Please review.
✅ Project coverage is 41.47%. Comparing base (37be4e9) to head (b3367e4).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6595      +/-   ##
==========================================
+ Coverage   41.38%   41.47%   +0.08%     
==========================================
  Files         348      349       +1     
  Lines       19885    20043     +158     
==========================================
+ Hits         8230     8313      +83     
- Misses      11655    11730      +75

Flag	Coverage Δ
unittest	`41.47% <52.53%> (+0.08%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: liubo02 <liubo02@pingcap.com>

liubog2008 · 2025-12-10T08:37:38Z

/test pull-e2e

liubog2008 · 2025-12-11T02:15:07Z

/test pull-e2e

ti-chi-bot · 2025-12-12T02:12:29Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: csuzhangxc

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [csuzhangxc]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ti-chi-bot · 2025-12-12T02:12:29Z

[LGTM Timeline notifier]

Timeline:

2025-12-12 02:12:29.002580447 +0000 UTC m=+1179893.816358020: ☑️ agreed by csuzhangxc.

ti-chi-bot · 2025-12-12T04:23:31Z

@liubog2008: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-e2e	`b3367e4`	link	unknown	`/test pull-e2e`

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

feat(syncer): add resource syncer

e7120d9

Signed-off-by: liubo02 <liubo02@pingcap.com>

ti-chi-bot bot requested a review from shonge December 9, 2025 09:30

github-actions bot added the v2 for operator v2 label Dec 9, 2025

ti-chi-bot bot added the size/XXL label Dec 9, 2025

fix verify

e8b8c33

Signed-off-by: liubo02 <liubo02@pingcap.com>

fgksgf reviewed Dec 9, 2025

View reviewed changes

fgksgf requested a review from Copilot December 9, 2025 09:44

Copilot started reviewing on behalf of fgksgf December 9, 2025 09:45 View session

Copilot AI reviewed Dec 9, 2025

View reviewed changes

fix lint

d72d881

Signed-off-by: liubo02 <liubo02@pingcap.com>

address comments

b3367e4

Signed-off-by: liubo02 <liubo02@pingcap.com>

csuzhangxc approved these changes Dec 12, 2025

View reviewed changes

ti-chi-bot bot added the lgtm label Dec 12, 2025

ti-chi-bot bot added the approved label Dec 12, 2025

ti-chi-bot bot merged commit 2dc570f into pingcap:main Dec 12, 2025
9 checks passed

	BaseDirFS: dirFs,
	BaseDirFS: dirFs,
	cache: map[string]string{},

		@@ -0,0 +1,3 @@
		# Resource Syncer

		This binary is designed for syncing resources(secrets, configmap, etc...) of kubernetes into pods

-This binary is designed for syncing resources(secrets, configmap, etc...) of kubernetes into pods
+This binary is designed for syncing secrets of Kubernetes into pods.
+> Note: Support for syncing configmaps is planned for a future release.

	logger.Error(err, "cannot list secrets", "namepsace", r.Namespace, "labels", r.Labels)
	logger.Error(err, "cannot list secrets", "namespace", r.Namespace, "labels", r.Labels)

-	}
+	}
+	// Ensure resourceVersionFile is not deleted during cleanup
+	files[resourceVersionFile] = struct{}{}

	setupLog.Error(err, "unable to new manager")
	setupLog.Error(err, "unable to run resource syncer")

	// secret name to it's resourceVersion
	// secret name to its resourceVersion

	// not really happended in real world
	// not really happened in real world

Conversation

liubog2008 commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Usage Example

Uh oh!

fgksgf Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

fgksgf Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

liubog2008 commented Dec 10, 2025

Uh oh!

liubog2008 commented Dec 11, 2025

Uh oh!

ti-chi-bot bot commented Dec 12, 2025

Uh oh!

ti-chi-bot bot commented Dec 12, 2025

[LGTM Timeline notifier]

Uh oh!

Uh oh!

ti-chi-bot bot commented Dec 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

liubog2008 commented Dec 9, 2025 •

edited

Loading

codecov-commenter commented Dec 9, 2025 •

edited

Loading