Crossplane: Stack Install does not update existing objects

[srueg]

What happened?

I installed a stack for which the CRDs already existed on the cluster. Since the stack install job ignores objects that already exist in the cluster, the existing CRDs were not updated: https://github.com/crossplane/crossplane/blob/b1f4a1e05da6973c13adee50a200a338b24e6d06/pkg/controller/stacks/install/installjob.go#L271
Unfortunately the existing CRDs were missing the correct labels (parent labels) which in turn led to the stack-manager not being able to discover them and failing with the following error:

stack-manager   failed to create RBAC permissions   {"controller": "stacks/stack.stacks.crossplane.io", "error": "failed to list all expected crds"

The reason that the stack CRDs already exist is, that we manage the whole cluster with Argo CD. If Argo CD doesn't know about the stack CRDs (i.e. it only creates the StackInstall) we observed race conditions, where Argo CD couldn't create the CRs because the CRD wasn't ready. It then also won't retry the sync because it doesn't know the CRD was created by the stack install. Our solution for this is to create the stack CRDs with Argo CD.
I'm also happy to discuss other approaches so that we don't have to manage the CRDs with Argo.

How can we reproduce it?

If the CRDs for a stack are already present in a cluster but missing the correct labels, the stack install job won't touch these existing CRDs.

What environment did it happen in?

Crossplane version: v0.8.0
Cloud provider: https://cloudscale.ch
Kubernetes version: v1.15.3
Kubernetes distribution: Rancher RKE
Stack: https://github.com/vshn/stack-cloudscale

[muvaf]

I think the reason it works this way is about ownership of the CRDs. The controller assumes that there will be one and only one owner(*StackInstall object) of the CRDs that the stack. This is especially useful for deletion of the stack where the look-up is done via labels. If the object is created by some other entity, it doesn't label it because then the delete logic could result in deletion of CRDs that this specific *StackInstall wasn't an owner of. @displague probably knows more.

But I think we should allow more flexibility in this design to accommodate the case where the CRD already exists in the cluster. @displague do you think we have to do the deletion through labels? I'm imagining that we already have the list of CRDs on Stack object, we can just go through that list and delete one by one. Then we'd be able to update the CRDs that already exist in the cluster with the parent labels because at that point those labels are merely for UI purposes, not part of any logic inside Crossplane.

[displague]

I agree that the Stack Manager should not prevent this workflow and I see how this workflow is valuable.

I propose that the Stack Manager should continue to delete Stacks labeled under its ownership (however this will be getting some rework in #1051), but the Stack Manager should not search for all CRDs by parent label. Rather, the listing of the needed CRDs could be done by field filters (apiVersion and kind). Existing CRDs will pass this check and will not be modified with parent labels nor with UI annotations. Since they will not have the parent labels, they will not be deleted on Stack/StackInstall delete, which I think is desirable in this setting.

I believe it is useful to verify that all CRDs are accounted for before starting the Stack controller because the stack controller may not fail in an easily detectable way when the CRDs are missing.

The warning, "failed to list all expected crds", is possible under normal conditions, while the Stack and CRD are being processed. This message could be friendlier, something like: "waiting for Stack required CRDs: foo, bar".

[jbw976]

@displague is this fixed now with #1311?

[displague]

@srueg You should find that this has been addressed in the latest Crossplane master, which will be released soon as Crossplane 0.9.0.

Specifically, so long as a Stack's CRDs exists in the cluster the requirements to start the controller will have been met. Removing the StackInstall for such a Stack will not affect the CRD. This CRD will not be managed (or labeled) by Crossplane in any way.

If the CRD contains the app.kubernetes.io/managed-by: stack-manager label, which the stack unpack process adds, then the Stack Manager will manage the CRD and delete it when no StackInstalls are using it. Make sure your Argo managed CRD does not have this label.

[srueg]

Very cool, thanks! I'll let you know once I get around to test it