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

status is not getting set on resources properly #2522

Closed
stevesloka opened this issue May 13, 2020 · 6 comments · Fixed by #2613
Closed

status is not getting set on resources properly #2522

stevesloka opened this issue May 13, 2020 · 6 comments · Fixed by #2613
Assignees
Labels
area/httpproxy Issues or PRs related to the HTTPProxy API. kind/bug Categorizes issue or PR as related to a bug.

Comments

@stevesloka
Copy link
Member

Was testing out other features on my cluster and found the expected status hadn't updated on my object. I assumed it was my code with the new status, but upon further investigation, I see various errors around setting status for HTTPProxy objects.

The interesting bit is I don't always get an error, meaning, I change my resources and I sometimes get the status updates, other times I do not BUT I do get an error log, and the last case is the status does not change and I do NOT get an error.

time="2020-05-13T20:23:32Z" level=error msg="failed to set status" context=contourEventHandler desc="this HTTPProxy is not part of a delegation chain from a root HTTPProxy" error="Operation cannot be fulfilled on httpproxies.projectcontour.io \"infosite\": the object has been modified; please apply your changes to the latest version and try again" name=infosite namespace=projectcontour-marketing status=orphaned
@stevesloka stevesloka added area/httpproxy Issues or PRs related to the HTTPProxy API. kind/bug Categorizes issue or PR as related to a bug. labels May 13, 2020
@stevesloka
Copy link
Member Author

NOTE: I validated with v1.4.0 and that seems to work fine.

@stevesloka stevesloka self-assigned this May 13, 2020
@youngnick
Copy link
Member

Was this with master? This is the exact thing I'm working on solving with my current HTTPProxy status refactor.

@youngnick
Copy link
Member

To be more clear - this happens when an update is rejected by the apiserver because the object that it's based on has been modified since Contour got it. It usually happens when there is more than one thing attempting to update the object concurrently (like two Informers acting on the same object).

@stevesloka
Copy link
Member Author

yup this was on master. It worked on a new set of kind clusters, then I spun up a new EKS cluster and it all worked, so no idea what went wrong.

My suspicion it's got to do with the move to the subresource for status.

@stevesloka
Copy link
Member Author

I take that back, it just happened again on a clean cluster. This one is a v1.15. I'm upgrading now to v1.16.

@stevesloka
Copy link
Member Author

So this isn't a version issue I don't think, I had to go back to Contour v1.3.0 for this to work properly.

What I've found is the child proxies do not get updated, only the root.

Here are my repro steps:

# Install Contour
$ kubectl apply -f examples/contour/ 

# Deploy example app
$ kubectl apply -f site/examples/proxydemo/ 

# Validate that all status for 3 proxies are "Valid"
$ kubectl get proxy -A

---
NAMESPACE                  NAME       FQDN                      TLS SECRET   STATUS   STATUS DESCRIPTION
projectcontour-marketing   blogsite                                          valid    valid HTTPProxy
projectcontour-marketing   infosite                                          valid    valid HTTPProxy
projectcontour-roots       root       local.projectcontour.io                valid    valid HTTPProxy
---

# Edit root proxy and change the inclusion from `blogsite` to `blogsite99` (anything invalid)
# Validate that the root proxy is "Invalid" and the child proxies are "Orphaned"
$ kubectl get proxy -A

# Actual (Note: The child proxies keep the old status, so in this example is was "Valid")
NAMESPACE                  NAME       FQDN                      TLS SECRET   STATUS    STATUS DESCRIPTION
projectcontour-marketing   blogsite                                          valid     valid HTTPProxy
projectcontour-marketing   infosite                                          valid     valid HTTPProxy
projectcontour-roots       root       local.projectcontour.io                invalid   include projectcontour-marketing/blogsit88e not found

