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

design: Add a pattern for adding extensible status information to HTTPProxy #2495

Closed
2 of 3 tasks
youngnick opened this issue May 4, 2020 · 10 comments
Closed
2 of 3 tasks
Labels
area/status Issues or PRs related to exposing status for APIs. blocked/needs-design Categorizes the issue or PR as blocked because it needs a design document. kind/design Categorizes issue or PR as related to design. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.

Comments

@youngnick
Copy link
Member

youngnick commented May 4, 2020

This issue covers the work to add more information to HTTPProxy's status stanza.

Features we need from HTTPProxy's status stanza:

  • Ability to see the way to reach the HTTPProxy (the equivalent to the Ingress status.loadBalancer stanza). Set load balancer address on HTTPProxy objects #2371 covers the work to implement this.
  • Ability to set multiple statuses on a HTTPProxy. Although the guidance for their design is somewhat muddy at the moment (see Update Condition guidance kubernetes/community#4521), Conditions are the API-standard way to do this.
  • A way for HTTPProxy resources to understand what routing conditions are delegated to them from parent HTTPProxies, and possibly a way for parent HTTPProxies to see what is visible underneath them.

Blocks #2325, through this blocks #370, #432, and #1691, and #399
Blocks #2021
Blocks #2371
Blocks #1598
Blocks #2141

Previous work was in #1868, although that was before I knew about Conditions.

We may also need to consider changing the status stanza to be an actual subresource, although that may be a large enough change that it should be a new document version We did need to update to a subresource to make handling the status updates easier, and to allow separate RBAC of them.

@youngnick youngnick added blocked/needs-design Categorizes the issue or PR as blocked because it needs a design document. kind/design Categorizes issue or PR as related to design. area/status Issues or PRs related to exposing status for APIs. labels May 4, 2020
@youngnick youngnick self-assigned this May 4, 2020
@youngnick youngnick added this to Unprioritized in Contour Project Board via automation May 4, 2020
@youngnick youngnick moved this from Unprioritized to In progress in Contour Project Board May 4, 2020
youngnick pushed a commit to youngnick/contour that referenced this issue May 4, 2020
This commit enables the status subresource for HTTPProxy.
This means that:
- normal HTTPProxy changes that include updates to the `status` stanza will have the status updates dropped
- updates to `status` can only be made using requests to the status subresource.

Also changes the HTTPProxy status updating to use the status subresource,
and adds RBAC for Contour to be able to update it.

Having the status subresource enabled is best-practice for CRDs and will be a foundation for further status
work.

It also means that HTTPProxy objects no longer need to include the `status` stanza to be valid.

Updates projectcontour#2495

Signed-off-by: Nick Young <ynick@vmware.com>
@bgagnon
Copy link
Contributor

bgagnon commented May 4, 2020

This would be a much welcome addition for us:

  • we auto-provision DNS from HTTPProxy, but we're lacking a way to communicate the status of that back to the user
  • we also auto-provision TLS certificates -- Contour's status field says valid when the Secret exists and invalid before that, but there's not a lot of nuance

Our solution for now is to emit detailed events (success/error) attached to the HTTPProxy for the above. If the status field can reflect the Envoy configuration status, plus some extensible fields, then we'd have a comprehensive status.

The only thing missing for a full CD pipeline would be for kubectl to grow the ability to wait on these status values (kubernetes/kubernetes#83094).

youngnick pushed a commit to youngnick/contour that referenced this issue May 5, 2020
This commit enables the status subresource for HTTPProxy.
This means that:
- normal HTTPProxy changes that include updates to the `status` stanza will have the status updates dropped
- updates to `status` can only be made using requests to the status subresource.

Also changes the HTTPProxy status updating to use the status subresource,
and adds RBAC for Contour to be able to update it.

Having the status subresource enabled is best-practice for CRDs and will be a foundation for further status
work.

It also means that HTTPProxy objects no longer need to include the `status` stanza to be valid.

Updates projectcontour#2495

Signed-off-by: Nick Young <ynick@vmware.com>
@youngnick
Copy link
Member Author

That's a good point @bgagnon - As I start experimenting with adding Conditions to HTTPProxy, I'll make sure I try out the support kubectl wait has for Conditions already.

In addition, after talking to @jpeach and @stevesloka today, I'm going to go ahead with #2371, and add the loadBalancer stanza to HTTPProxy. This will work in the same way that Ingress status updating works today - either pulled from the Envoy service, or passed directly to Contour.

youngnick pushed a commit to youngnick/contour that referenced this issue May 6, 2020
This commit enables the status subresource for HTTPProxy.
This means that:
- normal HTTPProxy changes that include updates to the `status` stanza will have the status updates dropped
- updates to `status` can only be made using requests to the status subresource.

Also changes the HTTPProxy status updating to use the status subresource,
and adds RBAC for Contour to be able to update it.

Having the status subresource enabled is best-practice for CRDs and will be a foundation for further status
work.

It also means that HTTPProxy objects no longer need to include the `status` stanza to be valid.

Updates projectcontour#2495

Signed-off-by: Nick Young <ynick@vmware.com>
youngnick pushed a commit to youngnick/contour that referenced this issue May 7, 2020
This commit enables the status subresource for HTTPProxy.
This means that:
- normal HTTPProxy changes that include updates to the `status` stanza will have the status updates dropped
- updates to `status` can only be made using requests to the status subresource.

Also changes the HTTPProxy status updating to use the status subresource,
and adds RBAC for Contour to be able to update it.

Having the status subresource enabled is best-practice for CRDs and will be a foundation for further status
work.

It also means that HTTPProxy objects no longer need to include the `status` stanza to be valid.

Updates projectcontour#2495

Signed-off-by: Nick Young <ynick@vmware.com>
youngnick pushed a commit to youngnick/contour that referenced this issue May 7, 2020
This commit enables the status subresource for HTTPProxy.
This means that:
- normal HTTPProxy changes that include updates to the `status` stanza will have the status updates dropped
- updates to `status` can only be made using requests to the status subresource.

Also changes the HTTPProxy status updating to use the status subresource,
and adds RBAC for Contour to be able to update it.

Having the status subresource enabled is best-practice for CRDs and will be a foundation for further status
work.

It also means that HTTPProxy objects no longer need to include the `status` stanza to be valid.

Refactors the `k8s.UnstructuredConverter` to have FromUnstructured and ToUnstructured methods, and uses ToUnstructured where required.

Updates projectcontour#2495

Signed-off-by: Nick Young <ynick@vmware.com>
youngnick pushed a commit to youngnick/contour that referenced this issue May 7, 2020
This commit enables the status subresource for HTTPProxy.
This means that:
- normal HTTPProxy changes that include updates to the `status` stanza will have the status updates dropped
- updates to `status` can only be made using requests to the status subresource.

Also changes the HTTPProxy status updating to use the status subresource,
and adds RBAC for Contour to be able to update it.

Having the status subresource enabled is best-practice for CRDs and will be a foundation for further status
work.

It also means that HTTPProxy objects no longer need to include the `status` stanza to be valid.

Refactors the `k8s.UnstructuredConverter` to have FromUnstructured and ToUnstructured methods, and uses ToUnstructured where required.

Updates projectcontour#2495

Signed-off-by: Nick Young <ynick@vmware.com>
youngnick pushed a commit to youngnick/contour that referenced this issue May 7, 2020
This commit enables the status subresource for HTTPProxy.
This means that:
- normal HTTPProxy changes that include updates to the `status` stanza will have the status updates dropped
- updates to `status` can only be made using requests to the status subresource.

Also changes the HTTPProxy status updating to use the status subresource,
and adds RBAC for Contour to be able to update it.

Having the status subresource enabled is best-practice for CRDs and will be a foundation for further status
work.

It also means that HTTPProxy objects no longer need to include the `status` stanza to be valid.

Refactors the `k8s.UnstructuredConverter` to have FromUnstructured and ToUnstructured methods, and uses ToUnstructured where required.

Updates projectcontour#2495

Signed-off-by: Nick Young <ynick@vmware.com>
youngnick pushed a commit that referenced this issue May 7, 2020
This commit enables the status subresource for HTTPProxy.
This means that:
- normal HTTPProxy changes that include updates to the `status` stanza will have the status updates dropped
- updates to `status` can only be made using requests to the status subresource.

Also changes the HTTPProxy status updating to use the status subresource,
and adds RBAC for Contour to be able to update it.

Having the status subresource enabled is best-practice for CRDs and will be a foundation for further status
work.

It also means that HTTPProxy objects no longer need to include the `status` stanza to be valid.

Refactors the `k8s.UnstructuredConverter` to have FromUnstructured and ToUnstructured methods, and uses ToUnstructured where required.

Updates #2495

Signed-off-by: Nick Young <ynick@vmware.com>
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 <ynick@vmware.com>
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 <ynick@vmware.com>
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 <ynick@vmware.com>
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 <ynick@vmware.com>
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 <ynick@vmware.com>
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 <ynick@vmware.com>
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 <ynick@vmware.com>
youngnick pushed a commit to youngnick/contour that referenced this issue Jun 30, 2020
Updates projectcontour#2495.

Signed-off-by: Nick Young <ynick@vmware.com>
youngnick pushed a commit to youngnick/contour that referenced this issue Jul 15, 2020
Updates projectcontour#2495.

Signed-off-by: Nick Young <ynick@vmware.com>
@stevesloka
Copy link
Member

@youngnick what were the thoughts around the basic info bits? I was looking to add things like fqdn, path, headers, etc to all proxies giving users a way to see what's been included.

Poking around a bit we need to add an Info section to the DetailedCondition.

Something like:

// Infos contains a slice of relevant info subconditions for this object.
// +optional
Infos []SubCondition `json:"infos,omitempty"`

Then if we re-use the same SubConcondition struct, we'd get something like this:

status:
  conditions:
  - infos:
    - message: baremetal.slokalabs.io.  # <--------------- NEW!
      reason: info
      status: "True"
      type: fqdn
    lastTransitionTime: "2020-11-16T21:29:19Z"
    message: Valid HTTPProxy
    observedGeneration: 2
    reason: Valid
    status: "True"
    type: Valid
  currentStatus: valid
  description: Valid HTTPProxy
  loadBalancer: {}

I'd like to agree on what we thing we should call all this bits. In this example I named it Infos since we have Errors and Warnings and couldn't think of a better name.

(@skriss )

stevesloka added a commit to stevesloka/contour that referenced this issue Nov 17, 2020
Adds a details subcodition array to the DetailedCondition allowing
for more information to be exposed to users via an HTTPProxy
Conditions struct. Things like fqdn, included paths or headers
can be written to child proxies allowing users to understand
what has been included to them from a parent resource.

Updates projectcontour#2495

Signed-off-by: Steve Sloka <slokas@vmware.com>
stevesloka added a commit to stevesloka/contour that referenced this issue Nov 17, 2020
Adds a details subcodition array to the DetailedCondition allowing
for more information to be exposed to users via an HTTPProxy
Conditions struct. Things like fqdn, included paths or headers
can be written to child proxies allowing users to understand
what has been included to them from a parent resource.

Updates projectcontour#2495

Signed-off-by: Steve Sloka <slokas@vmware.com>
stevesloka added a commit to stevesloka/contour that referenced this issue Nov 17, 2020
Adds a details subcodition array to the DetailedCondition allowing
for more information to be exposed to users via an HTTPProxy
Conditions struct. Things like fqdn, included paths or headers
can be written to child proxies allowing users to understand
what has been included to them from a parent resource.

Updates projectcontour#2495

Signed-off-by: Steve Sloka <slokas@vmware.com>
@jpeach
Copy link
Contributor

jpeach commented Nov 17, 2020

@youngnick what were the thoughts around the basic info bits? I was looking to add things like fqdn, path, headers, etc to all proxies giving users a way to see what's been included.

Why would you not just add these fields directly to status?

@stevesloka
Copy link
Member

stevesloka commented Nov 17, 2020

Makes sense to me @jpeach, was just trying to follow the idea of the conditions spec idea.

Following the thread of just adding to the status struct could have something like this:

// HTTPProxyStatus reports the current state of the HTTPProxy.
type HTTPProxyStatus struct {
  ...
  <existing> 
   ...
  
  FQDN []string
  Routes []RouteStatus 
}

// RouteStatus represents the status of fully qualified 
// conditions that might be included to this HTTPProxy
type RouteStatus struct {
  MatchConditions []MatchCondition
}

@stevesloka
Copy link
Member

Other bits of information like who my parent is, might be interesting to follow the chain of inclusion around.

@youngnick
Copy link
Member Author

We'll talk about this more, but my initial thoughts were:

  • whatever we do, it has to be namespaced by root fqdn, and then show the full set of conditions, since a HTTPProxy can be included multiple times, either under different roots or the same root.
  • I think that Conditions is actually not a good fit for this information - it's better as a field of some sort.
  • Showing who the parent is would also be useful.

youngnick pushed a commit to youngnick/contour that referenced this issue Nov 26, 2020
This commit removes all trace of the `contour.heptio.com` annotations,
and updates a few `heptio` references I found as well.

Closes projectcontour#2495.

Signed-off-by: Nick Young <ynick@vmware.com>
youngnick pushed a commit to youngnick/contour that referenced this issue Nov 26, 2020
This commit removes all trace of the `contour.heptio.com` annotations,
and updates a few `heptio` references I found as well.

This support has been deprecated since Contour v1.8.0 in September.

Closes projectcontour#2495.

Signed-off-by: Nick Young <ynick@vmware.com>
youngnick pushed a commit to youngnick/contour that referenced this issue Nov 26, 2020
This commit removes all trace of the `contour.heptio.com` annotations,
and updates a few `heptio` references I found as well.

This support has been deprecated since Contour v1.8.0 in September.

Closes projectcontour#2495.

Signed-off-by: Nick Young <ynick@vmware.com>
Contour Project Board automation moved this from In progress to 1.11 Release Nov 30, 2020
@youngnick youngnick reopened this Nov 30, 2020
Contour Project Board automation moved this from 1.11 Release to In progress Nov 30, 2020
@youngnick
Copy link
Member Author

Closed by mistake, my bad.

Copy link

The Contour project currently lacks enough contributors to adequately respond to all Issues.

This bot triages Issues according to the following rules:

  • After 60d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, the Issue is closed

You can:

  • Mark this Issue as fresh by commenting
  • Close this Issue
  • Offer to help out with triage

Please send feedback to the #contour channel in the Kubernetes Slack

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 14, 2024
Copy link

The Contour project currently lacks enough contributors to adequately respond to all Issues.

This bot triages Issues according to the following rules:

  • After 60d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, the Issue is closed

You can:

  • Mark this Issue as fresh by commenting
  • Close this Issue
  • Offer to help out with triage

Please send feedback to the #contour channel in the Kubernetes Slack

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Feb 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/status Issues or PRs related to exposing status for APIs. blocked/needs-design Categorizes the issue or PR as blocked because it needs a design document. kind/design Categorizes issue or PR as related to design. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.
Projects
No open projects
Development

No branches or pull requests

4 participants