Skip to content

feat(redis): add proxy/flush/isolation/resize/modify commands and resource-type dispatch#117

Merged
Ali1213 merged 2 commits into
ucloud:masterfrom
ucloud-umem-qingpfang:feat/redis-commands
Jul 14, 2026
Merged

feat(redis): add proxy/flush/isolation/resize/modify commands and resource-type dispatch#117
Ali1213 merged 2 commits into
ucloud:masterfrom
ucloud-umem-qingpfang:feat/redis-commands

Conversation

@ucloud-umem-qingpfang

Copy link
Copy Markdown
Contributor

Summary

Enrich the redis product with proxy/flush/isolation/resize/modify commands,
and replace id-prefix guessing with a resource-type lookup for delete/restart.

New commands

  • modify-name, modify-password
  • flush, isolation, resize
  • list-block, list-proxy
  • create-proxy, delete-proxy, resize-proxy

Refactor

  • Introduce describeRedisMode to dispatch delete/restart by resource
    type instead of guessing from the id prefix (uredis / uhredis /
    uregionredis / udredis)
  • Add BlockRow / ProxyRow row types

Deps / testdata

  • Bump github.com/ucloud/ucloud-sdk-go to v0.22.90
  • Update cmdtree / completion golden testdata for redis

Verification

  • go build ./products/redis/... passes.

Notes

  • Depends on the same ucloud-sdk-go v0.22.90 bump as the memcache resize PR;
    the SDK version bump is intentionally included in both PRs so each is
    independently buildable. Whichever merges first will land the go.mod /
    go.sum bump; the other should rebase and drop the dependency change to
    avoid a conflict.

…ource-type dispatch

New commands:
- modify-name, modify-password
- flush, isolation, resize
- list-block, list-proxy
- create-proxy, delete-proxy, resize-proxy

Refactor:
- Introduce `describeRedisMode` to dispatch `delete`/`restart` by resource
  type instead of guessing from the id prefix (`uredis` / `uhredis` /
  `uregionredis` / `udredis`)
- Add `BlockRow` / `ProxyRow` row types

Deps / testdata:
- Bump `github.com/ucloud/ucloud-sdk-go` to `v0.22.90`
- Update `cmdtree` / `completion` golden testdata for redis

Co-Authored-By: Claude <noreply@anthropic.com>

@Episkey-G Episkey-G left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the PR. Overall the switch from id-prefix guessing to DescribeUMem-based dispatch is the right direction. Below are the High and Medium priority items I'd like to see addressed before merge (verified against the ucloud-sdk-go v0.22.90 this PR pins).

🔴 High

1. resourceTypeToMode is missing the "distributed" case — breaks distributed-redis commands

In products/redis/internal/redis/resource_type.go:

case "g4v6", "single":
	return redisModeMasterReplica
case "performance", "cluster":   // ← "distributed" is not handled
	return redisModeDistributed

Checked against the exact SDK version this PR bumps to. services/umem/models.go in v0.22.90 documents ResourceType as:

// distributed: 分布式版Redis,或者分布式Memcache;single:主备版Redis,或者单机Memcache;performance:高性能版

So a distributed redis instance reports ResourceType == "distributed". The mapping only routes "performance"/"cluster" to distributed and omits "distributed" entirely (neither "cluster" nor "g4v6" appear anywhere in the SDK enum docs). The repo's own rows.go redisTypeMap ({"single":..., "distributed":...}, used by redis list) also relies on "distributed".

Impact — for a distributed instance, describeRedisMode returns redisModeUnknown, so these commands print unknown resource type, skip and do nothing:

  • deletethis is a regression, it previously worked via the udredis prefix
  • flush, modify-name, modify-password, resize

(restart / isolation are unaffected since they intentionally target master-replica only; the proxy/block commands don't go through this mapping.)

Suggested fix:

case "single", "g4v6":
	return redisModeMasterReplica
case "distributed", "performance", "cluster":
	return redisModeDistributed

Worth confirming the actual value with a live DescribeUMem against a distributed instance, but "distributed" must be handled.

2. rows.go is not gofmt-clean

gofmt -l products/redis/internal/redis/rows.go reports the file — the BlockRow fields have an extra alignment space. gofmt -w fixes it:

type BlockRow struct {
	BlockID    string
	BlockName  string
	BlockVip   string
	BlockPort  int
	BlockType  string
	BlockState string
	BlockSize  int
	UsedSize   int
	SlotBegin  int
	SlotEnd    int
	ReadWeight int
}

If CI runs gofmt/golangci-lint this will fail the build.

🟡 Medium

3. Two sources of truth for the type mapping

redisTypeMap (rows.go) and resourceTypeToMode (resource_type.go) now each maintain their own key set, and they already disagree — which is the root cause of #1. Consider consolidating into a single shared mapping so they can't drift again.

4. Extra DescribeUMem call + double client construction per id

In the command loops, each iteration calls describeRedisMode (which builds a umem client via cli.NewServiceClient and makes a DescribeUMem call), and then the operation helper (deleteMasterReplica, flushDistributed, …) builds another client. For N ids that's 2N client constructions + N extra round-trips. Building the client once outside the loop and reusing it would be cheaper.

5. resize distributed --block-id handling

When --block-id is missing for a distributed instance, the command prints a message and continues, but the skipped item isn't recorded in the results and doesn't affect the exit code — so in a batch invocation a silently-skipped instance is easy to miss. Consider surfacing it in the result set / exit status.


Nice touches worth calling out: the restart.go change to make([]request.Common, 0, len(idNames)) + append is a necessary correctness fix (the new continue would otherwise leave nil entries), and the cmdtree/completion golden files are consistent with the 10 new commands.

@ucloud-umem-qingpfang

Copy link
Copy Markdown
Contributor Author

目前对于distributed类型的分布式Redis已经全地域完全下线。不会出现该类型返回

@ucloud-umem-qingpfang

Copy link
Copy Markdown
Contributor Author

感谢您提交的 PR。总体来说,从猜测 ID 断开切换到基于 ID 的隔壁的调度DescribeUMem是正确的方向。以下是我希望在合并之前解决的优先级和ucloud-sdk-go v0.22.90优先级问题(已对照此PR中的标记进行验证)。

🔴高

1.resourceTypeToMode缺少大小写"distributed"——导致distributed-redis命令错误

products/redis/internal/redis/resource_type.go

目前对于”distrubted“类型的分布式Redis已经完全下线,不会出现该类型

- list.go: use resourceTypeToMode instead of the redisTypeMap lookup
  (single source of truth for resource-type → mode mapping)
- rows.go: drop the now-unused redisTypeMap; gofmt BlockRow/ProxyRow
- product.yaml: update owner

Co-Authored-By: Claude <noreply@anthropic.com>
@Ali1213 Ali1213 merged commit ccc4041 into ucloud:master Jul 14, 2026
12 of 13 checks passed
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.

3 participants