Skip to content

Refactor: Replace sync.map with cache pkg for email routine. - #259

Open
kash2104 wants to merge 3 commits into
kubevela:mainfrom
kash2104:issue-253
Open

kash2104 wants to merge 3 commits into
kubevela:mainfrom
kash2104:issue-253

Conversation

@kash2104

@kash2104 kash2104 commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Description of your changes

This PR replaces the sync.map with the cache.Cache[string] that has been introduced in kubevela/pkg PR to use the centralized package instead of declaring separate sync.map for email routine cache.

Fixes #253

I have:

  • Read and followed KubeVela's contribution process.
  • Related Docs updated properly. In a new feature or configuration option, an update to the documentation is necessary.
  • Run make reviewable to ensure this PR is ready for review.
  • Added backport release-x.y labels to auto-backport this PR if necessary.

How has this code been tested

Since this PR for the email routine cache hasn't been merged yet, used workspaces to test this feature and this email routine is being used in the kubevela repo.

  1. Create a go workspace
go work init
go work use <workspace-folder-name>
go work use <kubevela-folder-name>
  1. Build the project to ensure the workspace dependency is used.
go build ./...

Note: Avoid running go mod tidy in any repo, as it updates the module graph and throws error saying that package cache doesn't exist in kubevela/pkg

  1. Update the sync-crds target in the KubeVela dependency.mk to disable the workspace when downloading the pinned module version for CRDs.
sync-crds: ## Copy CRD from pinned module version in go.mod
	@moddir=$$(GOWORK=off go mod download -json $(PKG_MODULE) 2>/dev/null | grep '"Dir"' | cut -d'"' -f4); \
	mkdir -p config/crd/base; \
	for file in $(COMMON_CRD_FILES); do \
		src="$$moddir/crds/$$file"; \
		cp -f "$$src" "config/crd/base/"; \
	done

This ensures that the CRDs are copied from the module version pinned in go.mod rather than from the local workspace checkout.

  1. Build and install the updated KubeVela controller.
  2. Deploy a sample application:
vela up -f https://kubevela.io/example/applications/first-app.yaml
  1. Verify that the application is successfully reconciled and deployed, confirming that the local workspace version of github.com/kubevela/workflow and github.com/kubevela/kubevela is being used and that the changes work as expected.

Special notes for your reviewer

This would also require changes in the github.com/kubevela/kubevela repo since we need to initialize the cache and register similar to what has been done with rateLimiter and stepStatusCache.


Summary by cubic

Swaps the per-provider sync.Map tracking email send status for cache.Cache[string] from kubevela/pkg, and guards cache reads with a mutex so concurrent cold invocations can't both dial and send duplicate emails. Completed sends now delete their cache entry, so a later invocation for the same session ID starts a fresh send instead of hanging on the previous result.

Migration

  • Consumers must call the new InitEmailCache(ctx) during startup; the kubevela repo needs a matching change.
  • Requires the kubevela/pkg release that introduces the cache.Cache package.

Tests

  • Adds coverage for exactly-once sends, reconciles arriving during an in-flight send, and concurrent cold invocations never sending duplicates.

Written for commit f3b563d. Summary will update on new commits.

View guided diff

@codecov

codecov Bot commented Sep 21, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.33333% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.44%. Comparing base (d7db9c4) to head (f3b563d).
⚠️ Report is 30 commits behind head on main.

Files with missing lines Patch % Lines
pkg/providers/email/email.go 83.33% 2 Missing ⚠️
pkg/providers/legacy/email/email.go 83.33% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main     #259       +/-   ##
===========================================
+ Coverage   62.49%   75.44%   +12.95%     
===========================================
  Files          62       73       +11     
  Lines        4415     5169      +754     
===========================================
+ Hits         2759     3900     +1141     
+ Misses       1324     1269       -55     
+ Partials      332        0      -332     
Flag Coverage Δ
e2etests 30.68% <0.00%> (?)
unit-test 68.09% <83.33%> (+5.60%) ⬆️

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

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 4 files

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread pkg/providers/email/email.go Resolved
Comment thread pkg/providers/email/email_test.go Resolved
Comment thread pkg/providers/email/email_test.go Resolved
@kash2104 kash2104 changed the title refactor: Replace sync.map with cache pkg for email routine. Refactor: Replace sync.map with cache pkg for email routine. Sep 21, 2026
Signed-off-by: kash2104 <kparikh1104@gmail.com>
Signed-off-by: kash2104 <kparikh1104@gmail.com>
chore: Run linter and formatting.
Signed-off-by: kash2104 <kparikh1104@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor: Migrate email routine sync.Map to pkg/cache

1 participant