-
Notifications
You must be signed in to change notification settings - Fork 93
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: support for multi channel licenses #4767
Conversation
da82b40
to
8b2c5e3
Compare
downstream vers check uses IsSemverRequired from channel license
da0c0ea
to
1355d88
Compare
The approach I took was basically find all occurrences where we using license.spec.ChannelID or license.spec.IsSemverRequired and update those areas. In most cases we now use For dealing with the backfill of |
This LGTM. One note about backwards compatibility. Right now I can run With this change, single channel license install will start failing. I think we'll need to allow them to proceed. We can call out this change in behavior in release notes, which is probably acceptable. |
@divolgin Let them proceed with an install for the requested (not in license) channel, or proceed but switch over to the default channel in the supplied license? Guessing the later - the default channel. |
@pandemicsyn we should add an e2e test. |
I might've missed it, but where does KOTS send the user-selected channel ID to replicated.app? |
This will need https://github.com/replicatedhq/vandoor/pull/5889 but this commit should cover all the endpoints that now accept it on the replicated app side: f38716c I did skip updating |
@sgalsaleh @divolgin i centralized the post license sync update of Also nuked that stray selected channel id update in reconcileDeployment in this commit: ab0f4d7 |
Ended up centralizing the backfilling, I moved it into store.GetApp() ...tbh...not sure why I didn't stick it there to begin with. Also, heres a loom of license channel change working (going from "beta" with a v1.0.2 to "unstable" which was up to v1.0.3): https://www.loom.com/share/5b89a5a3423341f881c7510a2149b2fb?sid=4a072c59-b965-47b0-bb2d-7c5187b511c3 |
@sgalsaleh alrighty - pulled in the e2e test changes as well (3961a04) , batched the other pr feedback into a single commit this time since it was pretty small (3cd0a79) |
What this PR does / why we need it:
Add's support for multi channel licenses.
Spec & behaviors
app_version.selected_channel_id
, we do this by pulling from a newrequested_channel_slug
configmap entry during install and resolving that to a channel id-kots upgrades:
kots install
minus the the attempt to get the latest license.selectedChannelId
to actually pass on what the selected channel isWhich issue(s) this PR fixes:
Fixes https://app.shortcut.com/replicated/story/108865/multi-channel-license-update-kots-to-support-multi-channels
Special notes for your reviewer:
Steps to reproduce
Online install early checks:
Simple case where you ask to install a channel not in the licenses fails:
If you pass in an outdated license it will declined if the requested channel is not in the latest license:
Install proceeds if your requested channel is present (even when its not the default channel):
Same pattern for airgap:
With an airgap update via the ui:
Up to date Loom: https://www.loom.com/share/c82e9174906b427196a03a5d16ac3402?sid=7989633f-4ad7-4b86-9a5a-9c28d3ca8705
Does this PR introduce a user-facing change?
Does this PR require documentation?