feat: populate resourceID for trafficManagerBackend

[zhiying-lin]

What type of PR is this?

/kind feature

What this PR does / why we need it:

populate resourceID for trafficManagerBackend status

Which issue(s) this PR fixes:

Fixes #

Requirements:

• run make reviewable for basic local test
• Read and followed fleet-networking's Code of conduct.
• uses conventional commit messages
• includes documentation
• adds unit tests

How has this code been tested

e2e tests, https://github.com/Azure/fleet-networking/actions/runs/14377670818

Special notes for your reviewer

[kaito-pr-agent]

Title

(Describe updated until commit 3a2342e)

Add resourceID support for TrafficManagerBackend

---

Description

• Added resourceID field to TrafficManagerEndpointStatus in controller.go.

• Updated test cases to include resourceID validation in controller_integration_test.go.

• Modified fakeprovider to return resourceID in EndpointCreateOrUpdate method.

• Adjusted validator to ignore resourceID in TrafficManagerEndpointStatus comparison.

---

Changes walkthrough 📝

	Relevant files
Enhancement	4 files controller.go Populate resourceID for TrafficManagerEndpointStatus          +13/-4    controller_integration_test.go Update test cases for resourceID validation                            +44/-24  endpoint.go Modify EndpointCreateOrUpdate to return resourceID              +2/-0      backend.go Ignore resourceID in TrafficManagerEndpointStatus comparison +3/-1
Additional files	3 files profile.go +0/-1      profile.go +2/-1      traffic_manager_test.go +1/-0

---

Need help?

Type /help how to ... in the comments thread for any questions about PR-Agent usage.

Check out the documentation for more information.

[kaito-pr-agent]

PR Reviewer Guide 🔍

(Review updated until commit 3a2342e)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Potential Error Handling Improvement The error handling for the resourceID assignment could be improved. Currently, it logs an error but continues execution without setting the resourceID. This might lead to unexpected behavior where resourceID remains empty. if endpoint.ID == nil { err := controller.NewUnexpectedBehaviorError(fmt.Errorf("got nil ID for Azure Traffic Manager endpoint")) klog.ErrorS(err, "Unexpected value returned by the Azure Traffic Manager", "atmEndpointName", *endpoint.Name) } else { resourceID = *endpoint.ID } Ignoring ResourceID During Validation Ignoring both Name and ResourceID during validation might lead to inconsistencies between the expected and actual state of the Traffic Manager backend. Consider validating these fields explicitly. // It will be validated separately by comparing the values with the ones in the Azure traffic manager profile. cmpopts.IgnoreFields(fleetnetv1beta1.TrafficManagerEndpointStatus{}, "Name", "ResourceID"), // ignore the generated endpoint name cmpopts.SortSlices(func(s1, s2 fleetnetv1beta1.TrafficManagerEndpointStatus) bool {

[kaito-pr-agent]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Optimize ATM endpoint name generation Move ATM endpoint name generation outside loop pkg/controllers/hub/trafficmanagerbackend/controller.go [729-730] -atmEndpointName := fmt.Sprintf(AzureResourceEndpointNameFormat, backendName+"#", serviceName, memberClusterNames[0]) +atmEndpointNames := make([]string, len(memberClusterNames)) +for i, clusterName := range memberClusterNames { + atmEndpointNames[i] = fmt.Sprintf(AzureResourceEndpointNameFormat, backendName+"#", serviceName, clusterName) +} Suggestion importance[1-10]: 7 __ Why: Moving ATM endpoint name generation outside the loop improves performance and readability.	Medium
	Add missing endpoint properties Ensure consistent endpoint properties test/e2e/traffic_manager_test.go [771-775] res.Properties.Endpoints = append(res.Properties.Endpoints, &armtrafficmanager.Endpoint{ ID: &e.ResourceID, Name: ptr.To(e.Name), Type: ptr.To("Microsoft.Network/trafficManagerProfiles/azureEndpoints"), Properties: &armtrafficmanager.EndpointProperties{ Target: e.Target, + Weight: e.Weight, + TargetResourceID: e.TargetResourceID, + }, +}) Suggestion importance[1-10]: 6 __ Why: Ensuring consistent endpoint properties improves the completeness and correctness of the code.	Low
	Simplify ID assignment Remove unnecessary error handling for non-nil IDs pkg/controllers/hub/trafficmanagerbackend/controller.go [562-567] -resourceID := "" -if endpoint.ID == nil { - err := controller.NewUnexpectedBehaviorError(fmt.Errorf("got nil ID for Azure Traffic Manager endpoint")) - klog.ErrorS(err, "Unexpected value returned by the Azure Traffic Manager", "atmEndpointName", *endpoint.Name) -} else { - resourceID = *endpoint.ID -} +resourceID := *endpoint.ID Suggestion importance[1-10]: 5 __ Why: The existing error handling for non-nil IDs is unnecessary and can be simplified.	Low
	Simplify comparison options Remove redundant sorting option test/common/trafficmanager/validator/backend.go [36-37] cmpTrafficManagerBackendStatusByIgnoringEndpointName = cmp.Options{ cmpConditionOptions, cmpopts.IgnoreFields(fleetnetv1beta1.TrafficManagerEndpointStatus{}, "Name", "ResourceID"), // ignore the generated endpoint name - cmpopts.SortSlices(func(s1, s2 fleetnetv1beta1.TrafficManagerEndpointStatus) bool { - return s1.From.Cluster < s2.From.Cluster - }), } Suggestion importance[1-10]: 4 __ Why: Removing the redundant sorting option simplifies the comparison options without affecting functionality.	Low

[codecov]

Codecov Report

Attention: Patch coverage is 66.66667% with 4 lines in your changes missing coverage. Please review.

Project coverage is 80.10%. Comparing base (fc815f5) to head (3a2342e).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...ontrollers/hub/trafficmanagerbackend/controller.go	66.66%	2 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #287      +/-   ##
==========================================
- Coverage   80.16%   80.10%   -0.06%     
==========================================
  Files          29       29              
  Lines        4024     4032       +8     
==========================================
+ Hits         3226     3230       +4     
- Misses        636      638       +2     
- Partials      162      164       +2     

☔ View full report in Codecov by Sentry.
📢 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.

[kaito-pr-agent]

Persistent review updated to latest commit 3a2342e