Skip to content
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

[WIP] Fix issues with iFramed cluster pages #12548

Conversation

mantis-toboggan-md
Copy link
Member

@mantis-toboggan-md mantis-toboggan-md commented Nov 8, 2024

3 open issues all related to rendering the wrong iframed Ember page, or rendering the page with missing content:
#12506
#12541

possibly #12339

The code used to determine which component to render when editing any given type of cluster is convoluted. There are inexplicable mappings required to keep iframed ember pages functional (why must aks map to azureaks? why is the provider query param sometimes import and sometimes imported? Why is there an importProvider query param not used on imported clusters?) I am not even sure the above 3 issues have exactly the same root cause, but this area of dashboard code is so convoluted and prone to churn, I think they need to be addressed in tandem.

I think the above bugs came in via some combination of this and this PR. Limiting the scope of those changes seems to address #12506 and #12541 but not fully #12339 ( with this PR, I see the ember page as expected, but the 'mode' is wrong, create button shown instead of save)

Because I think the bugs are conntected to ember query params, I've collected data on the fields used to generate those query params, and what those query params are in 2.9-head ember standalone world - ember standalone has been removed from 2.10 but we can assume that these are the correct params the dashboard should be passing to ember. All below fields are on the relevant management cluster object, not the provisioning cluster object. If providerForEmberParam needs to be fixed, we need to verify that given the below status/config fields, the correct provider and importProvider query params are still set in every scenario.

cluster type status.provider status.driver spec.aksConfig/eksConfig/gkeConfig.imported ember provider qp* ember importProvider qp*
local rke1 rke imported n/a import other
local rke2 rke2 rke2 n/a import other
local k3s k3s k3s n/a k3s undefined
local hosted (eks) eks imported undefined import other
local MB ovh-downstream test cluster** '' imported undefined import other
rke1 provisioned 'rke' rancherKubernetesEngine n/a digitalocean undefined
rke2 provisioned rke2 imported n/a n/a n/a
k3s provisioned k3s imported n/a n/a n/a
hosted provisioned (aks) aks AKS false n/a n/a
rke1 imported rke imported n/a imported undefined
rke2 imported rke2 rke2 n/a rke2 undefined
k3s imported k3s k3s n/a k3s undefined
hosted imported (aks) aks AKS true azureaks undefined
3rd party kontainer provisioned '' ovhcloudmks n/a ovhcloudmks undefined

* undefined => an iframed page should be used for this cluster, but this query param should be present; n/a=>iframed ember pages should not be used for this cluster
** need to find out what this cluster actually is

Regarding ovh, once I get the right page to render I am seeing a console error that could be related
Screenshot 2024-11-07 at 12 46 44 PM

@gaktive
Copy link
Member

gaktive commented Nov 8, 2024

This would need a back port to v2.10.x -- .0 ideally but we're running out of time.

@codyrancher
Copy link
Contributor

I think the above bugs came in via some combination of this and this PR. Limiting the scope of those changes seems to address #12506 and #12541 but not fully #12339 ( with this PR, I see the ember page as expected, but the 'mode' is wrong, create button shown instead of save)

Because this all appears to be working in 2.9 I decided to diff dashboard between master and 2.9.

When diffing master with 2.9 I saw #12177 and when reverting it seems to resolve the issues we're having with editing. The above pr was created in response to #9868.

I'm leaning toward reverting #12177 and changing the fix for #9868 because ultimately it supposed to update a display property but seems to be having cascading consequences. I'll see if I can have some luck going down this path.

@mantis-toboggan-md
Copy link
Member Author

Closing in favor of #12583

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants