Dynamic pod labels: Spinnaker pod quarantine

[mandarjog]

Spinnaker uses dynamic pod labels to quarantine pods out of a service.
Removing a specific label directly from a pod, removes the endpoint from the service.

This is an example of dynamic pod labels.

Recently we have made several decisions regarding static nature of pod labels.

1. Pod labels are placed in nodemetada and deemed constants for the life of the proxy process.
2. Pilot uses this node metadata to decide ServiceInstances.

a. Investigate the mode of failure for spinnaker like behaviour.
b. Decide if we want to support this mode.

[mandarjog]

cc @douglas-reid @kyessenov @rshriram @howardjohn

[howardjohn]

@hzxuzhonghu

[kyessenov]

Is this on k8s?

[mandarjog]

yes, this is used with k8s and other platforms.

[kyessenov]

on k8s, this is also breaking downwards API.

[howardjohn]

I think you cannot get labels in `env` using downwards API - probably for this reason?

…

On Mon, Sep 23, 2019 at 5:45 PM Kuat ***@***.***> wrote: on k8s, this is also breaking downwards API. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#17332?email_source=notifications&email_token=AAEYGXMXHPUTOM7LCPWAJT3QLFPI5A5CNFSM4IZV6BEKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7MV73Q#issuecomment-534339566>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEYGXI6O3D3OGTB4Z2W32TQLFPI5ANCNFSM4IZV6BEA> .

[kyessenov]

You should be able to query pod labels via Downward API: https://kubernetes.io/docs/tasks/inject-data-application/downward-api-volume-expose-pod-information/#capabilities-of-the-downward-api On Mon, Sep 23, 2019 at 5:48 PM John Howard <notifications@github.com> wrote:

…

I think you cannot get labels in `env` using downwards API - probably for this reason? On Mon, Sep 23, 2019 at 5:45 PM Kuat ***@***.***> wrote: > on k8s, this is also breaking downwards API. > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > < #17332?email_source=notifications&email_token=AAEYGXMXHPUTOM7LCPWAJT3QLFPI5A5CNFSM4IZV6BEKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7MV73Q#issuecomment-534339566 >, > or mute the thread > < https://github.com/notifications/unsubscribe-auth/AAEYGXI6O3D3OGTB4Z2W32TQLFPI5ANCNFSM4IZV6BEA > > . > — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#17332?email_source=notifications&email_token=ACIYRRSHFD42LYVFFTXEEUTQLFPW5A5CNFSM4IZV6BEKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7MWFIY#issuecomment-534340259>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACIYRRRSAVEU6O3ULKDRQZTQLFPW5ANCNFSM4IZV6BEA> .

[hzxuzhonghu]

We can use downward api volume mount the labels to sidecar, and watch the file changes, restart envoy when it changes. This is same as And when we change labels in deployment, the pod will recreate too.

I am not sure if it is acceptable in this case.

[rshriram]

I think that is fine. And given that its not a common thing to do, its okay to do the envoy restart thing, given that the other 90% of users would benefit greatly from higher decoupling from k8s, lesser reliance on k8s api, as well as the reduced impact of eventual consistency. It seems like a worthwhile tradeoff for usability. What do you guys think?

[rshriram]

Ofcourse, if it turns out that 50% of the planet is using Spinnaker in production and that they all rely a LOT on this dynamic label thing, then we have to rethink. May be one option is to have some flag in Pilot that allows these people to choose the old style mode of always going to k8s for the pod labels.

[douglas-reid]

Having the labels be considered consts for the lifetime of the proxy was a known compromise to support the simple use case without adding a whole bunch of complexity.

What is the Envoy story around support for dynamic metadata? I know that we could add a filter that could run for the extensions use case, but that won't help Pilot.

[kyessenov]

Envoy supports dynamic metadata on listeners, routes, clusters, endpoints, but not on singleton resources such as bootstrap.

We probably want to implement some sort of MetadataDiscoveryService that can subscribe to metadata and update node-level metadata with it.

[mandarjog]

@rshriram i don’t think we can restart envoy, because quarantine is the main use case and we typically want to inspect the proxy and app after quarantine and not restart it.
Perhaps we can directly support quarantine instead of this particular method.

[mandarjog]

I think this is still an issue.

[mandarjog]

Not stale

[kyessenov]

Not stale.

[hzxuzhonghu]

We probably want to implement some sort of MetadataDiscoveryService that can subscribe to metadata and update node-level metadata with it.

As we have xds proxy now, i think this is easier now. I can dig into it.

[mandarjog]

I think we have multiple use cases. The quarantine main use case is to remove the it from receiving traffic. We want the outbound config to be unaffected.
Inbound policies should apply just like before if the pod happens to receive traffic. Perhaps even start drain.

we want the outbound traffic to work as normal if the pod needs to reach outside.

[kyessenov]

We don't have restart envoy when labels change. That argues for not having labels in the bootstrap to begin with which is a radical change from today. An interesting case is that pods can re-assign their canonical services as well.

[howardjohn]

dynamic change was implemented a while back