config3alphato3: convert config version 3-alpha to 3

[estroz]

Description of the change:

• internal/cmd/operator-sdk/alpha/config3alphato3: convert config version 3-alpha to 3, and exit after conversion
• internal/cmd/operator-sdk/cli: add PersistentPreRun to root cmd for helpful error message

Motivation for the change: Version 3-alpha has been removed after version 3 was released. While this change is not technically breaking because the spec at that version was alpha, it was used by default in operator-sdk commands so should be marked as breaking and have a convenient migration path. The alpha config3alphato3 command will convert most of a PROJECT file from version 3-alpha to 3, and leave comments with directions where automatic conversion is not possible.

/kind feature

Checklist

If the pull request includes user-facing changes, extra documentation is required:

• Add a new changelog fragment in changelog/fragments (see changelog/fragments/00-template.yaml)
• Add or update relevant sections of the docs website in website/content/en/docs
  Signed-off-by: Eric Stroczynski ericstroczynski@gmail.com

[jberkhahn]

if i just use operator-sdk and don't know kubebuilder exists, i might be confused about which version of what this is referring to, especially because kubernetes already has a thing called version that is special. maybe include some blurb in the command description about what this is specifically referring to.

[jmrodri]

For some reason, when we mentioned auto migration, I didn't expect another subcommand on the CLI.

[jmrodri]

it's almost like the pkgman-to-bundle and this config3alphato3 would be under some sort of grouping. Also config3alphato3 doesn't match what we described pkgman-to-bundle. Could it be config3alpha-to-3? Or could we have convert subcommand that does the following:

operator-sdk convert config3alpha-to-3

operator-sdk convert pkgman-to-bundle

Making convert a catch all for these one off conversion functions.

[Adirio]

it's almost like the pkgman-to-bundle and this config3alphato3 would be under some sort of grouping. [...] Or could we have convert subcommand that does the following:

operator-sdk convert config3alpha-to-3

operator-sdk convert pkgman-to-bundle

Making convert a catch all for these one off conversion functions.

Relative to the name of the command, I would maybe use "migrate" or "auto-migrate" over "convert". Will use "convert" for the discussion now to avoid missunderstandings.

From an implementation PoV, "convert" *cobra.Command would have to be created, both subcommands added, and then "convert" passed to WithExtraCommands. So it should be doable.

From a backwards compatibility PoV, if "pkgman-to-bundle" is already present in a released version, moving it into a subcommand would be technically a breaking change, so maybe it is needed to offer it as both opertor-sdk pkgman-to-bundle and operator-sdk convert pkgman-to-bundle (maybe adding a deprecation notice on the first).

Now, with my eyes set in the future, we may need some similar subcommand when we make some plugins more state-aware. For example, manifests/v2 could use a list of tracked resources which is was applied to. What do you think of making a command convert config and then accepting alpha-to-3 as an argument. In a future we would have other values, e.g. track-manifests-resources. If the user specifies the argument that tool would be used, if it just runs operator-sdk convert config it would try to be smart and decide which ones it should apply, even asking whatever info it is missing to the user. Example:

$ operator-sdk convert config
Project version 3-alpha detected, converting to version 3.
Manifests plugin detected without tracked reources.
Want to track the resource? [y/n]
Y
Tracking resources for manifests plugin.

Disclaimer: we can probably be smart enough not to need to ask the user if he wants to track resources, but I wanted to illustrate some user interaction trhough the migration process.
Discalimer2: If the command group idea is discarded, I would call this something like "fix-config" or "migrate-config", not just "config".

[estroz]

Thanks for the feedback everyone.

Regarding the subcommand naming/hierarchy comments: these migration-type commands do not represent stable APIs, or features for that matter. They exist only to help users migrate between project states in very specific cases. I do not want users to think they can rely on more migration commands being added that effectively run through SDK upgrade guides published with each release. Therefore either hiding them or putting them under operator-sdk alpha is my vote; the latter is better because it exposes help/docs via CLI.

[camilamacedo86]

@estroz,

Could we just add the info on why the project version has more data now in the changelog also provide the upstream links where we have the definition over what actually each field means just in case the person requires to fix anything manually? See; #4613 (comment) and #4613 (comment). (Also, would be nice we link the production doc but I think it still not published :-( )

After that, it shows ok for me.

[Adirio]

Therefore either hiding them or putting them under operator-sdk alpha is my vote; the latter is better because it exposes help/docs via CLI.

Just a comment, config-gen is being PRed against k9r alpha config-gen which means that if you try to add an extra command with alpha ... it will break once k9r adds it. We should probably allow plugins to also add alpha commands in a future (WithExtraAlphaCommands) so this would be solved but I wanted to mention it to keep it in mind.

We need to address the Adirio founds as well before get merged.

[camilamacedo86]

all comments addressed!

so it shows nice for we move forward.

[kuritka]

Hi,
Is it required to be a string ("3")?
version: 3-alpha-> version: "3"

[estroz]

@kuritka yes, that is how the project spec defines the version type.

[Adirio]

Hi,
Is it required to be a string ("3")?
version: 3-alpha-> version: "3"

That's part of YAML definition. 3-alpha doesn't require quotes because it can only be interpreted as a string. However, 3 would be interpreted as a number, so "3" is required.

[kuritka]

@estroz, @Adirio, thank you for explaining!