fix: update spin deploy to patch channel rather then recreate channels

[kate-goldenring]

Currently, spin deploy is deleting and recreating channels in order to ensure the correct bindle will be served by Hippo. This modifies the behavior to use the patch_channel command recently added to hippo-cli.

This PR cannot merge until the following changes have been made to hippo-cli and hippo-openapi -- marking as Draft in the meantime:

• fix: avoid Field wrapping PatchChannelCommand fields deislabs/hippo-cli#151
• fix: remove Field wrapping of PatchChannelCommand arguments hippo-openapi#10

Note: A later PR should address the fact that multiple API calls are made here where there could simply be one call to add_revision. This currently does not suffice because the naming schema of bindles cannot be ordered deterministically by Hippo using a RangeRule (see #680). If we change the naming schema, we could just add a revision and the range rule in hippo will correctly serve the latest bindle. I added a TODO in the module and can create an issue after this change goes through.

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

[vdice]

Strategy and code here looks good to me. Defer to @bacongobbler on account of the mentioned hippo-* dependency updates.

[bacongobbler]

Can you bump hippo to v0.16.1 and run cargo update?

spin/Cargo.toml

Line 21 in ee996c1

hippo = { git = "https://github.com/deislabs/hippo-cli", tag = "v0.15.0" }

Once that's done, we should be good to go.

[bacongobbler]

LGTM

[bacongobbler]

I'm seeing very odd behaviour... With two applications, I'm seeing the other app being updated each time I spin deploy.

><> spin deploy
Deployed bacongobbler-spin-hello-world version 1.1.2+qc3b3e6e
Available Routes:
  hello: https://bacongobbler-spin-outbound-http.platform.fermyon.link/hello
><> ./target/release/hippo channel list
[
  {
    "id": "0cebbc1f-4d00-42c2-9719-53eca890553e",
    "appId": "6e40ce4a-a5be-43e6-a7ff-2dfd36f58fcd",
    "name": "spin-deploy",
    "domain": "*******",
    "revisionSelectionStrategy": "UseSpecifiedRevision",
    "activeRevision": {
      "id": "85441d59-3d4e-41d2-b79f-7ceec10488ef",
      "appId": "082f5231-7f1d-4213-ac5a-5d346de3ecc1",
      "revisionNumber": "1.1.2+qc3b3e6e",
      "components": [
        {
          "id": "c8b004cf-11ce-476d-b696-410ed5f70a41",
          "source": "2cdbcf6871e4c6dd79962f999b2a3e60754ae75ba8ee4c77888d0548ac0df6d5",
          "name": "hello",
          "route": "/hello",
          "type": "http"
        }
      ]
    },
    "lastPublishAt": "2022-08-29T23:01:03.728173Z",
    "rangeRule": "1.0.0+q3558cb1",
    "environmentVariables": []
  },
  {
    "id": "dbb49cd9-0d0c-419d-b5d8-ed4047d59e25",
    "appId": "082f5231-7f1d-4213-ac5a-5d346de3ecc1",
    "name": "spin-deploy",
    "domain": ""*******",",
    "revisionSelectionStrategy": "UseRangeRule",
    "activeRevision": {
      "id": "c624f3cd-913f-4f8d-8bb9-08c10188e711",
      "appId": "082f5231-7f1d-4213-ac5a-5d346de3ecc1",
      "revisionNumber": "1.0.0+qf366a1d",
      "components": [
        {
          "id": "faad40dc-0ea6-467d-bba3-92c0324915d3",
          "source": "51634f19e230ba33e70058dbf9279ed7e607afa23719d880fc95aaf9acd6dd0e",
          "name": "hello",
          "route": "/hello",
          "type": "http"
        }
      ]
    },
    "lastPublishAt": "2022-08-31T23:13:59.976998Z",
    "rangeRule": "1.0.0+qf366a1d",
    "environmentVariables": []
  }
]

notice how the channel bound to bacongobbler-spin-hello-world never updated, but bacongobbler-spin-outbound-http did.

This might be a server-side issue with the RegisterRevisionCommand failing to map to the right channels.

[bacongobbler]

Ah... It's actually spin's get_channel_id.

spin/src/commands/deploy.rs

Line 304 in ee996c1

let channel = channels_vm.items.iter().find(|&x| x.name == name.clone());

This function takes a channel name, then returns the first channel that matches with the same name. If two applications use spin deploy, then the first channel will be returned. This may not map to the same app ID we expected, hence the odd behaviour.

This may also be the cause of some issues you were seeing in testing, @kate-goldenring.

We should be passing the application ID from line 173 to get_channel_id and filter the list down to channels that match this app ID, AND the channel name.

[bacongobbler]

LGTM, but can you please address the issue with get_channel_id so we update the correct channel?

[kate-goldenring]

@bacongobbler thank you for catching this! I believe it should be resolved with the latest changes. Can you double check on your side?

[bacongobbler]

Sorry, one last change... I think we need to apply the same logic to get_revision_id() as well. RevisionItems are mapped to an app ID, and two RevisionItems with the same version number but different app IDs can exist.

https://github.com/fermyon/hippo-openapi/blob/2b2f87e12b625f9c70010106837baf5cc21f80cd/clients/rust/src/models/revision_item.rs#L19

I know it's unrelated to this change, but it should help with these spurious errors we're seeing in the wild.

[kate-goldenring]

Great point @bacongobbler! I forgot that the bindle_id is the bindle version.

[bacongobbler]

Looks great! Just tested and deploys are rolling out as expected.

[bacongobbler]

LGTM

[vdice]

LGTM!