Skip to content

feat: Add a default domain reclaim policy #656

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 16, 2025

Conversation

jonstacks
Copy link
Collaborator

See #641 for more context

What

Now that Domains have a Reclaim Policy, we can set a default Domain Reclaim Policy for Domains created by the operator. It respects existing created domain CRs deletion policy. For example, if a domain CR was created manually with a Reclaim Policy of Retain, and the defaultDomainReclaimPolicy is set to Delete, it will not overwrite the existing ReclaimPolicy of Retain.

How

  • Add a setting for defaultDomainReclaimPolicy to the helm chart
  • Pass the defaultDomainReclaimPolicy through to the driver and the Controllers that need it
  • Only set the Reclaim Policy on creates

Breaking Changes

No. The current domain Reclaim Policy defaults to "Delete" and the defaultDomainReclaimPolicy mirrors this. We still do not delete Domain CRs when they drop out of scope of the driver.

@jonstacks jonstacks self-assigned this May 15, 2025
@jonstacks jonstacks requested a review from a team as a code owner May 15, 2025 21:25
@github-actions github-actions bot added area/controller Issues dealing with the controller area/helm-chart Issues dealing with the helm chart labels May 15, 2025
Copy link

codecov bot commented May 15, 2025

Codecov Report

Attention: Patch coverage is 30.00000% with 63 lines in your changes missing coverage. Please review.

Project coverage is 30.84%. Comparing base (92864af) to head (34c51fb).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
cmd/api-manager.go 0.00% 16 Missing ⚠️
cmd/common.go 0.00% 14 Missing ⚠️
cmd/agent-manager.go 0.00% 12 Missing ⚠️
internal/testutils/k8s-resources.go 0.00% 10 Missing ⚠️
...rnal/controller/agent/agent_endpoint_controller.go 0.00% 4 Missing ⚠️
...ernal/controller/ngrok/cloudendpoint_controller.go 25.00% 2 Missing and 1 partial ⚠️
pkg/managerdriver/domains.go 88.00% 1 Missing and 2 partials ⚠️
pkg/managerdriver/driver.go 80.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #656      +/-   ##
==========================================
+ Coverage   30.73%   30.84%   +0.11%     
==========================================
  Files         100      101       +1     
  Lines       15876    15919      +43     
==========================================
+ Hits         4879     4911      +32     
- Misses      10710    10723      +13     
+ Partials      287      285       -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.

@jonstacks jonstacks added this pull request to the merge queue May 16, 2025
Merged via the queue into ngrok:main with commit d642636 May 16, 2025
11 checks passed
@jonstacks jonstacks deleted the domain-deletion-strategy branch May 16, 2025 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/controller Issues dealing with the controller area/helm-chart Issues dealing with the helm chart
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants