Skip to content

Fix authz during shutdown #1395

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 15, 2025
Merged

Conversation

hzxuzhonghu
Copy link
Member

…rontend map for xdp use

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:
Fixes #1381

Special notes for your reviewer:

Does this PR introduce a user-facing change?:


@Copilot Copilot AI review requested due to automatic review settings May 13, 2025 08:35
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refines the handling of unhealthy workloads by ensuring that such workloads are removed from the BPF endpoint map while remaining in the frontend/backend maps for authorization purposes. The changes include updating test assertions for workload status, refactoring the processor to call a new handler for unhealthy workloads, and adjusting logging levels and inline comments in BPF C code.

Reviewed Changes

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

File Description
pkg/controller/workload/workload_processor_test.go Updated tests to verify unhealthy workloads are removed from the endpoint map while retained in frontend/backend maps.
pkg/controller/workload/workload_processor.go Introduced handleUnhealthyWorkload to manage unhealthy workloads and updated comments accordingly.
bpf/kmesh/workload/xdp.c Changed log level from INFO to DEBUG in the xdp routine.
bpf/kmesh/workload/include/frontend.h Updated inline comments to clarify that unhealthy backend status is not checked.

@@ -34,7 +34,7 @@ static inline int frontend_manager(struct kmesh_context *kmesh_ctx, frontend_val
direct_backend = true;
}

if (direct_backend) {
if (direct_backend) { // in this case, we donot check the healthy status of the backend, just let it go
Copy link
Preview

Copilot AI May 13, 2025

Choose a reason for hiding this comment

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

Consider correcting 'donot' to 'do not' for improved clarity.

Suggested change
if (direct_backend) { // in this case, we donot check the healthy status of the backend, just let it go
if (direct_backend) { // in this case, we do not check the healthy status of the backend, just let it go

Copilot uses AI. Check for mistakes.

…rontend map for xdp use

Signed-off-by: Zhonghu Xu <xuzhonghu@huawei.com>
Copy link

codecov bot commented May 13, 2025

Codecov Report

Attention: Patch coverage is 70.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 46.27%. Comparing base (3f0912e) to head (897be44).
Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
pkg/controller/workload/workload_processor.go 70.00% 2 Missing and 1 partial ⚠️
Files with missing lines Coverage Δ
pkg/controller/workload/workload_processor.go 61.74% <70.00%> (+<0.01%) ⬆️

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c255796...897be44. Read the comment docs.

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

@hzxuzhonghu hzxuzhonghu changed the title unhealthy workload should be removed from endpoint map, but kept in f… Fix authz during shutdown May 13, 2025
@kmesh-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by:

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

The pull request process is described here

Needs approval from an approver in each of these files:

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

@kmesh-bot kmesh-bot merged commit eed5c73 into kmesh-net:main May 15, 2025
11 checks passed
@hzxuzhonghu hzxuzhonghu deleted the graceful-shutdown branch May 15, 2025 02:09
@kmesh-bot
Copy link
Collaborator

In response to a cherrypick label: new pull request created: #1400

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Delete pod with long running connection met error
2 participants