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

PowerShell cmdlet for calling ADU Import Update REST API #74

Closed
wants to merge 3 commits into from

Conversation

devlie
Copy link
Member

@devlie devlie commented Jun 9, 2021

  • Updated AduUpdate.psm1 module to support Import Manifest v3.
  • Added AduImportUpdate.psm1 module to create import manifest and the import API input. It also uploads update artifacts to AzBlob for staging.
  • Added AduAzStorageBlobHelper.psm1 module to help with uploading files to AzBlob.
  • Added AduRestApi.psm1 module to call ADU Import Update REST API.
  • Merged the two READMEs into one so it will appear as soon as user load the directory in GitHub.
  • Updated READMEs with new instructions.

@@ -0,0 +1,80 @@
#
# Azure Device Update for IoT Hub
Copy link

@dpokluda dpokluda Jun 9, 2021

Choose a reason for hiding this comment

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

I believe our official name is just "Device Update for IoT Hub". #Resolved

}

$account = Get-AzStorageAccount -ResourceGroupName $ResourceGroupName -Name $StorageAccountName -ErrorAction Stop
$container = New-AzStorageContainer -Name $ContainerName -Context $account.Context -ErrorAction SilentlyContinue
Copy link

@dpokluda dpokluda Jun 9, 2021

Choose a reason for hiding this comment

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

This line is here to create container if it doesn't yet exist, correct? Don't you have to do out-null in case it already exists? #Resolved

@@ -0,0 +1,80 @@
#
# Azure Device Update for IoT Hub
# Powershell module for uploading file to Azure Storage Blob Container.
Copy link

@dpokluda dpokluda Jun 9, 2021

Choose a reason for hiding this comment

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

nit: I believe it's "PowerShell". #Resolved

@@ -0,0 +1,104 @@
#
# Azure Device Update for IoT Hub
# Powershell module for creating and preparing update for import into Azure Device Update.
Copy link

@dpokluda dpokluda Jun 9, 2021

Choose a reason for hiding this comment

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

nit: I am not sure about the guidance but I am not sure whether we are officially allowed to use "ADU" or "Azure Device Update" terms. #WontFix

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we'll be fine. Our hostname is already adu.microsoft.com


Write-Verbose $RequestBody

$response = Invoke-WebRequest -Uri $url -Method POST -Headers $headers -Body $RequestBody -Verbose:$VerbosePreference
Copy link

@dpokluda dpokluda Jun 9, 2021

Choose a reason for hiding this comment

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

Have you considered Invoke-RestMethod? #WontFix

Copy link
Member Author

Choose a reason for hiding this comment

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

I need access to the response headers. Invoke-RestMethod added -ResponseHeadersVariable but that's only available from PS6 and script currently requires min PS5.

[timespan] $Timeout = [timespan]::FromMinutes(30)
)

Write-Host "Waiting for import operation $operationId to complete " -NoNewline
Copy link

@dpokluda dpokluda Jun 9, 2021

Choose a reason for hiding this comment

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

nit: Could this be used for DELETE as well? #Resolved


if ($operation.Status -ne 'Succeeded')
{
Write-Error "Failed to import update."
Copy link

@dpokluda dpokluda Jun 9, 2021

Choose a reason for hiding this comment

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

Delete? #Resolved

function New-AduImportUpdateInput
{
<#
.SYNOPSIS
Copy link

@dpokluda dpokluda Jun 9, 2021

Choose a reason for hiding this comment

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

Do we want to include .EXAMPLE for each of exported functions? #Resolved

@JeffMill
Copy link
Contributor

JeffMill commented Jun 10, 2021

companion bash script?


In reply to: 858184608


In reply to: 858184608


In reply to: 858184608

$ContainerName,

# Skip connecting to Azure.
[switch] $SkipLogin
Copy link
Contributor

@JeffMill JeffMill Jun 10, 2021

Choose a reason for hiding this comment

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

what does this do? Is the assumption that there's a cached connection already? If so, wouldn't 44-46 be a no-op? #WontFix

Copy link
Member Author

Choose a reason for hiding this comment

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

Skips connect-azaccount call. For e.g. to get ref to 2nd container, you can append -SkipLogin.

connect-azaccount doesn't know to cache anything, hence this switch.

}

$account = Get-AzStorageAccount -ResourceGroupName $ResourceGroupName -Name $StorageAccountName -ErrorAction Stop
$container = New-AzStorageContainer -Name $ContainerName -Context $account.Context -ErrorAction SilentlyContinue
Copy link
Contributor

@JeffMill JeffMill Jun 10, 2021

Choose a reason for hiding this comment

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

-ErrorAction SilentlyContinue

why silently continue? I realize that 51 will ultimately fail if this does, but you might mask the root cause of failure, no? #WontFix

Copy link
Member Author

Choose a reason for hiding this comment

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

new-azstoragecontainer throws error when container already exists.

@JeffMill
Copy link
Contributor

JeffMill commented Jun 10, 2021

looks like this uses azure blob storage, so bash might be out (although there is a "cifs" tool).

I think you might be better writing this all in python instead of powershell.

python is fine cross platform (I realize that PS core works cross platform, but outside of 98052 it's not as widely used), and azure storage has a python SDK


In reply to: 858184608

```

## Bash Script

Copy link
Contributor

@JeffMill JeffMill Jun 10, 2021

Choose a reason for hiding this comment

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

nit?: i prefer two doc files. If I'm using bash why should I wade through half of this document to find what I need? #Resolved

Copy link
Member Author

Choose a reason for hiding this comment

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

Added TOC.

GitHub would render README.md when user navigates to the directory. Having 2 readme files prevent that.

Copy link
Contributor

@JeffMill JeffMill left a comment

Choose a reason for hiding this comment

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

🕐

@devlie
Copy link
Member Author

devlie commented Jun 14, 2021

Sorry, out of scope for now.


In reply to: 858186558

@devlie devlie closed this Jun 14, 2021
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