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

Dev Box - Adding customization support #152

Merged
merged 23 commits into from
Apr 16, 2024
Merged

Dev Box - Adding customization support #152

merged 23 commits into from
Apr 16, 2024

Conversation

huzaifa-d
Copy link
Contributor

@huzaifa-d huzaifa-d commented Apr 12, 2024

Summary of the pull request

Adds customization support for Dev Boxes from the Machine Configuration > Set up a target flow

Detailed description of the pull request / Additional comments

The WinGet DSC YAML formatted string received by the extension is parsed and broken into individual units. Each unit has the format as the original, but only a single task. This is also encoded to Base64 as this is the format required for the JSON request sent through Dev Box REST APIs. The incoming YAML is broken down as the current implementation of APIs don't support sub task progress monitoring.

An example of the JSON request:

{
  "tasks": [
    {
      "name": "winget",
      "runAs": "User",
      "parameters": {
        "inlineConfigurationBase64": "<_Base64 Encoded Single Task Winget DSC YAML_>"
      },
        ....
      {
      "name": "winget",
      "runAs": "User",
      "parameters": {
        "inlineConfigurationBase64": "<_Base64 Encoded Single Task Winget DSC YAML_>"
      }
    }
  ]
}

Once the customization request has been sent, it is polled every 10 seconds for updates. The execution begins after ~30 seconds as shown by testing. The statuses of individual tasks are raised as an event for Dev Home to update its UI.

Validation steps performed

Manually ran customization on a test VM

PR checklist

@huzaifa-d huzaifa-d marked this pull request as ready for review April 12, 2024 19:08
src/AzureExtension/DevBox/DevBoxInstance.cs Outdated Show resolved Hide resolved
src/AzureExtension/DevBox/Helpers/ConfigWrapper.cs Outdated Show resolved Hide resolved
Comment on lines 88 to 91
// Remove " dependsOn: -'Git.Git | Install: Git'" from the configuration
// This is a workaround as the current implementation does not support dependsOn
configuration = configuration.Replace("dependsOn:", string.Empty);
configuration = configuration.Replace("- 'Git.Git | Install: Git'", string.Empty);
Copy link
Contributor

Choose a reason for hiding this comment

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

this is still pretty weird to me, as I'm sure they'll let other folks other than dev home submit winget files which can include the depends on value. But also as Dev Home starts to supports more things like "bring your own config file", we'd want users to pass their own config file to their Dev Boxes. At that point they can have the depends on flag anywhere in the file, depending on the Dsc resources they use.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree that this is weird. Let me actually reach out to DevBox folks to understand which side this bug is on.

Comment on lines +51 to +52
"Failed" => ConfigurationUnitState.Unknown,
"TimedOut" => ConfigurationUnitState.Unknown,
Copy link
Contributor

Choose a reason for hiding this comment

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

Should Failed and TimedOut be seen as completed instead of unknown? @sshilov7

src/AzureExtension/DevBox/Helpers/ConfigWrapper.cs Outdated Show resolved Hide resolved
src/AzureExtension/DevBox/Helpers/ConfigWrapper.cs Outdated Show resolved Hide resolved
src/AzureExtension/DevBox/Helpers/ConfigWrapper.cs Outdated Show resolved Hide resolved
Comment on lines +68 to +69
"Failed" => ConfigurationSetState.Unknown,
"ValidationFailed" => ConfigurationSetState.Unknown,
Copy link
Contributor

Choose a reason for hiding this comment

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

Should Failed and ValidationFailed be seen as completed instead of unknown? @sshilov7 ? Technically we know its completed, we just don't have a specific type for them. But validationFailed, looks like it happens when opening the configuration set fails.

@kanismohammed kanismohammed self-requested a review April 16, 2024 21:54
@huzaifa-d huzaifa-d merged commit 4f3bfed into main Apr 16, 2024
3 checks passed
@krschau krschau added this to the DHAE v0.8 milestone Apr 23, 2024
@huzaifa-d huzaifa-d deleted the user/modanish/Config branch June 20, 2024 21:36
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.

Add Dev Box Customization Support
5 participants