Move Cloud commands out of Spin CLI and into a plugin #1452

kate-goldenring · 2023-05-04T21:53:29Z

WIP draft to move Fermyon Cloud command code (spin cloud deploy and spin cloud login) out of Spin CLI and into a plugin.

SIP.

Cloud plugin: https://github.com/fermyon/cloud-plugin

~~Do not merge until the plugin is added to the Spin plugins registry.~~ Added

src/bin/spin.rs

Signed-off-by: Kate Goldenring <kate.goldenring@fermyon.com>

kate-goldenring · 2023-05-10T21:57:34Z

@bacongobbler has been working on the SIP that will be added to this PR

Signed-off-by: Matthew Fisher <matt.fisher@fermyon.com>

bacongobbler · 2023-05-11T06:14:43Z

@bacongobbler has been working on the SIP that will be added to this PR

Added in 84fd741

Signed-off-by: Kate Goldenring <kate.goldenring@fermyon.com>

docs/content/sips/013-cloud-plugin.md

kate-goldenring · 2023-05-11T23:23:47Z

src/commands/external.rs

+            if plugin_name == "cloud" {
+                println!("The `cloud` plugin is required. Installing now.");
+                let plugin_installer = Install {
+                    name: Some("cloud".to_string()),


To test this out now (before the cloud plugin is added to the plugins repository), replace the name field with local_manifest_src that points to the local manifest after cloning the cloud-plugin repo

local_manifest_src: Some("/Path/to/repo/cloud-plugin/plugin/cloud.json".into()),

now added to the plugins repo so no need to modify

Signed-off-by: Kate Goldenring <kate.goldenring@fermyon.com>

kate-goldenring · 2023-05-15T23:39:30Z

The test_spin_deploy will continue to fail till we add the plugin to the spin-plugins repo

kate-goldenring · 2023-05-16T17:19:23Z

@tpmccallum added an issue to documentation on documenting this fermyon/developer#621

Signed-off-by: Matthew Fisher <matt.fisher@fermyon.com>

src/commands/external.rs

Signed-off-by: Matthew Fisher <matt.fisher@fermyon.com>

kate-goldenring · 2023-05-17T21:22:46Z

for the cloud plugin, @itowlson brought up the question of whether we should suppress prerelease compatibility warnings?. At the moment, using any plugin with canary or HEAD gives a "prerelease version of Spin, plugin may not be compatible." However, cloud login and cloud deploy are common paths so that is a lot of warnings and may become irksome. The main folks irked are contributors as users should be using a release.

bacongobbler · 2023-05-17T22:18:45Z

However, cloud login and cloud deploy are common paths so that is a lot of warnings and may become irksome.

How often is this warning displayed? If you're using a canary release, does it happen on every invocation to spin deploy?

Perhaps we could display a prerelease compatibility warning once per day. I don't think Spin has a date check system to warn the user, though.

IMO this is starting to feel like it falls out of scope of this PR and it should be tackled separately, either as a follow-up discussion or a new SIP to modify plugin loading behavior for "core" plugins. The system was designed to warn the user when you're using a pre-release of Spin that invokes a plugin.

I can see why we may want to tackle this problem now especially because cloud-plugin is part of the default on-ramp experience to the Fermyon Cloud. Personally I'd prefer that we take that discussion to another thread as the decisions we make here will impact other plugins we may want to bring into the core CLI.

itowlson · 2023-05-17T22:21:22Z

It's displayed every time any plugin is invoked. I'm happy to decouple it from the cloud PR though - it occurred to me just because this was a blessed plugin that aimed for a relatively seamless experience, but I will open a separate issue for it.

bacongobbler · 2023-05-17T22:21:44Z

It's good that you point out that this is just a canary/HEAD problem. Most users won't see this. I'd be fine with kicking this decision down the road.

docs/content/sips/013-cloud-plugin.md

Signed-off-by: Kate Goldenring <kate.goldenring@fermyon.com>

kate-goldenring · 2023-06-06T20:56:16Z

@itowlson @fibonacci1729 @bacongobbler @radu-matei this should be ready for another review. I also added a test that ensures that the cloud plugin is installed during a spin login and removed all fermyon-platform tests.

kate-goldenring · 2023-06-06T21:01:50Z

I am weirdly having an issue rebasing the commits to just 1. It will only go back HEAD~2 worth of commits with fatal: invalid upstream 'HEAD~5' and will only show a few commits even if i point to the first commits. Maybe we can squash merge this so it isn't a bunch of commits?

itowlson

mostly nits - looks good!

src/commands/cloud.rs

src/commands/external.rs

src/commands/plugins.rs

itowlson · 2023-06-06T21:16:43Z

@kate-goldenring I agree we don't need to keep the commit history but I think GH squashing is disabled because of GPG - not sure we can bypass it.

kate-goldenring · 2023-06-07T00:14:04Z

@itowlson I believe it should be cleaned up

itowlson

LGTM!

fibonacci1729

LGTM

Signed-off-by: Kate Goldenring <kate.goldenring@fermyon.com> Remove Fermyon Platform tests Signed-off-by: Kate Goldenring <kate.goldenring@fermyon.com> Test that cloud plugin is installed during a spin login Signed-off-by: Kate Goldenring <kate.goldenring@fermyon.com> Syntactical nits Signed-off-by: Kate Goldenring <kate.goldenring@fermyon.com>

bacongobbler · 2023-06-07T15:38:36Z

I got this when initially testing:

><> spin cloud login
The `cloud` plugin is required. Installing now.
Error: plugin 'cloud' not found at expected location /home/bacongobbler/.local/share/spin/plugins/.spin-plugins/manifests/cloud/cloud.json: No such file or directory (os error 2)

Once I ran spin plugin update, everything worked as expected:

><> spin cloud login
The `cloud` plugin is required. Installing now.
You're using a pre-release version of Spin (1.2.0-pre0). This plugin might not be compatible (supported: >=1.3). Continuing anyway.
Plugin 'cloud' was installed successfully!

Copy your one-time code:

MWtjLTDG

...and open the authorization page in your browser:

https://cloud.fermyon.com/device-authorization

Waiting for device authorization...
Device authorized!

Might be worth calling out in the release notes.

Otherwise, LGTM!

bacongobbler · 2023-06-07T15:39:23Z

One other thing unrelated to this PR: I noticed spin plugin list seems to show the cloud plugin twice:

 ><> spin plugin list
cloud 0.1.0 [installed]
cloud 0.1.0 [installed]
js2wasm 0.1.0
js2wasm 0.2.0
js2wasm 0.3.0
js2wasm 0.4.0 [installed]
py2wasm 0.1.0
py2wasm 0.1.1
py2wasm 0.2.0

kate-goldenring · 2023-06-07T15:40:24Z

@bacongobbler good find. I think we might want to add a spin plugin update in this case that it can't be found. Thoughts @itowlson?

Signed-off-by: Kate Goldenring <kate.goldenring@fermyon.com>

kate-goldenring · 2023-06-07T16:18:06Z

@bacongobbler I added a step to do spin plugin update for the user if it doesn't see the cloud plugin. We can probably remove this later once we have the badgering and it is highly unlikely to not have updated. We may be able to remove this step in the future once we think everyone will have it. Duplication of the plugin should now be fixed via fermyon/spin-plugins#25

itowlson · 2023-06-07T19:21:14Z

@bacongobbler Thanks for flagging the "double cloud" quirk in the list. It looks like at one point there were both a cloud.json and a cloud@0.1.0.json in the registry, both reporting v0.1.0. There's now just the one, so spin plugins update made the duplication go away (for me). @kate-goldenring Is this a situation I should guard against in the client, or is it a "shouldn't happen" in the registry?

itowlson · 2023-06-07T19:25:46Z

Doh sorry, I completely missed all the bits where @kate-goldenring covered the duplication stuff already, please ignore.

kate-goldenring marked this pull request as draft May 4, 2023 21:53

itowlson reviewed May 5, 2023

View reviewed changes

src/bin/spin.rs Show resolved Hide resolved

Remove Cloud related subcommands and alias to the cloud plugin

1926422

Signed-off-by: Kate Goldenring <kate.goldenring@fermyon.com>

kate-goldenring force-pushed the cloud-plugin-porting branch from 0281aa3 to 1926422 Compare May 10, 2023 21:54

Add SIP 013: Cloud Plugin

84fd741

Signed-off-by: Matthew Fisher <matt.fisher@fermyon.com>

Display cloud commands in Spin CLI help documentation

dd09271

Signed-off-by: Kate Goldenring <kate.goldenring@fermyon.com>

kate-goldenring marked this pull request as ready for review May 11, 2023 22:55

kate-goldenring added 2 commits May 11, 2023 16:05

Suggest installing cloud plugin if not installed

58fa04b

Signed-off-by: Kate Goldenring <kate.goldenring@fermyon.com>

Merge remote-tracking branch 'upstream/main' into cloud-plugin-porting

8f73459

kate-goldenring commented May 11, 2023

View reviewed changes

Remove license check on Cloud plugin

a173703

Signed-off-by: Kate Goldenring <kate.goldenring@fermyon.com>

kate-goldenring mentioned this pull request May 16, 2023

Update documentation in accordance with the new cloud plugin fermyon/developer#621

Closed

cloud plugin prompted to install

33537bf

Signed-off-by: Matthew Fisher <matt.fisher@fermyon.com>

bacongobbler reviewed May 16, 2023

View reviewed changes

src/commands/external.rs Outdated Show resolved Hide resolved

bacongobbler added 2 commits May 17, 2023 09:48

plugin auto-installs

2171c6f

Signed-off-by: Matthew Fisher <matt.fisher@fermyon.com>

fix markdown linter errors

31abe4a

Signed-off-by: Matthew Fisher <matt.fisher@fermyon.com>

itowlson mentioned this pull request May 18, 2023

Spin deploy and Spin cloud deploy seem to do the same thing #1520

Closed

kate-goldenring commented May 22, 2023

View reviewed changes

docs/content/sips/013-cloud-plugin.md Outdated Show resolved Hide resolved

Remove cloud crate and cloud-openapi reference

a9f48e2

Signed-off-by: Kate Goldenring <kate.goldenring@fermyon.com>

kate-goldenring mentioned this pull request May 25, 2023

Add --variables deploy flag and variables subcommand fermyon/cloud-plugin#7

Merged

Merge remote-tracking branch 'upstream/main' into cloud-plugin-porting

19d983f

kate-goldenring added the do-not-merge label May 30, 2023

kate-goldenring removed the do-not-merge label Jun 6, 2023

itowlson requested changes Jun 6, 2023

View reviewed changes

src/commands/cloud.rs Show resolved Hide resolved

src/commands/cloud.rs Outdated Show resolved Hide resolved

src/commands/cloud.rs Outdated Show resolved Hide resolved

src/commands/external.rs Show resolved Hide resolved

src/commands/plugins.rs Outdated Show resolved Hide resolved

itowlson approved these changes Jun 7, 2023

View reviewed changes

fibonacci1729 approved these changes Jun 7, 2023

View reviewed changes

kate-goldenring force-pushed the cloud-plugin-porting branch from 81791cc to 78b458a Compare June 7, 2023 15:38

bacongobbler approved these changes Jun 7, 2023

View reviewed changes

Update plugins if cloud plugin does not exist

07516f3

Signed-off-by: Kate Goldenring <kate.goldenring@fermyon.com>

kate-goldenring force-pushed the cloud-plugin-porting branch from f041b98 to 07516f3 Compare June 7, 2023 16:30

melissaklein24 added this to the 1.3.0 milestone Jun 7, 2023

kate-goldenring merged commit eb5e826 into fermyon:main Jun 7, 2023

rylev mentioned this pull request Dec 21, 2023

Remove unused files #2194

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move Cloud commands out of Spin CLI and into a plugin #1452

Move Cloud commands out of Spin CLI and into a plugin #1452

kate-goldenring commented May 4, 2023 •

edited

Loading

kate-goldenring commented May 10, 2023

bacongobbler commented May 11, 2023

kate-goldenring May 11, 2023

kate-goldenring Jun 6, 2023

kate-goldenring commented May 15, 2023

kate-goldenring commented May 16, 2023

kate-goldenring commented May 17, 2023

bacongobbler commented May 17, 2023 •

edited

Loading

itowlson commented May 17, 2023

bacongobbler commented May 17, 2023

kate-goldenring commented Jun 6, 2023

kate-goldenring commented Jun 6, 2023

itowlson left a comment

itowlson commented Jun 6, 2023

kate-goldenring commented Jun 7, 2023

itowlson left a comment

fibonacci1729 left a comment

bacongobbler commented Jun 7, 2023

bacongobbler commented Jun 7, 2023

kate-goldenring commented Jun 7, 2023

kate-goldenring commented Jun 7, 2023

itowlson commented Jun 7, 2023

itowlson commented Jun 7, 2023

Move Cloud commands out of Spin CLI and into a plugin #1452

Move Cloud commands out of Spin CLI and into a plugin #1452

Conversation

kate-goldenring commented May 4, 2023 • edited Loading

kate-goldenring commented May 10, 2023

bacongobbler commented May 11, 2023

kate-goldenring May 11, 2023

Choose a reason for hiding this comment

kate-goldenring Jun 6, 2023

Choose a reason for hiding this comment

kate-goldenring commented May 15, 2023

kate-goldenring commented May 16, 2023

kate-goldenring commented May 17, 2023

bacongobbler commented May 17, 2023 • edited Loading

itowlson commented May 17, 2023

bacongobbler commented May 17, 2023

kate-goldenring commented Jun 6, 2023

kate-goldenring commented Jun 6, 2023

itowlson left a comment

Choose a reason for hiding this comment

itowlson commented Jun 6, 2023

kate-goldenring commented Jun 7, 2023

itowlson left a comment

Choose a reason for hiding this comment

fibonacci1729 left a comment

Choose a reason for hiding this comment

bacongobbler commented Jun 7, 2023

bacongobbler commented Jun 7, 2023

kate-goldenring commented Jun 7, 2023

kate-goldenring commented Jun 7, 2023

itowlson commented Jun 7, 2023

itowlson commented Jun 7, 2023

kate-goldenring commented May 4, 2023 •

edited

Loading

bacongobbler commented May 17, 2023 •

edited

Loading