Add app.kubernetes.io/instance label for flux logs

kingdonb commented 3 years ago

~weaveworks/flux2-openshift#4~ not related, I have mixed myself up, #4 was not meant to be about the logs issue.

(I do not quite know how to test this, but I have a feeling it's not going to work)

kingdonb commented 3 years ago

Running through the build process by hand, I can see the labels aren't missing at the point that kustomize build gets called.

kingdonb commented 3 years ago

Stefan suggested we add the label in the location where we are already patching Flux deployments:

https://github.com/fluxcd/flux2/issues/1673#issuecomment-891714936

Here's my understanding of how the release process works, (and why that will not work) there are some parts I didn't quite understand trying to pick it apart, but I think I have worked most of it out:

release.sh pulls a copy of gotk-components.yaml from the flux CLI
passes it off to kustomize build, where the fsGroup is removed via patch (at this point, the expected labels are still present)
release.js massages this into the bundle-directory form expected by opm-index.sh – Deployment resources are all gone, replaced by templates/clusterserviceversion.yaml which is templated into the current Flux release as:

const csvFileName = `flux.v${version}.clusterserviceversion.yaml`

Not sure what happens exactly next,
operator-sdk bundle validate which I assume can abort the release if validation would fail
Then make opm_index calls opm-index.sh if everything went well so far, which builds OCI format with Docker and pushes the release out to the OpenShift channel?

What is fairly clear is that the Deployment resources are not part of the bundle directory format, they are in the clusterserviceversion, so any labels we might have passed into release.js are apparently discarded. #5 will not work.

ClusterServiceVersion evidently only supports adding labels for the purpose of indicating if support for multiple architectures are present: https://docs.openshift.com/container-platform/4.4/operators/operator_sdk/osdk-generating-csvs.html#olm-enabling-operator-for-multi-arch_osdk-generating-csvs

Is that why you suggest we use an operator to install Flux, @chanwit ? Because it will give us control over the labels, and whatever else we need, (like target namespace?)

I think that sounds over-complicated, the OLM does add some labels, are any of these labels suitable for use by flux cli? (Could we update flux logs to fall back and look for OLM labels if flux logs is called and no pods are selected? Would there be any way to do this without multiple round-trips?)

  labels:
    olm.deployment-spec-hash: 5c69bb97f5
    olm.owner: flux.v0.16.1
    olm.owner.kind: ClusterServiceVersion
    olm.owner.namespace: flux-system
    operators.coreos.com/flux.flux-system: ""

kingdonb commented 3 years ago

Maybe these annotations in the pod spec are more helpful:

olm.operatorGroup: flux-system
olm.operatorNamespace: flux-system

(I just realized we are probably selecting pods, not deployments)