Create Azure VM with Managed Service Identity #2
Conversation
|
@vishrutshah, |
Adding sample to verify that MSI extension service is running
vishrutshah
force-pushed
the
msi-sample
branch
from
ea2fed5 to
e4cc672
Compare
|
@veronicagg @sarangan12 Feel free to review when you get a chance. Thanks! |
veronicagg
left a comment
veronicagg
left a comment
There was a problem hiding this comment.
Some comments/suggestions inline.
|
|
||
| <a id="vnet"></a> | ||
| ### Create a virtual network | ||
|
|
There was a problem hiding this comment.
You may want to add a comment or two to each section, since the beginning starts with a sentence, but then it doesn't look like we "comment" much on what's expected.
Maybe here could be something, "We create a VM so we can install MSI, this looks like a regular Azure VM creation."
There was a problem hiding this comment.
should we also add a reference to a regular VM creation example?
| ``` | ||
|
|
||
| <a id="ipaddress"></a> | ||
| ### Create a public IP address |
There was a problem hiding this comment.
comment suggestion: We set up the VM by creating an IP address and its network interface,
| ``` | ||
|
|
||
| <a id="vm"></a> | ||
| ### Create a virtual machine with system identity |
There was a problem hiding this comment.
is this needed for any VM creation or is there anything special for MSI setting, maybe comment something on that?
There was a problem hiding this comment.
Assigning the identity of type SystemAssigned is the only part that is different from normal vm creation. I've added that information in the explanation.
| ``` | ||
|
|
||
| <a id="extension"></a> | ||
| ### Add MSI extension to the VM |
There was a problem hiding this comment.
Is there more we could explain here? are there some MSI docs we can refer to from Azure?
anything on where the values shown below should be coming from?
There was a problem hiding this comment.
sure. We don't have doc yet. but we can add it later once available on docs / msdn
|
|
||
| <a id="msi-extension"></a> | ||
| ### Verify MSI extension is running on VM by logging-in via ssh | ||
|
|
There was a problem hiding this comment.
probably a good idea to indicate what to look for? and why?
I see that in the actual code, we have a comment that says, run this command and look for X.
| ``` | ||
|
|
||
| <a id="delete"></a> | ||
| ### Delete the resources |
There was a problem hiding this comment.
maybe clarify that deleting this would get rid of everything, therefore if running the sample and wanting to keep stuff around, the line below should be commented out.
There was a problem hiding this comment.
added more comments for each steps for more insights as per the suggestion :)
sarangan12
left a comment
sarangan12
left a comment
There was a problem hiding this comment.
Minor nit comments as other PR. Otherwose looks fine
|
@veronicagg @sarangan12 I've addressed comments and added more docs around steps. Let me know what you think now. Thanks! |
|
@lmazuel found that this sample does require more configuration in terms of assigning roles with enough permissions. This sample is kind of incomplete :( and I missed it because the way I was creating VMs via this sample v/s way CLI / sample deployment template does differ i.e this would not work as-is. That being said, I am adding more code on role assignment that is required to make this correct. |
|
@veronicagg @sarangan12 Please re-review this sample. Thanks! |
|
@vishrutshah good to know, what wasn't working? how did it run for you before? |
|
@veronicagg I did create Azure VM from the Azure portal and added MSI extension extension then to test out sample to acquire token. While this sample I used to create linux VM and did not run the other sample on this VM but only checked whether token can be acquired using cURL request which was successful. Which missed the case that you can acquire the token but not use it unless you have role assignments. When @lmazuel looked at the sample for python he found that this was not complete and he discovered the missing step. |
4 participants
Part of Azure/azure-sdk-for-ruby#885