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

[kingdonb]

#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]

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

[kingdonb]

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

fluxcd/flux2#1673 (comment)

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]

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)

[stefanprodan]

We look for deployment labels https://github.com/fluxcd/flux2/blob/78f4dfa48d851ae51324af6adc6e150449c171a9/cmd/flux/logs.go#L137

[chanwit]

As @stefanprodan pointed out, I think this patch is good enough. Thank you @kingdonb.

[stefanprodan]

@chanwit for this to work, the namespace where OLM deploys Flux must be flux-system, does this happens or the namespace is randomly chosen?

[chanwit]

flux-system is annotated as the recommended namespace (the default value) when users click installing.
However, they still can choose other namespaces if they'd like to.

My solution is to document that user must install it only to the flux-system namespace.

[stefanprodan]

When you run flux install --export the app.kubernetes.io/instance: flux-system is added to all deployments, why is OLM removing that? Or @kingdonb maybe installed flux in a different namespace than flux-system?

[chanwit]

Just found that all Deployment got rewritten into this form:
https://github.com/weaveworks/flux2-openshift/blob/main/flux/0.16.1/manifests/flux.v0.16.1.clusterserviceversion.yaml#L522

[stefanprodan]

FFS... so OLM doesn't allow us to set metadata for our own deployments?

[stefanprodan]

I closed this PR as it does nothing, the label was already there, but it's removed by OLM.

[chanwit]

Labels could be specified there on each StrategyDeploymentSpec:
https://pkg.go.dev/github.com/operator-framework/api/pkg/operators/v1alpha1#StrategyDeploymentSpec

I'll tweak the generator to do so.

[stefanprodan]

@chanwit great find, this will definitely solve our issues.