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

Automatic Cluster handling for KAITO installation #1209

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tejhan
Copy link
Collaborator

@tejhan tejhan commented Jan 29, 2025

This PR handles attempted installation of KAITO on automatic clusters.

Context : Due to nrg lockdown & deny permissions of Automatic cluster, we are unable to create federated credentials in the node resource group. This is necessary for proper KAITO installation & due to the Automatic cluster SKU configuration, we are unable to change access / permissions to allow for such. Thus, I've added safeguards to prevent failed install on auto clusters alongside a message to the user.

Additionally, there have been some additions to the KAITO installation logic to terminate installation when errors do occur (were some cases where error would be caught but installation would continue). Also added conditional render to display error messages for failed installations.

image

.vsix for testing -> vscode-aks-tools-1.5.5-autokaito.vsix.zip

@tejhan tejhan self-assigned this Jan 29, 2025
@tejhan tejhan added Change of behaviour Changes existing behaviour of usage to a new way. Kaito Kaito related work items labels Jan 29, 2025
Copy link
Collaborator

@ReinierCC ReinierCC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice error handling. Changes Look Good! ✅

Copy link

@ivelisseca ivelisseca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good, I like the message! 👍🏽
If there is any external documentation explaining why AKS automatic is not supported by KAITO it might be cool to add the typical "Learn more" hyperlink.


// Prevent KAITO installation on automatic clusters
const skuName = currentJson.sku?.name;
if (skuName === "Automatic") {
Copy link
Member

@Tatsinnit Tatsinnit Jan 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Thought Share only please: Regarding Automatic Can we please thing of making this Enum, if this has known types. What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Change of behaviour Changes existing behaviour of usage to a new way. Kaito Kaito related work items
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants