Allow disabling the manifests generation for K8s/OpenShift/Knative

[Sgitario]

To me, it makes sense to separately enable/disable the generated resources by extension (kubernetes, knative, openshift, ...). We could disable the manifests generation for Kubernetes, but not for OpenShift for example.

Fix #34094

[Sgitario]

cc @metacosm @maxandersen

[metacosm]

I tend to think that there should be only one switch for all the flavors to keep things simple but also because I tend to think that if you're not interested in generating manifests, you probably don't care about generating manifests for any flavors.

[Sgitario]

I tend to think that there should be only one switch for all the flavors to keep things simple but also because I tend to think that if you're not interested in generating manifests, you probably don't care about generating manifests for any flavors.

There is something you mentioned that I agree with you: if you use the Quarkus Kubernetes extension to generate the Kubernetes, Knative and Openshift manifests (this is controlled by the deployment target property), then you should enable/disable the generation of these properties using the same 'quarkus.kubernetes.enabled' property. I will update the pull request tomorrow morning.

Other thing is when you use the Quarkus Openshift extension, then you should be able to enable/disable the Openshift manifests using 'quarkus.openshift.enabled' property.

[metacosm]

Other thing is when you use the Quarkus Openshift extension, then you should be able to enable/disable the Openshift manifests using 'quarkus.openshift.enabled' property.

That's a good point. Maybe the switch should be named differently altogether, like quarkus.kubernetes-manifest-generation.enabled? I guess it also depends on the extent of what's being disabled. @iocanel mentioned, and I agree with him, that the disabling should extend to processing, not simply manifest generation. While I agree with you that it would be more natural to use quarkus.openshift.enabled when using the OpenShift extension, I also think that it wouldn't be too weird to use quarkus.kubernetes.enabled either. That said, we do already "duplicate" all the properties to use the openshift part so 🤷🏼

[Sgitario]

That's a good point. Maybe the switch should be named differently altogether, like quarkus.kubernetes-manifest-generation.enabled?

I would like to avoid adding a new extra property (apart from the enabled ones) unless it's extremely necessary.

While I agree with you that it would be more natural to use quarkus.openshift.enabled when using the OpenShift extension, I also think that it wouldn't be too weird to use quarkus.kubernetes.enabled either. That said, we do already "duplicate" all the properties to use the openshift part so 🤷🏼

I've just updated the pull request. Now, the enabled flag is purely per extension, so we have two new properties:

• quarkus.kubernetes.enabled to enable/disable all the manifests that are owned by this extension (knative, kubernetes, kind, minikube, openshift when is used without the Quarkus OpenShift extension...)
• quarkus.openshift.enabled to enable/disable the Openshift manifests when having the Quarkus OpenShift extension in place.

Note that when #34025 is done, the OpenShift properties will default to the Kubernetes properties, so you will ultimately get the desired behavior you were expecting at.

[quarkus-bot]

✔️ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

[iocanel]

As discussed in @metacosm PR on diabling manifest generation and on chat. I favor the approach of completely disabling the @buildsteps of the processor with the the use of onlyIf or onlyIfNot.

Even if the processors do check a config property and exit right away, they still request the build items, like ports, rbac, jobs, decorators and pretty much everything declared under the kubenretes SPI, influencing the order of execution.

Does it make a huge difference? Maybe not performance wise, but does make a huge difference maintainance wise.
Some examples:

• the FeatureBuildItem is still produced, can you tell what its impact?
• container image build items are still produced (possibly enabling / disabling image building and pushing)
  ... and so on.

We shouldn't have to worry about such issues. Everything should be cleanly disabled and no buildstep related to kubernetes should run directly or indirectly.

Last but not least this (disabling the buildsteps) is a pattern we already use and I don't see why we should do something else here, especially since we've already discussed it.

Last regarding the dileme on wether we should disalbe things globally, per extension or per deployment target, the answer is definitely globally. Everything else is too hard to explain to the user and may possibly have sideeffects. Users that need to have fine grained control over what's generated can use deployment-target.

[iocanel]

added comment above

[Sgitario]

As discussed in @metacosm PR on diabling manifest generation and on chat. I favor the approach of completely disabling the @BuildSteps of the processor with the use of onlyIf or onlyIfNot.

I was unaware we could use the config objects in @BuildSteps, so I'm +100 of using it.

Last regarding the dileme on wether we should disalbe things globally, per extension or per deployment target, the answer is definitely globally.

