-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
check if nic.NetworkInterfaceReferenceProperties.Primary field exists… #208
Conversation
… before trying to dereference it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @pather87
Thanks for opening this PR + issue :)
I've spent a little while trying to reproduce this machine configuration within Azure and I'm having a hard-time doing so (although, I can see this response is valid by the API documentation). Out of interest would it be possible to know how this machine was created? It'd be great if we could get a reproducible configuration for this so we could add an acceptance test to ensure we don't cause this error again - however this otherwise LGTM :)
Thanks!
Hey @tombuildsstuff I have checked the Debug output of the terraform run on a working version. We have around 30 machines running in Azure and it appears that only 2 specific machines show this behaviour. Our terraform configuration of all these machines is quite the same, so i doubt that it has something to do with that. These were one of the first machines that have been setup, so i am uncertain that this can be reproduced. IMO it might be related to a special behaviour of Azure at the time of installation. Regards |
Hey @pather87
Got you - so from my understanding Azure appears to have some kind of schema for machines - given those two machines were provisioned a while ago, they're likely to be using an older schema. When I've seen this in the past I believe All that said - this PR LGTM and I'm just running the tests now :) Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @pather87
Thanks for this change - this LGTM! :)
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks! |
… before trying to dereference it #207