ci: auto-sync releases to Gitee repository #1616
ci: auto-sync releases to Gitee repository #1616imshubham22apr-gif wants to merge 0 commit intokmesh-net:mainfrom
Conversation
kmesh-bot
requested review from
LiZhenCheng9527,
Okabe-Rintarou-0,
YaoZengzeng and
hzxuzhonghu
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces significant refactoring and testing improvements to the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Code refactored, Tests now stand, strong and true, Bugs flee in fear. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request primarily focuses on improving error handling, modularity, and testability across several ctl commands, and expanding debug capabilities. Specifically, HTTP request logic in ctl/authz, ctl/dump, and ctl/monitoring was refactored into dedicated functions that return errors, enhancing error propagation and testability. The ctl/log command was updated to use cobra.Command.RunE for better error handling and to print output to cmd.OutOrStdout(). New test files were introduced for authz, common, dump, log, monitoring, secret, and waypoint commands. The eBPF log reader was moved to a Linux-specific build file, with a stub for other platforms. Furthermore, new debug endpoints were added to pkg/status/status_server.go for dumping K8s services and authorization policies, accompanied by new tests. A review comment highlighted an improvement opportunity in ctl/dump/dump.go to replace os.Exit(1) with returning an error for more graceful error handling and improved modularity.
ctl/dump/dump.go
Outdated
| log.Error(err) | ||
| os.Exit(1) |
There was a problem hiding this comment.
The use of os.Exit(1) here prevents the calling function from handling the error gracefully. It's generally better to return the error so that the caller can decide how to manage the application's exit or recovery. This improves the modularity and testability of the RunDump function.
| log.Error(err) | |
| os.Exit(1) | |
| log.Error(err) | |
| return err |
There was a problem hiding this comment.
Pull request overview
This pull request implements several enhancements to the Kmesh project:
-
Status Server Endpoints: Adds two new debug endpoints (
/debug/config_dump/servicesand/debug/config_dump/policies) for dumping K8s services and security policies known to Kmesh, with a reusable genericdumpJSONhelper function. -
Logger Platform-Specific Refactoring: Separates the Linux-specific eBPF logging functionality from the generic logger into platform-specific files (
logger_linux.gofor Linux with eBPF support, andlogger_stub.gofor non-Linux platforms). -
CLI Module Refactoring: Improves error handling and testability in multiple CLI modules (
ctl/log,ctl/dump,ctl/authz,ctl/monitoring) by:- Converting functions to return errors instead of calling exit or logging directly
- Deferring Kubernetes client initialization to when needed (in
ctl/secret) - Extracting helper functions for better testability
-
Test Coverage: Adds comprehensive unit tests for newly refactored and extracted functions across multiple CLI modules and GitHub workflow automation.
-
GitHub Workflow: Adds a new workflow to sync releases to Gitee mirror repository.
Changes:
- Adds debug endpoints for services and policies dumps in status server
- Refactors logger to use platform-specific build tags
- Improves CLI error handling and testability with better function signatures
- Defers expensive initialization until needed (Kube client in secret module)
- Adds comprehensive test coverage for CLI modules
- Adds GitHub workflow for Gitee synchronization
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/status/status_server.go | Adds new services and policies config dump endpoints with generic dumpJSON helper |
| pkg/status/status_server_test.go | Adds tests for new config dump endpoints |
| pkg/logger/logger.go | Removes platform-specific code, keeps generic logger functionality |
| pkg/logger/logger_linux.go | New file with Linux-specific eBPF logging functionality |
| pkg/logger/logger_stub.go | New file with stub functions for non-Linux platforms |
| ctl/log/log.go | Refactors functions to return errors and use output writers |
| ctl/log/log_test.go | Adds test coverage for log module functions |
| ctl/monitoring/monitoring.go | Extracts SetObservability helper and improves error handling |
| ctl/monitoring/monitoring_test.go | Adds test coverage for monitoring functions |
| ctl/authz/authz.go | Extracts SetAuthz and GetAuthzStatus helpers |
| ctl/authz/authz_test.go | Adds test coverage for authz functions |
| ctl/dump/dump.go | Extracts GetConfigDump helper function |
| ctl/dump/dump_test.go | Adds test coverage for dump functions |
| ctl/secret/secret.go | Defers Kube client initialization to when needed |
| ctl/secret/secret_test.go | Adds test coverage for secret module |
| ctl/waypoint/waypoint_test.go | Adds test coverage for waypoint module |
| ctl/common/common_test.go | Adds test coverage for common module |
| .github/workflows/sync-gitee.yml | New workflow for syncing to Gitee |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| # Get default logger's level: | ||
| kmeshctl log <kmesh-daemon-pod> default`, | ||
| Args: cobra.MinimumNArgs(1), | ||
| Run: func(cmd *cobra.Command, args []string) { | ||
| RunGetOrSetLoggerLevel(cmd, args) | ||
| RunE: func(cmd *cobra.Command, args []string) error { | ||
| return RunGetOrSetLoggerLevel(cmd, args) | ||
| }, | ||
| } | ||
| cmd.Flags().String("set", "", "Set the logger level (e.g., default:debug)") |
ctl/log/log_test.go
Outdated
| func TestNewCmd(t *testing.T) { | ||
| cmd := NewCmd() | ||
| if cmd.Use != "log" { | ||
| t.Fatalf("Use = %q, want %q", cmd.Use, "log") | ||
| } | ||
|
|
||
| if flag := cmd.Flags().Lookup("set"); flag == nil { | ||
| t.Error("set flag not registered") | ||
| } | ||
| } |
ctl/monitoring/monitoring.go
Outdated
| if err := SetObservability(url, observablityType); err != nil { | ||
| log.Error(err) | ||
| } | ||
| } |
imshubham22apr-gif
force-pushed
the
ci/sync-gitee
branch
from
b76bfbc to
5ba31c9
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
What type of PR is this?
/kind feature
What this PR does / why we need it:
This PR adds a GitHub Actions workflow to automatically mirror the repository to Gitee whenever a new release is published. This ensures that the Gitee mirror stays in sync with the official releases on GitHub.
Which issue(s) this PR fixes:
Fixes #134
Special notes for your reviewer:
I used wearerequired/git-mirror-action for the sync logic. To make this operational, a repository admin will need to:
Does this PR introduce a user-facing change?
NONE