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.

Sign up for GitHub

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

Jump to bottom

backup server in upstream for VirtualServer and TransportServer not removed when the backup services is deleted #4774

Closed
3 tasks done
Tracked by #4452
jjngx opened this issue Dec 8, 2023 · 2 comments · Fixed by #5053 · May be fixed by #4947
Closed
3 tasks done
Tracked by #4452

backup server in upstream for VirtualServer and TransportServer not removed when the backup services is deleted #4774

jjngx opened this issue Dec 8, 2023 · 2 comments · Fixed by #5053 · May be fixed by #4947
Assignees
@jjngx @shaun-nx
Labels
backlog Pull requests/issues that are backlog items bug An issue reporting a potential bug
Milestone
v3.4.3

Comments

@jjngx
Copy link
Contributor

jjngx commented Dec 8, 2023 •
edited by shaun-nx

Updates required:

  • Deleting BackupService associated with the Upstream should remove backup entry from NGINX config file (Upstream)
  • Deleting ConfigMap entry - resolver-addresses should trigger Warning

Updates required for tests:

  • update test_transport_server_backup_service.py to validate connection to external service when backend application is unavailable
  • Add unit tests to validate fields
  • Add python tests to make sure if removing backup service sends VS/TS into warning

Investigation notes:
When debugging the issue, we found that both VS and TS resources we not checking if they had a reference to the backup service. Adding a line in reference_checker.go for both VS and TS resources looks to have resolved the issue.

Service reference check for VirtualServer: https://github.com/nginxinc/kubernetes-ingress/blob/main/internal/k8s/reference_checkers.go#L141

Service reference check for TransportServer: https://github.com/nginxinc/kubernetes-ingress/blob/main/internal/k8s/reference_checkers.go#L175

Tasks
Beta Give feedback
@jjngx jjngx mentioned this issue Dec 8, 2023
Closed
@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 8, 2023

Hi @jjngx thanks for reporting!

Be sure to check out the docs and the Contributing Guidelines while you wait for a human to take a look at this 🙂

Cheers!

@jjngx jjngx added enhancement Pull requests for new features/feature enhancements backlog Pull requests/issues that are backlog items labels Dec 8, 2023
@jjngx jjngx self-assigned this Dec 8, 2023
@jjngx jjngx added this to the v3.4.1 milestone Dec 8, 2023
@brianehlert
Copy link
Collaborator

I am fine with not adding additional resource watchers at this point in time. The configuration will eventually update and I think that is fine to support the upstream backup capability.

Such as trying to be smart if the ExternalnameService changes or is deleted. We will address that with additional work.
That is captured here: #4816

@danielnginx danielnginx modified the milestones: v3.4.1, v3.4.3 Feb 7, 2024
@shaun-nx shaun-nx linked a pull request Feb 7, 2024 that will close this issue
Re-enable VS backup service tests #4947
Open
6 tasks
@danielnginx danielnginx mentioned this issue Feb 7, 2024
Open
5 tasks
@shaun-nx shaun-nx changed the title Add validation for backup service (obj ExternalName) backup server in upstream for VirtualServer and TransportServer not removed when the backup services is deleted Feb 8, 2024
@shaun-nx shaun-nx mentioned this issue Feb 8, 2024
Merged
6 tasks
@shaun-nx shaun-nx added bug An issue reporting a potential bug and removed enhancement Pull requests for new features/feature enhancements labels Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jjngx jjngx

@shaun-nx shaun-nx

Labels
backlog Pull requests/issues that are backlog items bug An issue reporting a potential bug
Projects
NGINX Ingress Controller
Archived in project
Milestone
v3.4.3
4 participants
@brianehlert @jjngx @shaun-nx @danielnginx