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

updates/deletes to unrelated Services trigger DAG rebuilds #4793

Closed
skriss opened this issue Oct 13, 2022 · 4 comments · Fixed by #4827
Closed

updates/deletes to unrelated Services trigger DAG rebuilds #4793

skriss opened this issue Oct 13, 2022 · 4 comments · Fixed by #4827
Assignees
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. Hacktoberfest Denotes an issue ready for any "Hacktoberfest" contributor. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/bug Categorizes issue or PR as related to a bug.
Milestone

Comments

@skriss
Copy link
Member

skriss commented Oct 13, 2022

Spun out from #4386, it looks like the same thing happens for Services since we are not calling serviceTriggersRebuild on removals (which happen for both updates and deletes).

@skriss skriss added kind/bug Categorizes issue or PR as related to a bug. lifecycle/needs-triage Indicates that an issue needs to be triaged by a project contributor. labels Oct 13, 2022
@skriss
Copy link
Member Author

skriss commented Oct 13, 2022

Adding to 1.24 to be tackled along with the Secrets work.

@skriss skriss removed the lifecycle/needs-triage Indicates that an issue needs to be triaged by a project contributor. label Oct 13, 2022
@skriss skriss added this to Contour Oct 13, 2022
@skriss skriss added this to the 1.24.0 milestone Oct 13, 2022
@skriss skriss moved this to Todo in Contour Oct 13, 2022
@vishal-chdhry
Copy link
Member

vishal-chdhry commented Oct 25, 2022

Hi! @skriss I would love to contribute but I can't understand what I have to do. Can you please guide me a little

@skriss
Copy link
Member Author

skriss commented Oct 25, 2022

Hey @vishal-chdhry! Here's some more detail, let me know if you have further questions.

#4792 did something similar, but for Secrets -- it's a good template to follow.

When we insert a Service into the DAG cache, we call kc.serviceTriggersRebuild to see if the DAG needs to be rebuilt (https://github.com/projectcontour/contour/blob/main/internal/dag/cache.go#L112). However, when we update/delete a Service, we are not performing that same check (https://github.com/projectcontour/contour/blob/main/internal/dag/cache.go#L263-L267). We need to add that check to the remove logic.

We should also ensure that https://github.com/projectcontour/contour/blob/main/internal/dag/cache_test.go#L953 has test cases for both (a) deleting a Service that is referenced by an HTTPProxy, which should trigger a rebuild; and (b) deleting a Service that is not referenced, which should not trigger a rebuild.

https://github.com/projectcontour/contour/blob/main/internal/featuretests/v3/cluster_test.go#L622 would also be good to add to, to ensure that updating/deleting the Service does not trigger xDS traffic. You can look at https://github.com/projectcontour/contour/pull/4792/files#diff-39afe30b2f5c7952f94cbdcd0e39f1d985f3ed59b9d292100637eb82a1f21c6a for an example of how something similar was tested for Secrets.

@vishal-chdhry
Copy link
Member

\assign

@skriss skriss moved this from Todo to In Progress in Contour Oct 28, 2022
skriss pushed a commit that referenced this issue Nov 2, 2022
Updates/deletes to Services that are not referenced
by an Ingress, HTTPProxy, or HTTPRoute no longer
trigger DAG rebuilds and xDS traffic.

Closes #4793.

Signed-off-by: Vishal Choudhary <contactvishaltech@gmail.com>
Repository owner moved this from In Progress to Done in Contour Nov 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. Hacktoberfest Denotes an issue ready for any "Hacktoberfest" contributor. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/bug Categorizes issue or PR as related to a bug.
Projects
Status: Done
2 participants