Skip to content
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

feat: add new workload pod and fixed orphan tcp connections #263

Merged
merged 12 commits into from
Jun 20, 2024

Conversation

hcavarsan
Copy link
Owner

@hcavarsan hcavarsan commented Jun 19, 2024

changes:

  • new workload type pod: implemented connection forwarding based on pod labels. (
    ref: Port-forward to pods not available #260 )
  • tcp connections fix: resolved orphaned tcp connections when port-forwarding is stopped.
  • configuration export fix: only relevant fields are now included in export configurations.

@hcavarsan hcavarsan added the enhancement New feature or request label Jun 19, 2024
@hcavarsan hcavarsan self-assigned this Jun 19, 2024
Copy link
Contributor

coderabbitai bot commented Jun 19, 2024

Walkthrough

The updates involve enhancing functionalities related to JSON configurations, port forwarding tasks, and Kubernetes context handling. Key changes include introducing a mechanism to remove blank or default fields from JSON, utilizing tokio::sync::Notify to control port forwarding tasks, refining pod and service lookup logic, and updating the frontend to handle new workload types and configuration fields. These improvements aim to streamline operations, enhance flexibility, and ensure robust handling of various Kubernetes workloads.

Changes

Files Change Summary
crates/kftray-tauri/src/config.rs Refactored remove_blank_fields to remove_blank_or_default_fields, adding default config handling. Added is_value_default function, updated export_configs.
...kubeforward/commands.rs Integrated tokio::sync::Notify to cancel port forwarding tasks, updated handling for different workloads (pod_label, service).
...kubeforward/kubecontext.rs Added imports (HashSet, Resource), PodInfo struct, and refactored pod and service list functions for improved handling.
...kubeforward/pod_finder.rs Refactored pod finding logic with separate functions for finding by service name or pod label, updated error handling.
...kubeforward/port_forward.rs Added Notify usage, integrated CANCEL_NOTIFIER for cancellation in port forwarding tasks, adjusted relevant method signatures.
frontend/src/.../AddConfigModal/... Renamed variables for clarity, added new state variables, updated queries and logic for handling pods and services, modified UI for workload type.
frontend/src/.../Main/index.tsx Included target field in several places, updated logic to handle workload_type values for port forwarding.
frontend/src/.../PortForwardRow/... Enhanced port forwarding logic to include pod workload type, adjusted UI to reflect changes in workload types.
frontend/src/types/index.ts Updated Status, Config, Response interfaces to include new target field.
.github/workflows/ci.yaml Updated GitHub Actions workflow to use docker/build-push-action@v6.
.github/workflows/main.yml Added step for reconfiguring for Linux ARM64 in the workflow.

Sequence Diagram(s)

Port Forwarding Cancellation

sequenceDiagram
    participant User
    participant UI as User Interface
    participant App as KFTray App
    participant PF as Port Forwarding Task
    participant CancelNotifier as CANCEL_NOTIFIER

    User->>UI: Initiates port forwarding
    UI->>App: Start port forwarding with workload type
    App->>PF: Starts task
    PF-->>App: Task running
    User->>UI: Requests cancellation
    UI->>App: Cancels port forwarding
    App->>CancelNotifier: Notify cancellation
    CancelNotifier->>PF: Signals task to cancel
    PF->>App: Task cancelled
    App-->>User: Cancellation confirmed
Loading

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between d7fa15b and 32d0dd6.

Files ignored due to path filters (2)
  • crates/kftray-tauri/tauri.conf.json is excluded by !**/*.json
  • frontend/dist/index.html is excluded by !**/dist/**
Files selected for processing (12)
  • crates/kftray-tauri/src/config.rs (7 hunks)
  • crates/kftray-tauri/src/kubeforward/commands.rs (13 hunks)
  • crates/kftray-tauri/src/kubeforward/kubecontext.rs (6 hunks)
  • crates/kftray-tauri/src/kubeforward/pod_finder.rs (3 hunks)
  • crates/kftray-tauri/src/kubeforward/port_forward.rs (10 hunks)
  • crates/kftray-tauri/src/main.rs (1 hunks)
  • crates/kftray-tauri/src/models/config.rs (3 hunks)
  • crates/kftray-tauri/src/models/kube.rs (2 hunks)
  • frontend/src/components/AddConfigModal/index.tsx (19 hunks)
  • frontend/src/components/Main/index.tsx (8 hunks)
  • frontend/src/components/PortForwardTable/ContextsAccordion/PortForwardRow/index.tsx (5 hunks)
  • frontend/src/types/index.ts (3 hunks)
Additional comments not posted (45)
crates/kftray-tauri/src/models/config.rs (2)

29-30: Added a new optional field target to the Config struct.

This addition aligns with the PR objectives to support new workload types and is correctly implemented using optional serialization to maintain backward compatibility.


39-40: Updated default values for local_port, remote_port, protocol, and target.

These changes are consistent with the PR's intention to enhance configuration management. Ensure that these new defaults are documented and communicated to users if they represent a significant change from previous configurations.

Also applies to: 43-43, 49-49

crates/kftray-tauri/src/models/kube.rs (1)

39-42: Introduced a new struct PodInfo with a single field labels_str.

This struct is well-defined and supports the PR's goals of better handling different workload types. The use of labels_str suggests flexibility in selecting pods based on multiple labels.

crates/kftray-tauri/src/kubeforward/pod_finder.rs (2)

21-33: Refactored the find method to support both service names and pod labels.

The refactoring is logical and supports the PR's objectives by enhancing the flexibility of pod selection based on different criteria. Ensure that this method is covered by unit tests, especially for the new branching logic.


Line range hint 35-89: Added methods find_pod_by_service_name and find_pod_by_label to facilitate pod discovery based on service names or labels.

These methods are well-implemented and improve the modularity of the code. They also align with the PR's goals of handling different workload types more effectively. Again, ensure comprehensive testing for these new methods.

crates/kftray-tauri/src/main.rs (1)

115-115: Updated the list of Tauri command handlers to include new Kubernetes-related functionalities.

The addition of these handlers is essential for integrating the new backend functionalities with the Tauri frontend. This ensures that the new features are accessible and manageable through the application's GUI.

frontend/src/types/index.ts (3)

14-14: The addition of the 'target' field in the Status interface is consistent with the PR objectives of handling different workload types.


34-34: The addition of the 'target' field in the Config interface supports new functionality and is consistent with other parts of the application.


49-49: The addition of the 'target' field in the Response interface aligns with the new features introduced in the PR.

crates/kftray-tauri/src/kubeforward/kubecontext.rs (5)

1-1: The added imports and the new PodInfo struct are essential for supporting the new functionalities related to Kubernetes pod management.

Also applies to: 15-15, 39-39


113-116: The modified function signature of list_pods enhances clarity and aligns with the new PodInfo struct usage.


117-155: The implementation of the list_pods function correctly utilizes the new PodInfo struct and standard Rust practices for error handling and asynchronous programming.


277-277: The modified function signature of list_service_ports aligns with the new error handling strategy and improves the function's usability.


279-379: The implementation of list_service_ports effectively handles different scenarios based on target_port and improves error handling, aligning with the PR's objectives.

crates/kftray-tauri/src/config.rs (3)

22-36: The new function is_value_default and the modifications to remove_blank_or_default_fields enhance configuration management by effectively handling default values.

Also applies to: 45-50


296-298: The modifications to the export_configs function improve the integrity of exported configurations by ensuring that they do not contain redundant default values.


Line range hint 425-466: The test for remove_blank_or_default_fields effectively verifies that the function correctly removes blank and default fields, ensuring its reliability.

frontend/src/components/PortForwardTable/ContextsAccordion/PortForwardRow/index.tsx (2)

106-110: The modifications to the startPortForwarding and stopPortForwarding functions to handle 'service' and 'pod' workload types for TCP protocols align with the PR's objectives and are correctly implemented.

Also applies to: 139-143


255-270: The modifications to the tooltip display logic to show 'Pod' instead of 'Service' when the workload type is 'pod' enhance the user interface consistency and clarity.

frontend/src/components/Main/index.tsx (7)

40-40: Introduced a new state variable target in the initial state setup for newConfig. Ensure that this is consistently used throughout the component and properly integrated with backend changes.


85-85: Similarly, target is reset when opening the modal. This is consistent with the initial state setup. Good practice in maintaining state consistency.


251-251: The new target field is being set from the fetched config during edit operations. This ensures that the target field is editable and maintained across component states.


280-280: The target field is included in the submission of edited configurations. This is crucial for ensuring that edits to the target field are persisted.


329-329: The target field is included when saving a configuration, whether it’s a new addition or an edit. This inclusion is necessary for the correct operation of the feature.
[APROVED]


Line range hint 349-401: The logic for stopping and starting port forwarding now takes into account the workload_type of 'pod'. This is a significant change that aligns with the backend modifications to support different types of workloads.


474-474: Handling of the workload_type 'pod' in the handlePortForwarding function. This change is necessary for the new functionality and is consistent with the backend changes.

crates/kftray-tauri/src/kubeforward/port_forward.rs (6)

14-14: Added import for tokio::sync::Notify. This is used for handling cancellation signals, which is a critical feature for managing long-running network operations safely.


32-32: Usage of a global static CANCEL_NOTIFIER which is crucial for managing cancellation signals across asynchronous tasks in port forwarding operations.


Line range hint 103-129: Enhanced the port_forward_tcp function to use the CANCEL_NOTIFIER for handling cancellations effectively. This is a robust enhancement for improving the reliability and responsiveness of the port forwarding feature.


Line range hint 156-213: Refactored forward_connection to include the cancel_notifier. This change is part of enhancing the cancellation handling in the port forwarding logic, allowing for cleaner termination of connections.


Line range hint 243-367: Implemented detailed cancellation handling in both create_client_to_upstream_task and create_upstream_to_client_task. These changes ensure that tasks can be cancelled promptly, which is crucial for resource management and error handling.


Line range hint 380-421: The detect_connection_close function now also listens for cancellation notifications, allowing the system to react quickly to cancellation requests and close connections appropriately.

crates/kftray-tauri/src/kubeforward/commands.rs (8)

31-31: Introducing tokio::sync::Notify for task cancellation is a good practice in asynchronous environments.


54-54: The static reference CANCEL_NOTIFIER is well implemented using Arc to manage shared state across async tasks.


129-136: The log message construction within the match block is well implemented. It dynamically adjusts the message based on the workload type, enhancing the maintainability of the code.


220-226: The error handling in this section is robust, providing detailed error messages that are specific to the configuration and protocol. This will be helpful for debugging issues with port forwarding setups.


237-242: The error message here is clear and informative, which is crucial for understanding the failure points when creating PortForward instances.


281-283: Using notify_waiters to signal cancellation across potentially multiple waiting tasks is an efficient way to handle cleanup in asynchronous operations.


427-429: Proper use of the cancellation notifier in the stop_port_forward function ensures that all related tasks are notified to terminate, adhering to good concurrency practices.


79-85: The handling of different workload types using a match statement is clear and well-structured. However, ensure proper handling and testing when the target or service fields are None, as unwrap_or_default() might introduce unexpected behaviors.

frontend/src/components/AddConfigModal/index.tsx (5)

53-53: The introduction of selectedServiceOrTarget as a state variable facilitates handling both services and targets, aligning with the backend changes to support multiple workload types.


155-169: Query setup for listing pods is well implemented, with appropriate checks to ensure the query is only enabled when necessary. This prevents unnecessary API calls.


251-256: The logic to handle the selection of either a service or target based on the workload type is well implemented, providing flexibility in the UI to adapt to different configurations.


660-686: The conditional rendering based on the workload type (pod vs service) is a good approach to dynamically adjust the UI. This makes the component more adaptable to different use cases.


528-528: Adding the 'pod' option to the workload type selector is a necessary update to support the new functionality introduced in the backend. Ensure that this option is well tested across different scenarios.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 32d0dd6 and 7fb377a.

Files selected for processing (1)
  • .github/workflows/main.yml (1 hunks)
Files skipped from review due to trivial changes (1)
  • .github/workflows/main.yml

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 7fb377a and 6ee7507.

Files selected for processing (1)
  • .github/workflows/main.yml (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/main.yml

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 6ee7507 and 2a22d1f.

Files selected for processing (1)
  • .github/workflows/main.yml (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/main.yml

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 2a22d1f and a701340.

Files selected for processing (1)
  • .github/workflows/main.yml (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/main.yml

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between a701340 and 204eddf.

Files selected for processing (1)
  • .github/workflows/main.yml (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/main.yml

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 204eddf and 27a8cc8.

Files selected for processing (1)
  • .github/workflows/main.yml (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/main.yml

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 27a8cc8 and a87c4d5.

Files selected for processing (8)
  • crates/kftray-tauri/src/kubeforward/commands.rs (13 hunks)
  • crates/kftray-tauri/src/kubeforward/kubecontext.rs (6 hunks)
  • crates/kftray-tauri/src/kubeforward/pod_finder.rs (2 hunks)
  • crates/kftray-tauri/src/kubeforward/pod_selection.rs (2 hunks)
  • crates/kftray-tauri/src/kubeforward/port_forward.rs (10 hunks)
  • crates/kftray-tauri/src/main.rs (1 hunks)
  • frontend/src/components/AddConfigModal/index.tsx (18 hunks)
  • frontend/src/components/PortForwardTable/ContextsAccordion/PortForwardRow/index.tsx (5 hunks)
Files skipped from review as they are similar to previous changes (6)
  • crates/kftray-tauri/src/kubeforward/commands.rs
  • crates/kftray-tauri/src/kubeforward/kubecontext.rs
  • crates/kftray-tauri/src/kubeforward/pod_finder.rs
  • crates/kftray-tauri/src/kubeforward/port_forward.rs
  • crates/kftray-tauri/src/main.rs
  • frontend/src/components/PortForwardTable/ContextsAccordion/PortForwardRow/index.tsx
Additional comments not posted (2)
frontend/src/components/AddConfigModal/index.tsx (2)

537-537: Update UI elements and logic to handle new workload types effectively.

The changes to the UI elements and the logic for handling different workload types, such as the addition of the 'pod' option and the conditional rendering based on workload_type, are well-implemented. These changes enhance the flexibility of the configuration modal and improve the user experience by accommodating different Kubernetes resource types.

Also applies to: 669-721


155-170: Review the conditions for enabling queries to ensure they align with the intended functionality.

This verification will help ensure that the conditions for enabling queries are consistent and correctly implemented across the component.

Also applies to: 172-172, 195-223

Verification successful

The conditions for enabling the queries in the frontend/src/components/AddConfigModal/index.tsx file appear to be consistent and well-aligned with the intended functionality. Each useQuery call has specific conditions based on the state variables, such as newConfig.context, newConfig.namespace, and newConfig.workload_type.

  • namespaceQuery is enabled if newConfig.context is defined.
  • podsQuery is enabled if newConfig.context, newConfig.namespace are defined, and newConfig.workload_type is 'pod'.
  • serviceOrTargetQuery is enabled if newConfig.context, newConfig.namespace are defined, and newConfig.workload_type is not 'proxy'.
  • portQuery conditions are based on newConfig.context, newConfig.namespace, and newConfig.workload_type.

These conditions are consistent and ensure that queries are only enabled when the necessary state variables are available.

Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify the conditions under which queries are enabled and ensure they align with the expected states.

# Test: Check for consistency in the conditions across different queries.
rg --type tsx --files-with-matches $'useQuery\\(.*enabled:'

Length of output: 87



Script:

#!/bin/bash
# Description: Verify the conditions under which queries are enabled and ensure they align with the expected states.

# Test: Check for consistency in the conditions across different queries.
rg --type-add 'tsx:*.{ts,tsx}' --files-with-matches $'useQuery\\(.*enabled:'

Length of output: 76



Script:

#!/bin/bash
# Description: Search for all `useQuery` calls in the codebase for manual inspection.

# Test: Find all instances of `useQuery` in TypeScript files.
rg --type-add 'tsx:*.{ts,tsx}' 'useQuery('

Length of output: 113



Script:

#!/bin/bash
# Description: Search for all `useQuery` calls in the codebase for manual inspection.

# Test: Find all instances of `useQuery` in TypeScript files.
rg --type-add 'tsx:*.{ts,tsx}' 'useQuery\('

Length of output: 377



Script:

#!/bin/bash
# Description: Extract the code around `useQuery` calls to inspect the conditions for enabling the queries.

# Extract relevant code snippets for each `useQuery` call.
rg --type-add 'tsx:*.{ts,tsx}' -A 10 -B 10 'useQuery\(' frontend/src/components/AddConfigModal/index.tsx

Length of output: 1970

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between a87c4d5 and e31a5ab.

Files ignored due to path filters (1)
  • crates/kftray-tauri/tauri.conf.json is excluded by !**/*.json
Files selected for processing (6)
  • .github/workflows/ci.yaml (1 hunks)
  • .github/workflows/main.yml (2 hunks)
  • crates/kftray-tauri/src/config.rs (7 hunks)
  • crates/kftray-tauri/src/kubeforward/commands.rs (13 hunks)
  • crates/kftray-tauri/src/kubeforward/port_forward.rs (10 hunks)
  • frontend/src/components/AddConfigModal/index.tsx (18 hunks)
Files skipped from review due to trivial changes (1)
  • .github/workflows/ci.yaml
Files skipped from review as they are similar to previous changes (5)
  • .github/workflows/main.yml
  • crates/kftray-tauri/src/config.rs
  • crates/kftray-tauri/src/kubeforward/commands.rs
  • crates/kftray-tauri/src/kubeforward/port_forward.rs
  • frontend/src/components/AddConfigModal/index.tsx

@hcavarsan hcavarsan merged commit 9ac1beb into main Jun 20, 2024
7 checks passed
@hcavarsan hcavarsan deleted the add-pod-label branch June 20, 2024 15:39
hcavarsan added a commit that referenced this pull request Jun 20, 2024
* test

* feat: add new workload pod and fixed orphan tcp connections

* feat: add new workload pod and fixed orphan tcp connections

* feat: add new workload pod and fixed orphan tcp connections

* feat: add new workload pod and fixed orphan tcp connections

* feat: add new workload pod and fixed orphan tcp connections

* feat: add new workload pod and fixed orphan tcp connections

* Update main.yml

* Update main.yml

* feat: add new workload pod and fixed orphan tcp connections

* feat: add new workload pod and fixed orphan tcp connections

* feat: add new workload pod and fixed orphan tcp connections
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant