kubeapps icon indicating copy to clipboard operation
kubeapps copied to clipboard

Enable version selection contraints in the UX for Carvel and Flux

Open absoludity opened this issue 3 years ago • 6 comments

Description:

One of the benefits of Carvel and Flux is that they can automatically upgrade your package installation, but we're not currently enabling this by default. When you install a package in Kubeapps (using Carvel or Flux), by default we explicitly set the constraint to the exact selected package version. As a result, the default constraint does not allow any upgrades (even patch releases). As documented, you can change the default upgrade policy for Carvel packages installed via Kubeapps, cluster-wide.

Instead, we may want to

  1. expose the version constraint field, or
  2. present a UX such as:
  • ( ) Install the selected version only (no upgrades)
  • ( ) Install the selected version and any future patches
  • ( ) Install the selected version and any future minor upgrades
  • ( ) Install the selected version and any future upgrades (this option only visible if you have already selected the latest version)

with a sensible default (perhaps future patches).

When the user makes a selection, we'd calculate the version constraint (hmm, or could we defer this to the backend plugin, with a small API change? I like the idea of that better).

Thoughts? (cc @antgamdia , since you've already thought about the constraints and set defaults for Carvel)

absoludity avatar Feb 25 '22 06:02 absoludity

So this would be for the package installation screen, right? Agree, from the UX point of view we could have both 1. and 2. together if we wanted. A dropdown could contain all options, when selecting 1. a textbox might appear to introduce the custom version constraint field. Given how UI-oriented is Kubeapps, I would say that it would be easier for users to just pick one of the options in 2. without losing the possibility to customize with 1..

For Carvel, didn't @antgamdia already add the option through chart values like kubeappsapis.kappController.packages.v1alpha1.defaultUpgradePolicy?

Another question is if we would support both a Kubeapps-wide setting (like Antonio did very well for Carvel) and a per-installation setting?

could we defer this to the backend plugin

+1 IMHO UI should be as business agnostic as possible

A minor side comment on this from the UI design point of view: the user input part of the installation screen (the part above the YAML tabs) might need a re-design? Having three controls "floating" there. I don't mean in the short term of course. image

castelblanque avatar Feb 25 '22 09:02 castelblanque

So this would be for the package installation screen, right?

Yep.

Agree, from the UX point of view we could have both 1. and 2. together if we wanted. A dropdown could contain all options, when selecting 1. a textbox might appear to introduce the custom version constraint field.

We'll need to have both in some shape or form, because we can only store the text version constraint on the actual CR (HelmRelease or PackageInstall), so for example, when editing the installed package, all we have is the text version (unless we do some magic parsing it).

Given how UI-oriented is Kubeapps, I would say that it would be easier for users to just pick one of the options in 2. without losing the possibility to customize with 1..

Yeah - I agree, with the caveat that by enabling users to just pick one of the options, we're not taking away their ability to customize the constraint via the CLI. My current thinking: When a user selects one of the easy options, it populates the text field with the calculated version constraint. From there they can choose to edit it (at which point the radio of easy options is no longer selected), and they can reselect one of the radio options, re-calculating the text field.

When they later edit the installed package, the UI would be the same (ie. starts with text field populated with the current constraint, but they can select a radio option with slightly different labels: "Include the current version and any future patches").

For Carvel, didn't @antgamdia already add the option through chart values like kubeappsapis.kappController.packages.v1alpha1.defaultUpgradePolicy?

Yep, with the Carvel plugin it already sets the version constraint according to the config. I'd thought it wasn't working but it was just that I was assuming the default config would upgrade. I'll update this issue - the first step should be doing the equivalent for flux, using the default config, before we consider updating the UI.

Another question is if we would support both a Kubeapps-wide setting (like Antonio did very well for Carvel) and a per-installation setting?

Yep, the kubeapps-wide setting is just a default until we have a UX allowing user selection

could we defer this to the backend plugin

+1 IMHO UI should be as business agnostic as possible

A minor side comment on this from the UI design point of view: the user input part of the installation screen (the part above the YAML tabs) might need a re-design? Having three controls "floating" there. I don't mean in the short term of course. image

Yes - though it's not as simple as redesigning it... we actually want to make this something plugins can provide (currently we have code that checks if it's a carvel/flux plugin before adding the service account field.

absoludity avatar Feb 28 '22 06:02 absoludity

@castelblanque and I chatted about this one during the meeting and think it might be best in two stages:

  1. Quick win by updating the flux plugin to work similarly to the carvel one in that the plugin itself can be configured with a defaultUpgradePolicy (see the Cavel docs for defaultUpgradePolicy as well as the code). Perhaps @gfichtenholt you can let us know what you think, or we can have a call to chat about it sometime this week.
  2. The longer road of updating the UX as outlined above. This would be easy if we continued down the path of hard-coding UI elements for certain plugins but we don't want to do that. We need to start addressing the case for plugins providing certain UI elements as we've chatted.

absoludity avatar Feb 28 '22 11:02 absoludity

  1. Quick win by updating the flux plugin to work similarly to the carvel one in that the plugin itself can be configured with a defaultUpgradePolicy (see the Cavel docs for defaultUpgradePolicy as well as the code). Perhaps @gfichtenholt you can let us know what you think, or we can have a call to chat about it sometime this wee

Looked at the docs and the code. Looks pretty straightforward. defaultUpgradePolicy just determines what the version constraint expression looks like. I don't have any problems doing the same in flux plugin. Might even move the code into pkg so we can just re-use it

Is defaultUpgradePolicy business meant to stay long term as a feature? It's very coarse grained and not very UX friendly, IMO

gfichtenholt avatar Feb 28 '22 23:02 gfichtenholt

Instead, we may want to expose the version constraint field, or present a UX such as: (...)

.castelblanque and I chatted about this one during the meeting and think it might be best in two stages (...)

I also agree. I like having the configurable global default but still provide a UI-friendly way to configure it. For this option, I think it makes sense to have the "patch, minor, any" approach we have been discussing.

Given that (currently) we are not exposing the plugin's config back to the UI, we can't pre-select this default in the UI. But we can just allow empty values there (like selecting "default"); anyway, not much complexity in implementing this selector.

In short, yep, +1 to add the global default in Flux and +1 to implement it on the UI side with a simple dropdown.

Mid-term, we should consider the plugin to provide those options to the frontend somehow. I'm kind of against providing UI components (like the "custom component" logic we already have) since it would be coupled to our current client's tech stack. I would rather go with a simple json schema manifest, declaring the properties a plugin would accept (including whether they are required or not), plus a mapping to which API request body it refers to.

Like:

// carvel plugin schema
{
  "type": "object",
  "properties": {
    "serviceAccountName": { "type": "string", binding: "v1alpha1GetServiceAccountNamesResponse/serviceaccountNames/[0]"},
    "versionConstraint": { "type": "string", "enum": ["default", "patch", "minor", "major"] },
  },
  "required": ["serviceAccount"]
}

antgamdia avatar Mar 01 '22 15:03 antgamdia

I would rather go with a simple json schema manifest, declaring the properties a plugin would accept (including whether they are required or not), plus a mapping to which API request body it refers to.

Like:

// carvel plugin schema
{
  "type": "object",
  "properties": {
    "serviceAccountName": { "type": "string", binding: "v1alpha1GetServiceAccountNamesResponse/serviceaccountNames/[0]"},
    "versionConstraint": { "type": "string", "enum": ["default", "patch", "minor", "major"] },
  },
  "required": ["serviceAccount"]
}

+1, I'll create a separate task for this (EDIT: #4365). Note, if the json schema is the response of an API request (which I assume we'll want), the plugin can choose to send static text loaded from a resource, but could also generate it dynamically, for example, in this case, to use the plugin config to set the default version constraint, rather than needing a "default" option which is opaque to users?

absoludity avatar Mar 01 '22 23:03 absoludity