Index directory layout migration

[chriskim06]

Currently there is a krew system index-upgrade command to migrate users to the new directory structure needed to support multiple indexes. It's been mentioned before about potentially making this migration happen automatically.

Both options seem fine to me. The receipts migration used a subcommand to perform the migration but I also think it might be more user friendly to have the index migration happen under the hood. @ahmetb @corneliusweig what are your thoughts?

/area multi-index

[ahmetb]

I'm currently indifferent. We haven't heard of many complaints about the explicit migration in the past, right @corneliusweig ? My main concern is people leaping forward 3 versions at a time (think: they don't have indexmigration yet).

So we need to probably manually test direct migration from 0.2 to latest to see if it's gracefully migrated (either explicitly or implicitly).

[chriskim06]

I noticed a bug when I tried the update from 0.2 to master. Here's what I did:

• downloaded 0.2 version
• installed a plugin
• built master and tried running krew list
• prompted to run receipts-upgrade
• ran receipts-upgrade and got prompted to run index-upgrade
• ran index-upgrade and got prompted to run receipts-upgrade
• neither migration happened (although receipts/ dir was created)

Here's the output I saw after using the build from master (I kept both binaries in this folder so that's why this one has -master appended to the end):

$ KREW_ROOT='/home/kimc/oldkrew' X_KREW_ENABLE_MULTI_INDEX=1 ./krew-linux_amd64-master list
WARNING: To be able to run kubectl plugins, you need to add
the following to your ~/.bash_profile or ~/.bashrc:

    export PATH="${PATH}:${HOME}/.krew/bin"

and restart your shell.

You need to perform a migration to continue using krew.
Please run `kubectl krew system receipts-upgrade`
F0508 16:48:41.225263   27413 root.go:79] krew home outdated

$ KREW_ROOT='/home/kimc/oldkrew' X_KREW_ENABLE_MULTI_INDEX=1 ./krew-linux_amd64-master system receipts-upgrade
WARNING: To be able to run kubectl plugins, you need to add
the following to your ~/.bash_profile or ~/.bashrc:

    export PATH="${PATH}:${HOME}/.krew/bin"

and restart your shell.

You need to perform a migration to continue using krew.
Please run `kubectl krew system index-upgrade`
F0508 16:48:47.981024   27449 root.go:79] krew index outdated

$ KREW_ROOT='/home/kimc/oldkrew' X_KREW_ENABLE_MULTI_INDEX=1 ./krew-linux_amd64-master system index-upgrade
WARNING: To be able to run kubectl plugins, you need to add
the following to your ~/.bash_profile or ~/.bashrc:

    export PATH="${PATH}:${HOME}/.krew/bin"

and restart your shell.

You need to perform a migration to continue using krew.

I'm not exactly sure why this happens, but I'll look into it and comment here with what I find.

[chriskim06]

Ok so the above happens because in root.go the index migration check happens after the receipts migration check in the preRun function. Once I got past that I noticed that the receipt migration failed because it calls ensureIndexesUpdated and that tries to list the indexes installed. The receipt migration code also uses environment.Paths and a lot of those will be assuming a migrated index once the env var gate is removed.

I think a simple solution would be to move the index migration before the receipts migration (just a hunch, haven't dug too deep into it).

[ahmetb]

I think a simple solution would be to move the index migration before the receipts migration (just a hunch, haven't dug too deep into it).

Doing index migration first, then receipts migration can open a can of worms. If we can maintain the current order, that would be the best (if there's an easy solution). If not, we can consider ripping out the receipts migration for the upcoming 0.X version.

[chriskim06]

So in my first pass at this I tried changing some stuff around. The receipts migration uses environment.Paths/oldenvironment.Paths, and I had to switch some of the functions to use the old paths . I was able to get it working to a point where it gets the list of installed plugins and tries to reinstall them, but this happens through an exec.Command("kubectl", "krew", "install", plugin). This then fails because of the index migration check.

I'm not sure if there's an easy solution here since the install code will be using the new index paths. What would the path forward for users on 0.2 look like if the receipts migration was removed from 0.X?

[ahmetb]

What would the path forward for users on 0.2 look like if the receipts migration was removed from 0.X?

At this point 0.2 was a long time ago.
We also promised to delete it 0.4 or 0.5. Now that these issues are arising 0.4 sounds like a good time :) @corneliusweig to decide.

[corneliusweig]

There is another option. So far, we see the migration as a two-step process: 0.2->0.3->0.4. However, if we ignore the 0.2->0.3 migration, we can write a single {0.2,0.3}->0.4 migration and avoid the hassle of two different migration code paths. This would mean that users need to re-install all plugins as they had to do for 0.2->0.3. It should be straightforward to extend the old migration code to the new one.

Dropping the migration code 0.2->0.3 has some drawbacks. For example, we need to come up with an escape hatch for users 0.2 users, such as

ls -d $KREW_HOME/store/ > plugins.tmp \
  && rm -r $KREW_HOME \
  && {reinstall krew} \
  && krew install < plugins.tmp

And this should work for all OSes, so it's not as easy as it sounds.

[ahmetb]

Doesn't sound like a bad idea. Though, it might be a little too burdensome to refactor. We already have a receiptsmigration package that uses old way of doing things, I'm not sure how these two would work together.

What if we detected receiptsmigration and told v0.2.x users:

This version is not supported anymore. Please manually migrate:
1. Uninstall Krew: [[link]]
2. Reinstall Krew: [[link]]
3. Install current plugins: kubectl krew install [...]

instead of mechanically trying to migrate?

[corneliusweig]

Yes, this is more or less what I tried to sketch with my bash snippet. This can work, but is not great UX. And the crucial point is to get 3) right. Because after 0.3 we use the receipts folder to list plugins, and if someone has a 0.2 KREW_HOME, this won't work for him. That's why I'm worried that this may be more complex than it sounds.

We could try out if we can get the unified 0.2|0.3->0.4 migration to work. If it's too hairy we can still abandon this idea.

[chriskim06]

So I took a quick pass at getting the unified 0.2|0.3->0.4 migration to work. I think the main issue boils down to needing to reinstall the plugins to generate receipts for 0.2 users since the install path will assume a migrated index layout. I haven't had time since I last tried to make changes, but I have a feeling that it would get pretty messy trying to get that to work without performing the index migration first, but as @ahmetb mentioned, this could cause other issues.

[chriskim06]

@ahmetb @corneliusweig gently pinging. How do you guys want to proceed with this?

[ahmetb]

I recommend we rip out receipts migration as part of this release. That way the indexmigration can be a log simpler and automatic I assume.

[chriskim06]

That works for me, @corneliusweig if that sounds good to you, then I can open a PR to

• remove receiptsmigration (check in root.go, receipts-upgrade subcommand, and tests)
• make indexmigration automatic
• add integration tests

[corneliusweig]

If that is so much simpler, then let's not complicate things and remove the old migration code. So yes, go ahead!

[ahmetb]

Please make sure to create a separate PR for removing receiptsMigration. That’d be easier to navigate.