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

Canary grouping must take labels into account #4170

Closed
kflynn opened this issue Mar 17, 2022 · 0 comments
Closed

Canary grouping must take labels into account #4170

kflynn opened this issue Mar 17, 2022 · 0 comments

Comments

@kflynn
Copy link
Member

kflynn commented Mar 17, 2022

The following configuration attempts to use labels to associate the quote-backend-foo
Mapping only with foo.example.com, and quote-backend-bar only with bar.example.com:

---
apiVersion: getambassador.io/v3alpha1
kind: Host
metadata:
  name: foo-host
spec:
  hostname: foo.example.com
  mappingSelector:
    matchLabels:
      host: foo
  ...
---
apiVersion: getambassador.io/v3alpha1
kind: Host
metadata:
  name: bar-host
spec:
  hostname: bar.example.com
  mappingSelector:
    matchLabels:
      host: bar
  ...
---
apiVersion: getambassador.io/v3alpha1
kind: Mapping
metadata:
  name: quote-backend-foo
  labels:
    host: foo
spec:
  prefix: /test/
  service: quote
---
apiVersion: getambassador.io/v3alpha1
kind: Mapping
metadata:
  name: quote-backend-bar
  labels:
    host: bar
spec:
  prefix: /test/
  service: quote

Since the two Mappings have different host labels, they will associate with different Hosts
and should not be placed in the same canary group. However, as of Emissary 2.2.2, the canary-group
logic doesn't pay attention to labels, so the two Mappings will (wrongly) be placed in the same
canary group. Since Envoy-config generation operates on groups rather than on individual Mappings,
the effect here is that https://foo.example.com/test/ will be correctly routed, but
https://bar.example.com/test/ will 404.

As an additional constraint, consider the same Hosts above, but these two Mappings:

---
apiVersion: getambassador.io/v3alpha1
kind: Mapping
metadata:
  name: quote-labels-1
  labels:
    host: foo
    irrelevant-label: "1"
spec:
  prefix: /labels/
  service: quote-1
---
apiVersion: getambassador.io/v3alpha1
kind: Mapping
metadata:
  name: quote-labels-2
  labels:
    host: foo
    irrelevant-label: "2"
spec:
  prefix: /labels/
  service: quote-2

These Mappings should be placed in the same canary group, since the Hosts don't differentiate
based on the irrelevant-label label. So we mustn't look at every label -- only the ones that
the Hosts actually care about matter.

To work around this, you can either:

  1. Give the two Mappings a precedence with different values so that they can't be canaried together, or
  2. Use the hostname rather than labels.
haq204 pushed a commit that referenced this issue Aug 10, 2023
If two mappings have different host labels but share the same prefix, then they should not be placed in the same canary group. Previously, labels were not taken into account when grouping mappings into canary groups, which caused mappings sharing the same prefix to be part of the same canary group when using host labels (hostname or host field was not specified in the mapping).

In order to solve this, we update the mapping group_id logic to incorporate host selector labels. This means that the mappings are grouped based on the following criteria:
- HTTP method
- Prefix
- Headers
- Query parameters
- Host labels
- Precedence

Because this affects only how mappings are grouped, the following behavior is retained:
- hostname, host, and :authority header fields take precendence over host labels for the purposes of grouping.
- If hostname, host, and/or :authority header fields are specified along with host labels, then they must match before they can be associated.
- A canary group can be associated with multiple Hosts based on selector labels.

Fixes #4170.

Signed-off-by: Hamzah Qudsi <hqudsi@datawire.io>
@haq204 haq204 closed this as completed in eef86a4 Aug 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant