[fluxv2] Add defaultUpgradePolicy in the Fluxv2

[antgamdia]

Description:

As per the offline discussion we had, a short-term solution for allowing passing the version selection in Flux v2 would be just allowing the Fluxv2 to be configured with a plugin-wide config as we were doing in Carvel (see #4346 (comment))

Later on, we can start discussing long-term solutions like #4365, but it is not a priority right now, as the community's feedback is that this version selection is not extensively being used out there, so just adding a plugin-wide option would be enough for now.

[gfichtenholt]

Hi, I have a follow-up question:
Flux plugin implements a core API, like CreateInstalledPackage, where the caller may pass, among other things, a version reference. ref https://github.com/kubeapps/kubeapps/blob/90f135c82868a9c2599469eef9e3a0b03ac718d3/cmd/kubeapps-apis/proto/kubeappsapis/core/packages/v1alpha1/packages.proto#L826.
Let's say for the sake of argument the caller sends in a version reference that is not a specific version but rather looks like a semver expression: ">5 <9". That is a perfectly legal version semver expression. Let's also suppose the defaultUpgradePolicy for the plugin happens to be set to "minor". Based on looking at what carvel plugin currently does and what we'd expect from flux I think the result persisted in the CR will be something like ">=5.0.0 <6.0.0" which is very different from what the caller intended. If I have misunderstood something please point it out. Thanks

[absoludity]

Curerntly the Kubeapps UI does not allow specifying a semver expression at all - how we deal with that when it does is more related to #4346 . Rather, the UX currently always sends the exact version. The carvel plugin currently assumes this and uses it together with the defaultUpgradePolicy to set the result in the CR (as in versionConstraintWithUpgradePolicy )

I think it's fine for the flux plugin to also assume this for now (ie. that it will currently receive a specific version). I realise you're not thinking about the Kubeapps UX specifically, so are thinking what it should do as an API right now, but that's exactly what we're currently deferring to #4346, AIUI. In fact, I'd say feel free to respond with a bad request, or unimplemented, if it's not, just to be explicit about this assumption in the code, until we work on #4346 .

That's my understanding. Is that what you would expect @antgamdia ?

[gfichtenholt]

Wow. Was very careful avoid any mention of current UI. I have a general purpose server that implements an API. Current kubeapps UI is just one caller. My question was really about the API semantics.
Can't really respond with an error, I have integration tests (i.e. another kind of client) that rely on this feature to test automatic flux upgrades, though I suppose I could now re-write these tests to use the defaultUpgradePolicy feature to do what I need...

[absoludity]

Wow. Was very careful avoid any mention of current UI. I have a general purpose server that implements an API. Current kubeapps UI is just one caller. My question was really about the API semantics.

Yeah, I realise. The kubeapps-apis service has been designed so it can be used by other clients, but we do try to be pragmatic about ensuring we support the current Kubeapps UX as a priority.

Can't really respond with an error, I have integration tests (i.e. another kind of client) that rely on this feature to test automatic flux upgrades, though I suppose I could now re-write these tests to use the defaultUpgradePolicy feature to do what I need...

That's fine - I just meant you could do that if it's easier for you to focus on what we need now. But if you've already written integration tests where the received version reference can be an expression, I don't see an issue with it. In that case, I'd assume that if the version expression received by the server is already an expression, then you use it as is. The defaultUpgradePolicy is only relevant while our main client doesn't yet expose expressions, nor is it a priority to do so right now based on recent community feedback.

[gfichtenholt]

Oh ok. Would it be acceptable then if I see on the server side that the requested version happens to be a semver expression to ignore the default Upgrade policy in that case?

[absoludity]

Yes, exactly.

[gfichtenholt]

Yes, exactly.

👍