I can't entirely agree with this since from my point of view all the extensions that have their own configuration should be independent of each other (mainly speaking about openshift, and kubernetes).

Since we again see things differently, I'm closing this in favor of what you and the rest of the Quarkus team agree with.

[iocanel]

Last regarding the dileme on wether we should disalbe things globally, per extension or per deployment target, the answer is definitely globally.

I can't entirely agree with this since from my point of view all the extensions that have their own configuration should be independent of each other (mainly speaking about openshift, and kubernetes).

We already have global options in the kubernetes configuration (see quarkus.kubernetes.deployment-target). This is no different, but we maybe need a more descriptive name. I could see something like quarkus.kubernetes.manifest-generation working.

[iocanel]

Since we again see things differently, I'm closing this in favor of what you and the rest of the Quarkus team agree with.

@Sgitario: Maybe written speech does not really help in this case, but this reads to me as something written by someone upset. I can almost hear the door slamming behind your back. 😄

My intention was not to irritate you, and apologies if in anyway I did.

So, please let me ellaborate on my comment (hoping that understanding my POV will smooth things out):

There are three ways of approaching this:

• global manifest generation flag (my suggestion)
• per deployment target (we both agree that this is covered by quarkus.kubernetes.deployment-target)
• per extension (your suggestion)

The problem with the per extension approach is that OpenshiftConfig does not live in quarkus-openshift but in quarkus-kubernetes. So, if we add there something like quarkus.openshift.enabled we will end up with a property that behaves diferently depending the extension one uses:

• from quarkus-openshift -> disables manifest generation completely.
• from quarkus-kubernetes -> ???

So, we end up with a case that is not really obvious. Regardless of the behavior we choose for quarkus-kubernetes we will have to document different cases to explain how things work and that is strong indication to me that the user experience is suboptimal. If its hard to explain / document, we need to rethink it.

So, please don't get upset, we both want the same thing (the best user experience possible) we just have different perspectives.
And yes, I understand that it can be tiresome when we so often don't see things eye to eye. I just want you to know that there is nothing personal. On the contrary, I am really greatful for all the great work you are doing. ❤️

[Sgitario]

Since we again see things differently, I'm closing this in favor of what you and the rest of the Quarkus team agree with.

@Sgitario: Maybe written speech does not really help in this case, but this reads to me as something written by someone upset. I can almost hear the door slamming behind your back. smile

haha, not at all. But in the end, when there are different views, it's your call (I don't mean you directly, but also the rest of the Quarkus team) to proceed with what you think it's for the better.

So, please let me ellaborate on my comment (hoping that understanding my POV will smooth things out):

There are three ways of approaching this:

• global manifest generation flag (my suggestion)
• per deployment target (we both agree that this is covered by quarkus.kubernetes.deployment-target)
• per extension (your suggestion)

The problem with the per extension approach is that OpenshiftConfig does not live in quarkus-openshift but in quarkus-kubernetes. So, if we add there something like quarkus.openshift.enabled we will end up with a property that behaves diferently depending the extension one uses:

• from quarkus-openshift -> disables manifest generation completely.
• from quarkus-kubernetes -> ???

So, we end up with a case that is not really obvious. Regardless of the behavior we choose for quarkus-kubernetes we will have to document different cases to explain how things work and that is strong indication to me that the user experience is suboptimal. If its hard to explain / document, we need to rethink it.

I completely agree with the last statement If its hard to explain / document, we need to rethink it and this is precisely my point to do it per extension: users that add the quarkus-openshift extension would expect to configure things using quarkus.openshift.xxx properties as they are already doing (that OpenShiftConfig lives in quarkus-kubernetes from my point of view is an implementation detail).
Note that after #34487 is merged, the default value of quarkus.openshift.enabled would be quarkus.kubernetes.enabled. At the moment, when the OpenShift extension is added, both K8s and OpenShift manifests are generated, so users would still have the freedom to disable the Kubernetes manifests generation doing quarkus.kubernetes.enabled=false but not the OpenShift manifests quarkus.openshift.enabled=true.

This is slightly different when using the quarkus.kubernetes.deployment-target property, because in this case, this property lies on the Kubernetes extension, so regardless if you generate knative or openshift, it's governed by quarkus.kubernetes.enabled only.

[iocanel]

I completely agree with the last statement If its hard to explain / document, we need to rethink it and this is precisely my point to do it per extension: users that add the quarkus-openshift extension would expect to configure things using quarkus.openshift.xxx properties as they are already doing.

@Sgitario: but what would happen if the property above is used with quarkus-kubernetes?