Adding API calls to create a Fleet #1160

JunyuQian · 2025-01-07T23:16:45Z

This is the initial pull request, implementing the API call for fleet creation and the callback function triggered by the user’s right-click on the ‘Create Fleet’ button.

The entry point (the ‘Create Fleet’ button) for this function is not yet available in the UI. This ensures that users will not encounter features still under development, enabling seamless and incremental integrations.

Tatsinnit · 2025-01-07T23:37:59Z

💡 🥷 Note only please: (for help for broken build) Build is broken because the package @azure/arm-containerservicefleet is missing from the package.json please add it and it will work magically :) (obviously under the hood it will find the package from package manager)

Signed-off-by: JunyuQian <[email protected]>

…aks-tools into feature-branch

serbrech · 2025-01-08T00:52:42Z

src/commands/aksFleet/aksFleetManager.ts

+    }
+
+    const client = new ContainerServiceFleetClient(getCredential(sessionProvider.result), subscriptionId);
+    createFleet(client, "junyuqian", "vscode-fleet", { location: "Australia East" });


Since the scope of this PR is now so much smaller, can we make a user-input right away?

@Tatsinnit I'll leave it to you. I don't know how much complexity that would add. fine to merge and deal with it later.

Indeed. Nice idea, It is not that complicated, in the earlier version demo shared, it was all flowing through the quickpick et. al.

Since it's all backend, and anonymised - lets either at least make clear //TODO Comment please here for anyone else trying to understand why this createFleet function is trying to do here, in case of folks want to help.

Future PR note: Also, from U/X we should make it simple pass it on from the web view of just quick pick kind of thing.

This is what I would do to make sure this is basically super early stage work and really avoid the hard-coded bits: :)

// Temporary code for incremental check-in. // TODO: Replace hardcoded values with dynamic parameters or configuration settings. // Initialize the ContainerServiceFleetClient with session credentials and subscription ID. // Hardcoded 'subscriptionId' should be parameterized in future updates. const client = new ContainerServiceFleetClient( getCredential(sessionProvider.result), // Retrieve credentials from session provider. subscriptionId // TODO: Ensure subscriptionId is dynamically passed or configured. ); // Create a fleet using hardcoded parameters. // TODO: Replace hardcoded 'junyuqian', 'vscode-fleet', and 'Australia East' with configurable inputs. createFleet( client, "Fleet-Resrource-Name", // Fleet resource group name (hardcoded). "Fleet-Name", // Fleet name (hardcoded). { location: "Australia East" } // Location (hardcoded). ); // NOTE: This temporary implementation assumes static context for testing purposes. // Ensure these hardcoded values are replaced with appropriate dynamic configurations // before finalizing this code for production level work which will be user focused.

Thank you so much Tats! It's so pleasant to read your code with all these neat comments! This will be the standard I'll align to when I write explanatory comments later :)

dvadas · 2025-01-08T00:59:35Z

package.json

@@ -331,6 +331,10 @@
            {
                "command": "aks.aksKaitoTest",
                "title": "Test KAITO models"
+            },
+            {
+                "command": "aks.aksCreateFleet",


Might be better to remove these command/title changes from the PR (since they're related to the menu, not the API), and leave only the new package on line 646.

Tatsinnit · 2025-01-08T01:06:28Z

package.json

@@ -541,6 +549,12 @@
                    "command": "aks.aksReconcileCluster",
                    "group": "navigation"
                }
+            ],
+            "aks.fleetMangerSubMenu": [


💡Note please: I thought you are not hooking the command yet. and this is backend only kind of PR?

Tatsinnit · 2025-01-08T01:07:52Z

package.json

@@ -442,6 +446,10 @@
                    "submenu": "aks.kaitoInstallSubMenu",
                    "when": "view == kubernetes.cloudExplorer && viewItem =~ /aks\\.cluster/i",
                    "group": "9@3"
+                },
+                {
+                    "submenu": "aks.fleetMangerSubMenu",


💡 Please note: same goes here, as a reference to this comment: https://github.com/Azure/vscode-aks-tools/pull/1160/files#r1906207748

I thought you are just adding backend code for now and classes and there submenu hooks are not needed?

Tatsinnit · 2025-01-08T01:10:42Z

package.json

+            },
+            {
+                "command": "aks.aksCreateFleet",
+                "title": "Create Fleet"


💡 Please note: Why do we need this with this checkin please? I think this change is not needed for this PR, only thing you need is the @azure/arm-containerservicefleet": "^1.1.0 which will get your supporting backend changes working. Please do correct me or add thoughts.

JunyuQian · 2025-01-08T02:36:22Z

Hi @Tatsinnit @serbrech @dvadas, thank you all for your comments! All the problems have been addressed and please let me know if there is anything I can improve :)

Tatsinnit · 2025-01-08T09:35:15Z

src/commands/aksFleet/aksFleetManager.ts

+    // TODO: Replace hardcoded 'junyuqian', 'vscode-fleet', and 'Australia East' with configurable inputs.
+    createFleet(
+        client,
+        "Fleet-Resrource-Name", // Fleet resource group name (hardcoded).


💡 Minor: Please take care in the latter PR, spelling Fleet-Resource-Name rather then Fleet-Resrource-Name :) Thanks.

Tatsinnit

💡 ❤️ Looks good to me, this is skeleton only PR, nothing is hooked yet for the work under create fleet idea of integration to VSC.

ReinierCC · 2025-01-08T22:30:39Z

Good work so far! It looks like your commits are unsigned so you might have some difficulty when you go to merge your PR.

You'll likely need to generate a new GPG key and resign your commits.

I dealt with this same issue recently, so if you have any questions feel free to reach out!

Tatsinnit · 2025-01-08T22:44:27Z

Good work so far! It looks like your commits are unsigned so you might have some difficulty when you go to merge your PR.

You'll likely need to generate a new GPG key and resign your commits.

I dealt with this same issue recently, so if you have any questions feel free to reach out!

❤️ Thank you so much @ReinierCC , nice catch, yes please lets get your commit signed @JunyuQian

Here are few docs as to how to sign your commits.

Please feel free to reach out if you get stuck,

Just as example by verification all your commit will have this verified tag

Thanks heaps.

initial pull request

25b3f1c

Tatsinnit assigned JunyuQian Jan 7, 2025

JunyuQian and others added 4 commits January 8, 2025 10:47

adding library dependencies

c8227c8

Merge branch 'main' into feature-branch

7385fe5

Signed-off-by: JunyuQian <[email protected]>

hide right-click button

ce47926

Merge branch 'feature-branch' of https://github.com/JunyuQian/vscode-…

aa7325d

…aks-tools into feature-branch

serbrech reviewed Jan 8, 2025

View reviewed changes

serbrech approved these changes Jan 8, 2025

View reviewed changes

dvadas reviewed Jan 8, 2025

View reviewed changes

Tatsinnit reviewed Jan 8, 2025

View reviewed changes

Tatsinnit added enhancement 🚀 New feature or request or improvements on existing code. Incremental Build 🏗️ labels Jan 8, 2025

removing irrelevant code and adding comments

4e60e6b

Tatsinnit requested review from qpetraroia, ReinierCC, tejhan and hsubramanianaks January 8, 2025 02:40

serbrech approved these changes Jan 8, 2025

View reviewed changes

Tatsinnit reviewed Jan 8, 2025

View reviewed changes

Tatsinnit previously approved these changes Jan 8, 2025

View reviewed changes

fixing typo and adding signature

468ba12

JunyuQian dismissed Tatsinnit’s stale review via 468ba12 January 9, 2025 00:08

Tatsinnit approved these changes Jan 9, 2025

View reviewed changes

serbrech approved these changes Jan 9, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding API calls to create a Fleet #1160

Adding API calls to create a Fleet #1160

JunyuQian commented Jan 7, 2025

Tatsinnit commented Jan 7, 2025

serbrech Jan 8, 2025

serbrech Jan 8, 2025

Tatsinnit Jan 8, 2025

Tatsinnit Jan 8, 2025

JunyuQian Jan 8, 2025

dvadas Jan 8, 2025

Tatsinnit Jan 8, 2025

Tatsinnit Jan 8, 2025

Tatsinnit Jan 8, 2025 •

edited

Loading

JunyuQian commented Jan 8, 2025

Tatsinnit Jan 8, 2025 •

edited

Loading

Tatsinnit left a comment •

edited

Loading

ReinierCC commented Jan 8, 2025 •

edited

Loading

Tatsinnit commented Jan 8, 2025

Adding API calls to create a Fleet #1160

Are you sure you want to change the base?

Adding API calls to create a Fleet #1160

Conversation

JunyuQian commented Jan 7, 2025

Tatsinnit commented Jan 7, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Tatsinnit Jan 8, 2025 • edited Loading

Choose a reason for hiding this comment

JunyuQian commented Jan 8, 2025

Tatsinnit Jan 8, 2025 • edited Loading

Choose a reason for hiding this comment

Tatsinnit left a comment • edited Loading

Choose a reason for hiding this comment

ReinierCC commented Jan 8, 2025 • edited Loading

Tatsinnit commented Jan 8, 2025

Tatsinnit Jan 8, 2025 •

edited

Loading

Tatsinnit Jan 8, 2025 •

edited

Loading

Tatsinnit left a comment •

edited

Loading

ReinierCC commented Jan 8, 2025 •

edited

Loading