# Expected 
NAMESPACE                  NAME       FQDN                      TLS SECRET   STATUS     STATUS DESCRIPTION
projectcontour-marketing   blogsite                                          orphaned   this HTTPProxy is not part of a delegation chain from a root HTTPProxy
projectcontour-marketing   infosite                                          orphaned   this HTTPProxy is not part of a delegation chain from a root HTTPProxy
projectcontour-roots       root       local.projectcontour.io                invalid    include projectcontour-marketing/blogsit66e not found

@stevesloka stevesloka assigned youngnick and unassigned stevesloka May 15, 2020
youngnick pushed a commit to youngnick/contour that referenced this issue Jun 19, 2020
Updates the DAG StatusClient to use the StatusUpdateWriter pattern.

Fixes projectcontour#2560
Fixes projectcontour#2580
Fixes projectcontour#2522

We should be able to remove the StatusClient eventually, but that API also needs refactoring for adding Conditions (currently under projectcontour#2495), so for now I've just updated it to send updates via the StatusUpdateWriter rather than make API calls directly.

Signed-off-by: Nick Young <[email protected]>
youngnick pushed a commit to youngnick/contour that referenced this issue Jun 19, 2020
Updates the DAG StatusClient to use the StatusUpdateWriter pattern.

Fixes projectcontour#2560
Fixes projectcontour#2580
Fixes projectcontour#2522

We should be able to remove the StatusClient eventually, but that API also needs refactoring for adding Conditions (currently under projectcontour#2495), so for now I've just updated it to send updates via the StatusUpdateWriter rather than make API calls directly.

Signed-off-by: Nick Young <[email protected]>
youngnick pushed a commit to youngnick/contour that referenced this issue Jun 19, 2020
Updates the DAG StatusClient to use the StatusUpdateWriter pattern.

Fixes projectcontour#2560
Fixes projectcontour#2580
Fixes projectcontour#2522

We should be able to remove the StatusClient eventually, but that API also needs refactoring for adding Conditions (currently under projectcontour#2495), so for now I've just updated it to send updates via the StatusUpdateWriter rather than make API calls directly.

Signed-off-by: Nick Young <[email protected]>
youngnick pushed a commit to youngnick/contour that referenced this issue Jun 19, 2020
Updates the DAG StatusClient to use the StatusUpdateWriter pattern.

Fixes projectcontour#2560
Fixes projectcontour#2580
Fixes projectcontour#2522

We should be able to remove the StatusClient eventually, but that API also needs refactoring for adding Conditions (currently under projectcontour#2495), so for now I've just updated it to send updates via the StatusUpdateWriter rather than make API calls directly.

Signed-off-by: Nick Young <[email protected]>
youngnick pushed a commit to youngnick/contour that referenced this issue Jun 19, 2020
Updates the DAG StatusClient to use the StatusUpdateWriter pattern.

Fixes projectcontour#2560
Fixes projectcontour#2580
Fixes projectcontour#2522

We should be able to remove the StatusClient eventually, but that API also needs refactoring for adding Conditions (currently under projectcontour#2495), so for now I've just updated it to send updates via the StatusUpdateWriter rather than make API calls directly.

Signed-off-by: Nick Young <[email protected]>
youngnick pushed a commit to youngnick/contour that referenced this issue Jun 19, 2020
Updates the DAG StatusClient to use the StatusUpdateWriter pattern.

Fixes projectcontour#2560
Fixes projectcontour#2580
Fixes projectcontour#2522

We should be able to remove the StatusClient eventually, but that API also needs refactoring for adding Conditions (currently under projectcontour#2495), so for now I've just updated it to send updates via the StatusUpdateWriter rather than make API calls directly.

Signed-off-by: Nick Young <[email protected]>
youngnick pushed a commit that referenced this issue Jun 19, 2020
Updates the DAG StatusClient to use the StatusUpdateWriter pattern.

Fixes #2560
Fixes #2580
Fixes #2522

We should be able to remove the StatusClient eventually, but that API also needs refactoring for adding Conditions (currently under #2495), so for now I've just updated it to send updates via the StatusUpdateWriter rather than make API calls directly.

Signed-off-by: Nick Young <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/httpproxy Issues or PRs related to the HTTPProxy API. kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants