Skip to content

Conversation

@mugeshsp
Copy link
Contributor

Reason for Change:
This PR implements a comprehensive DeleteEndpointState API in CNS (Container Network Service) to improve endpoint lifecycle management in stateless CNI scenarios. The changes include creating the Delete Endpoint State API, handler, and helper functions, along with integrating the functionality into the network manager with proper validation for FrontendNIC scenarios.

Notes:

  • feat: Add conditional check for NIC type (FrontendNic) - Enhanced the deletion of state file logic to specifically target NodeNetworkInterfaceFrontendNIC types, ensuring only appropriate endpoints are cleaned up through the CNS API
  • Added DeleteEndpointState method to enable HTTP DELETE requests to the CNS endpoint API
  • Implemented DeleteEndpointStateHandler and DeleteEndpointStateHelper and to handle HTTP DELETE requests for endpoint cleanup
  • Tested and confirmed that the deletion of endpoints takes place in the environment

Copilot AI review requested due to automatic review settings August 13, 2025 14:54
@mugeshsp mugeshsp requested review from a team as code owners August 13, 2025 14:54
@mugeshsp mugeshsp requested a review from rbtr August 13, 2025 14:54
@mugeshsp mugeshsp changed the title Fix: Delete State From State File Fix: Delete State From State File (FrontendNIC) Aug 13, 2025
Copy link
Contributor

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 implements a DeleteEndpointState API in CNS to improve endpoint lifecycle management for stateless CNI scenarios. The changes enable proper cleanup of endpoint state files through HTTP DELETE requests with validation for FrontendNIC types.

  • Adds DeleteEndpointState method to CNS client for HTTP DELETE requests
  • Implements DeleteEndpointStateHandler and helper functions in REST server
  • Integrates endpoint deletion into network manager with FrontendNIC validation

Reviewed Changes

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

File Description
network/manager.go Adds conditional endpoint deletion logic for FrontendNIC types in stateless CNI mode
cns/restserver/ipam.go Implements HTTP DELETE handler and helper function for endpoint state management
cns/client/client.go Adds client method to send DELETE requests to CNS endpoint API

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

}

// Delete the endpoint from state
err = service.DeleteEndpointStateHelper(endpointID)
Copy link

Copilot AI Aug 13, 2025

Choose a reason for hiding this comment

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

The request body decoding appears unnecessary for a DELETE operation. Consider removing this unused variable and the associated decode logic (lines 1349-1354) since DELETE requests typically don't require request body parsing for endpoint deletion.

Suggested change
err = service.DeleteEndpointStateHelper(endpointID)
// Delete the endpoint from state
err := service.DeleteEndpointStateHelper(endpointID)

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@behzad-mir behzad-mir left a comment

Choose a reason for hiding this comment

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

Look good overall. Need to address the changes I mentioned.


return &response, nil
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Please address the Lint erros in the PR

@mugeshsp mugeshsp requested a review from a team as a code owner August 21, 2025 11:41
@mugeshsp mugeshsp force-pushed the mugeshsp/aci-state-file branch from f446217 to de798b6 Compare August 21, 2025 11:44
@github-actions
Copy link

github-actions bot commented Sep 9, 2025

This pull request is stale because it has been open for 2 weeks with no activity. Remove stale label or comment or this will be closed in 7 days

@github-actions github-actions bot added the stale Stale due to inactivity. label Sep 9, 2025
@github-actions
Copy link

Pull request closed due to inactivity.

@github-actions github-actions bot closed this Sep 17, 2025
@github-actions github-actions bot deleted the mugeshsp/aci-state-file branch September 17, 2025 00:01
@mugeshsp mugeshsp restored the mugeshsp/aci-state-file branch October 13, 2025 23:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stale Stale due to inactivity.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants