Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Flux CRDs in OLM ClusterServiceVersion #61

Open
blezoray opened this issue Jul 2, 2024 · 22 comments · May be fixed by #62
Open

Add Flux CRDs in OLM ClusterServiceVersion #61

blezoray opened this issue Jul 2, 2024 · 22 comments · May be fixed by #62
Labels
help wanted Extra attention is needed

Comments

@blezoray
Copy link

blezoray commented Jul 2, 2024

Hello,

With Openshift (4.14), in release 0.6.0, flux-operator ClusterServiceVersion contains only FluxInstanceand FluxReport CRDs that it manages.

But, for endusers, the console shows only these 2 kind of resources:
image

In order to let the console showing endusers flux resources, like GitRepository ..., all the flux CRDs must be added to the ClusterServiceVersion even if the flux-operator controller doesn't manage them.

Could you add them to the CSV ?

BR.

@stefanprodan
Copy link
Member

Why would anyone create Flux resources from a UI? All of these should be in Git

@blezoray
Copy link
Author

blezoray commented Jul 2, 2024

A good UI permits to quickly test and discover a solution. It also permits to see all their owned resources, their status, the resources dependencies.
Example with Strimzi

image

@stefanprodan
Copy link
Member

stefanprodan commented Jul 2, 2024

Ok so this is about a readonly view, I get it now.

Does OLM allows adding CRDs to the owned list even if the operator does not "owns" them? As in, the Flux CRDs are not included in the OLM Bundle, the whole goal of the Flux Operator is to install/upgrade them based on the FluxInstance and the specified Flux version. Also some CRDs may never be installed, for example, you can configure a FluxInstance without helm-controller, so there will be no HelmRelease CRD in the cluster.

@blezoray
Copy link
Author

blezoray commented Jul 2, 2024

Yes, it is the case for External Secrets Operator, based on helm operator SDK. There is the OperatorConfig equivalent to FluxInstance

The consequence is that flux-operator must not install CRDs

@stefanprodan
Copy link
Member

Wouldn't the UI crash if some CRD let's say HelmRelease is listed in the CVS but is not installed in the cluster?

@stefanprodan stefanprodan linked a pull request Jul 3, 2024 that will close this issue
@stefanprodan
Copy link
Member

stefanprodan commented Jul 3, 2024

After adding the CRDs, the bundle no longer builds as seen in #62

Error: error checking provided apis in bundle flux-operator.v0.6.0: couldn't find toolkit.fluxcd.io/v1beta2/Bucket (source) in bundle. found: map[fluxcd.controlplane.io/v1/FluxInstance (fluxinstances):{} fluxcd.controlplane.io/v1/FluxReport (fluxreports):{}]

@stefanprodan
Copy link
Member

Yeah I don't think this can work, Operator SDK wants all CRDs inside the bundle:

$ operator-sdk bundle validate .
ERRO[0000] Error: Value toolkit.fluxcd.io/v1, Kind=GitRepository: owned CRD "toolkit.fluxcd.io/v1, Kind=GitRepository" not found in bundle "flux-operator.v0.6.0" 
ERRO[0000] Error: Value toolkit.fluxcd.io/v1, Kind=HelmRepository: owned CRD "toolkit.fluxcd.io/v1, Kind=HelmRepository" not found in bundle "flux-operator.v0.6.0" 
ERRO[0000] Error: Value toolkit.fluxcd.io/v1, Kind=HelmChart: owned CRD "toolkit.fluxcd.io/v1, Kind=HelmChart" not found in bundle "flux-operator.v0.6.0" 
ERRO[0000] Error: Value toolkit.fluxcd.io/v1beta2, Kind=Bucket: owned CRD "toolkit.fluxcd.io/v1beta2, Kind=Bucket" not found in bundle "flux-operator.v0.6.0" 
ERRO[0000] Error: Value toolkit.fluxcd.io/v1beta2, Kind=OCIRepository: owned CRD "toolkit.fluxcd.io/v1beta2, Kind=OCIRepository" not found in bundle "flux-operator.v0.6.0" 
ERRO[0000] Error: Value toolkit.fluxcd.io/v1, Kind=Kustomization: owned CRD "toolkit.fluxcd.io/v1, Kind=Kustomization" not found in bundle "flux-operator.v0.6.0" 
ERRO[0000] Error: Value toolkit.fluxcd.io/v2, Kind=HelmRelease: owned CRD "toolkit.fluxcd.io/v2, Kind=HelmRelease" not found in bundle "flux-operator.v0.6.0" 
ERRO[0000] Error: Value toolkit.fluxcd.io/v1beta3, Kind=Provider: owned CRD "toolkit.fluxcd.io/v1beta3, Kind=Provider" not found in bundle "flux-operator.v0.6.0" 
ERRO[0000] Error: Value toolkit.fluxcd.io/v1beta3, Kind=Alert: owned CRD "toolkit.fluxcd.io/v1beta3, Kind=Alert" not found in bundle "flux-operator.v0.6.0" 
ERRO[0000] Error: Value toolkit.fluxcd.io/v1, Kind=Receiver: owned CRD "toolkit.fluxcd.io/v1, Kind=Receiver" not found in bundle "flux-operator.v0.6.0" 
ERRO[0000] Error: Value toolkit.fluxcd.io/v1beta2, Kind=ImageRepository: owned CRD "toolkit.fluxcd.io/v1beta2, Kind=ImageRepository" not found in bundle "flux-operator.v0.6.0" 
ERRO[0000] Error: Value toolkit.fluxcd.io/v1beta2, Kind=ImagePolicy: owned CRD "toolkit.fluxcd.io/v1beta2, Kind=ImagePolicy" not found in bundle "flux-operator.v0.6.0" 
ERRO[0000] Error: Value toolkit.fluxcd.io/v1beta2, Kind=ImageUpdateAutomation: owned CRD "toolkit.fluxcd.io/v1beta2, Kind=ImageUpdateAutomation" not found in bundle "flux-operator.v0.6.0" 

@stefanprodan stefanprodan added the help wanted Extra attention is needed label Jul 3, 2024
@blezoray
Copy link
Author

blezoray commented Jul 3, 2024

I think you need to add also the CRD in the manifest directory, like for FluxInstance and FluxReport.

@stefanprodan
Copy link
Member

That's not an option, it would break the whole functionality of Flux Operator.

@blezoray
Copy link
Author

blezoray commented Jul 3, 2024

@stefanprodan
Copy link
Member

stefanprodan commented Jul 3, 2024

Flux is not External Secrets Operator, Flux is composable, for example on a production cluster you would not want the image automation CRDs nor controllers. Also the Flux Operator is meant to install different versions of Flux, which come with different versions of CRDs. If the CRDs are in bundle, then Flux Operator will fight with OLM over those resources.

@blezoray
Copy link
Author

blezoray commented Jul 3, 2024

Except if you have admission or mutating webhook, there is no impact on a CustomResource if there is no controller that manages it. The resource will be created in the ETCD and that's all

@stefanprodan
Copy link
Member

The impact is to Flux, if you set version: v2.2.x and the CRDs are from v2.3 all controllers will crash loop, also the automated upgrade will fail, as OLM will undo the CRDs if Flux Operator upgrades them to let's say a future v2.4

@blezoray
Copy link
Author

blezoray commented Jul 3, 2024

in this case, you should manage different versions of CRDs, v1beta1, v1beta2, ...

@stefanprodan
Copy link
Member

stefanprodan commented Jul 3, 2024

We do, but that's not as easy as it sounds, Kubernetes doesn't allow CRDs downgrade and the default conversion webhook for CRs will bump the version in etcd only at the first mutation. The whole goal of Flux Operator is to manage the CRDs/CRs upgrade for the user, if the CRDs are applied at some version set in OLM we are back where the Weaveworks operator was and I have no intention of doing that.

@blezoray
Copy link
Author

blezoray commented Jul 3, 2024

There is no easy way to manage CRDs and as I know, OLM also doesn't support downgrade: you must remove the current version and install the previous version.

Today, the Weaveworks operator in the community catalog is not configurable at all, typically for multi-tenancy. So, for me the solution is to flux operator with the flux CRDs.

@stefanprodan
Copy link
Member

stefanprodan commented Jul 3, 2024

OLM can't remove a Flux version at all, because finalizers will block it, some managed workloads will be deleted and the whole cluster will be stuck. The Flux Operator knows how to uninstall Flux properly without affecting any workloads deployed on the cluster. So with the operator, if you want to downgrade from a minor to another, you can by simply deleting the FluxInstance and create a new one, while no workloads will be touched. This is possible because the operator manages the CRDs, if we place them in the OLM bundle all breaks.

@stefanprodan
Copy link
Member

Maybe there is some programmatic way of telling OpenShift which CRDs are available, if there is such an API then the operator would call it during the report generation.

@blezoray
Copy link
Author

blezoray commented Jul 4, 2024

Maybe there is some programmatic way of telling OpenShift which CRDs are available, if there is such an API then the operator would call it during the report generation.

I don't think so. the CRDs are statically described and if you change them through the API, OLM would overwrite them.

Is there many changes in the flux CRDs during the last 2 or 3 years that justify you need to manager CRDs at controller level ?

@stefanprodan
Copy link
Member

Is there many changes in the flux CRDs during the last 2 or 3 years that justify you need to manager CRDs at controller level ?

Every single Flux minor comes with changes to CRDs, Flux evolves along with its APIs

@blezoray
Copy link
Author

blezoray commented Jul 4, 2024

Therefore, the operator should have a new version with OLM packaging. I don't see another alternative

@stefanprodan
Copy link
Member

stefanprodan commented Jul 4, 2024

How it is today, without the CRDs listed in the OLM bundle, the operator works as designed. The operator can install, upgrade, downgrade to/from any Flux version. I don't see why I must break this critical functionality just to have some UI status in OpenShift. If Flux Enterprise customers really want the UI listing, then ControlPlane will probably invest in finding a solution, but so far we have no such requests from customers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants