PingSource dataplane lifecycle is tied to the controller lifecycle.

[n3wscott]

Setting the owner ref to the controller deployment seems like a bug to me, this means that the dataplane and control plane lifecycles are tied together and an upgrade of the control plane takes down the dataplane.

Originally posted by @n3wscott in https://github.com/knative/eventing/pull/3040/files

[n3wscott]

mattmoor(opens in new tab):ducktagon: Today at 9:55 AM
I just noticed the CONTROLLER_{UID,NAME} in ping source. There is a nasty bug here that likely manifests from a misunderstanding of the downward API. 🧵

5 replies

mattmoor(opens in new tab):ducktagon: 1 minute ago
These env vars are consumed here:

eventing/pkg/reconciler/pingsource/resources/mt_receive_adapter.go

Lines 59 to 62 in 004ebd4

	APIVersion: v1.SchemeGroupVersion.String(),
	Kind: "Deployment",
	Name: os.Getenv("CONTROLLER_NAME"), // guarantee to be non-empty
	UID: types.UID(os.Getenv("CONTROLLER_UID")), // guarantee to be non-empty

pkg/reconciler/pingsource/resources/mt_receive_adapter.go:59-62(opens in new tab)
APIVersion: v1.SchemeGroupVersion.String(),
Kind: "Deployment",
Name: os.Getenv("CONTROLLER_NAME"), // guarantee to be non-empty
UID: types.UID(os.Getenv("CONTROLLER_UID")), // guarantee to be non-empty
https://github.com/knative/eventing|knative/eventingknative/eventing | Added by GitHub(opens in new tab)

mattmoor(opens in new tab):ducktagon: 1 minute ago
Note that the kind is Deployment

mattmoor(opens in new tab):ducktagon: 1 minute ago
These are set here:

eventing/config/core/deployments/controller.yaml

Lines 50 to 57 in 004ebd4

	- name: CONTROLLER_NAME
	valueFrom:
	fieldRef:
	fieldPath: metadata.name
	- name: CONTROLLER_UID
	valueFrom:
	fieldRef:
	fieldPath: metadata.uid

config/core/deployments/controller.yaml:50-57(opens in new tab)
- name: CONTROLLER_NAME
valueFrom:
fieldRef:
fieldPath: metadata.name
- name: CONTROLLER_UID
Show more
https://github.com/knative/eventing|knative/eventingknative/eventing | Added by GitHub(opens in new tab)

mattmoor(opens in new tab):ducktagon: < 1 minute ago
The downward API will project the Pod name and UID into these properties

[grantr]

Seems like this is fixed by #3128. Reopen if I'm misunderstanding.