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(windows): Create standalone daemon for non-k8s orchestration #1385

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

BeegiiK
Copy link
Contributor

@BeegiiK BeegiiK commented Feb 24, 2025

Description

Please provide a brief description of the changes made in this pull request.

Related Issue

If this pull request is related to any issue, please mention it here. Additionally, make sure that the issue is assigned to you before submitting this pull request.

Checklist

  • I have read the contributing documentation.
  • I signed and signed-off the commits (git commit -S -s ...). See this documentation on signing commits.
  • I have correctly attributed the author(s) of the code.
  • I have tested the changes locally.
  • I have followed the project's style guidelines.
  • I have updated the documentation, if necessary.
  • I have added tests, if applicable.

Screenshots (if applicable) or Testing Completed

Please add any relevant screenshots or GIFs to showcase the changes made.

Additional Notes

Add any additional notes or context about the pull request here.


Please refer to the CONTRIBUTING.md file for more information on how to contribute to this project.

@BeegiiK BeegiiK changed the title Create standalone daemon for non-k8s orchestration feat(windows): Create standalone daemon for non-k8s orchestration Feb 28, 2025
@BeegiiK BeegiiK self-assigned this Feb 28, 2025
@rbtr rbtr requested a review from Copilot February 28, 2025 16:59
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.

PR Overview

This PR introduces a standalone daemon for non-K8s orchestration. Key changes include:

  • Adding a standalone cache implementation with its corresponding test.
  • Implementing a new standalone daemon in the cmd package.
  • Introducing a BootstrapManager to initialize and start the daemon in non-Kubernetes environments.

Reviewed Changes

File Description
pkg/controllers/cache/standalone_cache_test.go Adds tests for standalone cache functionality
cmd/standalone_daemon.go Implements a new standalone daemon for Retina orchestration
cmd/bootstrap_manager.go Introduces a bootstrap manager to bootstrap the daemon
pkg/controllers/cache/standalone_cache.go Implements cache logic used by the standalone daemon
cmd/standard/daemon.go Updates daemon configuration and startup to integrate new behavior
cmd/root.go Updates CLI to use the new BootstrapManager instead of the daemon directly

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

cmd/bootstrap_manager.go:43

  • [nitpick] Consider using the logger for output instead of fmt.Printf to maintain consistent logging and potentially capture log levels and context.
fmt.Printf("Bootstrapping Retina")

defer c.rwMutex.RUnlock()

if pod, exists := c.ipToPod[ip]; exists {
return &pod
Copy link
Preview

Copilot AI Feb 28, 2025

Choose a reason for hiding this comment

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

Returning the address of the local variable 'pod' (a copy from the map) might lead to unexpected behavior; consider storing pointers directly in the map to avoid copying issues.

Suggested change
return &pod
return pod

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
}

func NewStandaloneDaemon(cfg *config.Config, zl *log.ZapLogger) (*StandaloneDaemon, error) {
fmt.Println("starting Standalone Retina daemon")
Copy link
Preview

Copilot AI Feb 28, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider using the provided logger instead of fmt.Println to ensure consistent logging practices across the project.

Suggested change
fmt.Println("starting Standalone Retina daemon")
zl.Info("starting Standalone Retina daemon")

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant